JDK-8151779 : Some intrinsic flags could be replaced with one general flag
  • Type: Enhancement
  • Component: hotspot
  • Sub-Component: compiler
  • Affected Version: 9,10
  • Priority: P4
  • Status: Resolved
  • Resolution: Fixed
  • OS: generic
  • CPU: generic
  • Submitted: 2016-03-13
  • Updated: 2024-11-13
  • Resolved: 2020-06-23
The Version table provides details related to the release that this issue/RFE will be addressed.

Unresolved : Release in which this issue/RFE will be addressed.
Resolved: Release in which this issue/RFE has been resolved.
Fixed : Release in which this issue/RFE has been fixed. The release containing this fix may be available for download as an Early Access Release or a General Availability Release.

To download the current JDK release, click here.
JDK 16
16 b03Fixed
Related Reports
Duplicate :  
JDK-8235981 - Introduce a flag to enable JVM intrinsics
Relates :  
JDK-8235981 - Introduce a flag to enable JVM intrinsics
Relates :  
JDK-8145348 - Make intrinsics flags diagnostic
Relates :  
JDK-8247732 - validate user-input intrinsic_ids in ControlIntrinsic
Description
Currently we have individual flags to control some intrinsics generation. The more intrinsics we add - more flags we have. We should avoid adding new flags which are costly to maintain and remember.

We have  DisableIntrinsic flag to specify list of intrinsics which should be disabled:
 diagnostic(ccstrlist, DisableIntrinsic, "",

We could add opposite flag to specify list of intrinsics which should be enabled:

diagnostic(ccstrlist, EnableIntrinsic, "",

Some intrinsics flag control set of intrinsics, for example, InlineUnsafeOps so we may keep them as flags. But flags which control specific intrinsic, for example, UseAESIntrinsics could be converted.

Note, some flags setting are based on hardware feature and are set ergonomically in vm_version_<arch>.cpp files.
Comments
Changeset: 4076ca82 Author: Xin Liu <xliu@openjdk.org> Date: 2020-06-23 10:40:26 +0000 URL: https://git.openjdk.java.net/panama-foreign/commit/4076ca82
02-07-2020
Changeset: 4076ca82 Author: Xin Liu <xliu@openjdk.org> Date: 2020-06-23 10:40:26 +0000 URL: https://git.openjdk.java.net/amber/commit/4076ca82
02-07-2020
Changeset: 4076ca82 Author: Xin Liu <xliu@openjdk.org> Date: 2020-06-23 10:40:26 +0000 URL: https://git.openjdk.java.net/mobile/commit/4076ca82
02-07-2020
URL: https://hg.openjdk.java.net/jdk/jdk/rev/caad1ce9bb8f User: phh Date: 2020-06-23 17:42:01 +0000
23-06-2020
here is discussion: https://mail.openjdk.java.net/pipermail/hotspot-compiler-dev/2020-May/038311.html
12-06-2020
It was followup JDK-8145348 changes when we converted intrinsics flags from product to diagnostic and have to go through CCC approval process for all of them. At that time Joe D. said: "Hi Rahul, I'm voting to approve your ccc request for 8145348: Make all intrinsics flags to diagnostic However, I encourage the HotSpot team to consider a different command line interface for managing these features. Something like UseIntrinsics:<List of intrinsics +'ed or -'ed> That way there is something closer to one command line flag as opposed to a over a dozen. Thanks, -Joe " Recently Vladimir I. filed similar JDK-8235981 based on discussion in JDK-8167065 comments about need control flags for all new x86 CPU features we use: https://bugs.openjdk.java.net/browse/JDK-8167065?focusedCommentId=14306703&page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel#comment-14306703 I think it would be nice to have common solution for this RFE and JDK-8235981 like suggested ControlIntrinsic flag. Note, originally long ago when we started adding new intrinsics we thought to have control flags only temporary during original debugging and testing and remove them for release. Currently we use some of flags to enable intrinsics on platforms which supports them. Which could be done internally in HotSpot instead of user visible flags. And definitely we may need to keep some feature flag as separate flag, like UseAVX.
04-03-2020
>> ./share/runtime/globals.hpp:245: product(bool, UseBASE64Intrinsics, false, > Should that really be a product flag given all the others are diagnostic? I second that. there is also 'src/hotspot/cpu/x86/globals_x86.hpp:194: product(bool, UseSSE42Intrinsics, false, ', which should be made diagnostic.
03-01-2020
> 1) Curb the increase of intrinsics options. It should be very common to bring up a new intrinsic. If we introduce a new java option every time, we have to maintain it. It would carry over a multiple of JDK versions. one option with arguments is a scalable solution. unless we are willing (which I don't we are) to sacrifice robustness and ease of use of intrinsics related options, we will have to maintain the similar list of supported values for DisableIntrinsic/EnableIntrinsic, and it will carry over multiple JDK release the same way as actual VM flags with the same problems plus some new ones such as a deprecation of supported values. > 2) support Compiler Directive. To triage compiler problem, directives are useful. Developer can tweak intrinsics on/off fine-grained in compiler directive. true, but CompilerDirective supports "real" VM flags. granted, some amount of extra work is required to make it recognize a new VM flag, but I believe it can (and even should) be improved.
02-01-2020
hi, Igor, I totally agree with you. That's why I'd like to wait for more clarification before digging into code. IMHO, I can come up 2 supportive arguments: 1) Curb the increase of intrinsics options. It should be very common to bring up a new intrinsic. If we introduce a new java option every time, we have to maintain it. It would carry over a multiple of JDK versions. one option with arguments is a scalable solution. 2) support Compiler Directive. To triage compiler problem, directives are useful. Developer can tweak intrinsics on/off fine-grained in compiler directive.
02-01-2020
I have a hard time understanding the very premise of this RFE (The more intrinsics we add - more flags we have. We should avoid adding new flags which are costly to maintain and remember) as I don't see how having two flags which can have N values is any less costly than having N flags which take 2 values. [~kvn] Vladimir, could you please elaborate on why you think it would be beneficial?
31-12-2019
> [Xin Liu] If we have both DisableIntrinsic and EnableIntrinsic, which one should take precedence? > [Vladimir K.] DisableIntrinsic list prevails if an intrinsic is specified on both EnableIntrinsic and DisableIntrinsic. Though the usual rule in hotspot, where we process flags in order left-to-right, is that last flag wins. > ./share/runtime/globals.hpp:245: product(bool, UseBASE64Intrinsics, false, Should that really be a product flag given all the others are diagnostic? If only diagnostic flags are involved then no CSR request is required. Though I'd argue that for a change like this you should still keep the old flags around for a couple of releases and alias them to the new flag. It's also unclear how the way the flags are specified on the command-line actually map through to variables in the VM that can be used to affect the JIT logic.
31-12-2019
I found those flags are candidates. ./share/runtime/globals.hpp:242: diagnostic(bool, UseGHASHIntrinsics, false, ./share/runtime/globals.hpp:245: product(bool, UseBASE64Intrinsics, false, ./share/runtime/globals.hpp:339: diagnostic(bool, UseAESIntrinsics, false, ./share/runtime/globals.hpp:342: diagnostic(bool, UseAESCTRIntrinsics, false, ./share/runtime/globals.hpp:345: diagnostic(bool, UseSHA1Intrinsics, false, ./share/runtime/globals.hpp:349: diagnostic(bool, UseSHA256Intrinsics, false, ./share/runtime/globals.hpp:353: diagnostic(bool, UseSHA512Intrinsics, false, ./share/runtime/globals.hpp:357: diagnostic(bool, UseCRC32Intrinsics, false, ./share/runtime/globals.hpp:360: diagnostic(bool, UseCRC32CIntrinsics, false, ./share/runtime/globals.hpp:363: diagnostic(bool, UseAdler32Intrinsics, false, ./share/runtime/globals.hpp:366: diagnostic(bool, UseVectorizedMismatchIntrinsic, false, ./share/opto/c2_globals.hpp:704: diagnostic(bool, UseMathExactIntrinsics, true, ./share/opto/c2_globals.hpp:707: diagnostic(bool, UseCharacterCompareIntrinsics, false, ./share/opto/c2_globals.hpp:710: diagnostic(bool, UseMultiplyToLenIntrinsic, false, ./share/opto/c2_globals.hpp:713: diagnostic(bool, UseSquareToLenIntrinsic, false, ./share/opto/c2_globals.hpp:716: diagnostic(bool, UseMulAddIntrinsic, false, ./share/opto/c2_globals.hpp:719: diagnostic(bool, UseMontgomeryMultiplyIntrinsic, false, ./share/opto/c2_globals.hpp:722: diagnostic(bool, UseMontgomerySquareIntrinsic, false, If we have both DisableIntrinsic and EnableIntrinsic, which one should take precedence? The argument of DisableIntrinsic is a list of intrinsic ID. Of course, it could be very general, but it's hard to memorize. Is that good idea to replace those flags with a list of intrinsics IDs?
31-12-2019
This affects Future JIT too. Still need to work on it but it is not urgent.
05-03-2018
DisableIntrinsic list prevails if an intrinsic is specified on both EnableIntrinsic and DisableIntrinsic.
14-03-2016