RFR[M]: 8151779: Some intrinsic flags could be replaced with one general flag

[Liu, Xin]

Hi, Corey and Vladimir, 

I recently go through vmSymbols.hpp/cpp. I think I understand your comments. 
Each UseXXXIntrinsics does control a bunch of intrinsics (plural). Thanks for the hint.

Even though I feel I know intrinsics mechanism of hotspot better, I still need a clarification of JDK- 8151779. 

There're 321 intrinsics (https://chriswhocodes.com/hotspot_intrinsics_jdk15.html).
If there's no any option, they are all available for compilers.  That makes sense because intrinsics are always beneficial.  
But there're reasons we need to disable a subset of them. A specific architecture may miss efficient instructions or fixed functions. Or simply because an intrinsic is buggy.

Currently, JDK provides developers 2 ways to control intrinsics. 
1. Some diagnostic options. Eg. InlineMathNatives, UseBase64Intrinsics.
Developers can use one option to disable a group of intrinsics.  That is to say, it's a coarse-grained approach.
 
2. DisableIntrinsic="a,b,c"
By passing a string list of vmIntrinsics::IDs, it's capable of disabling any specified intrinsic.

But even putting above 2 approaches together, we still can't precisely control any intrinsic.
If we want to enable an intrinsic which is under control of InlineMathNatives but keep others disable, it's impossible now.  [please correct if I am wrong here].
I think that the motivation JDK-8151779 tried to solve.

If we provide a new option EnableIntrinsic and put it least priority, then we can precisely control any intrinsic.
Quote Vladimir Kozlov "DisableIntrinsic list prevails if an intrinsic is specified on both EnableIntrinsic and DisableIntrinsic."

 "-XX:ControlIntrinsic=+_dabs,-_fabs,-_getClass" looks more elegant, but it will confuse developers with DisableIntrinsic.  
If we decide to deprecate DisableIntrinsic, I think ControlIntrinsic may be a better option. Now I prefer to provide EnableIntrinsic for simplicity and symmetry. 
What do you think?

Thanks,
--lx


On 4/13/20, 1:47 PM, "hotspot-compiler-dev on behalf of Corey Ashford" <hotspot-compiler-dev-bounces at openjdk.java.net on behalf of cjashfor at linux.ibm.com> wrote:

    CAUTION: This email originated from outside of the organization. Do not click links or open attachments unless you can confirm the sender and know the content is safe.



    On 4/13/20 10:33 AM, Liu, Xin wrote:
    > Hi, compiler developers,
    > I attempt to refactor UseXXXIntrinsics for JDK-8151779.  I think we still need to keep UseXXXIntrinsics options because many applications may be using them.
    >
    > My change provide 2 new features:
    > 1) a shorthand to enable/disable intrinsics.
    > A comma-separated string. Each one is an intrinsic. An optional tailing symbol + or '-' denotes enabling or disabling.
    > If the tailing symbol is missing, it means enable.
    > Eg. -XX:UseIntrinsics="AESCTR-,CRC32C+,CRC32-,MathExact"
    > This jvm option will expand to multiple options -XX:-UseAESCTRIntrinsics, -XX:+UseCRC32CIntrinsics, -XX:-UseCRC32Intrinsics, -XX:UseMathExactIntrinsics
    >
    > 2) provide a set of macro to declare intrinsic options
    > Developers declare once in intrinsics.hpp and macros will take care all other places.
    > Here are example: https://cr.openjdk.java.net/~xliu/8151779/00/webrev/src/hotspot/share/compiler/intrinsics.hpp.html
    > Ion Lam is overhauling jvm options.  I am thinking how to be consistent with his proposal.
    >

    Great idea, though to be consistent with the original syntax, I think
    the +/- should be in front of the name:

    -XX:UseIntrinsics=-AESCTR,+CRC32C,...


    > I handle UseIntrinsics before VM_Version::initialize. It means that platform-specific initialization still has a chance to correct those options.
    > If we do that after VM_Version::initialize,  some intrinsics may cause JVM crash.  Eg. +UseBase64Intrinsics on x86_64 Linux.
    > Even though this behavior is same as -XX:+UseXXXIntrinsics, from user's perspective, it's not straightforward when JVM overrides what users specify implicitly. It's dilemma here,  stable jvm or fidelity of cmdline.  What do you think?
    >
    > Another problem is naming convention. Almost all intrinsics options use UseXXXIntrinsics. One exception is UseVectorizedMismatchIntrinsic.
    > Personally, I think it should be "UseXXXIntrinsic" because one option is for one intrinsic, right?  Is it possible to change this name convention?

    Some (many?) intrinsic options turn on more than one .ad instruct
    instrinsic, or library instrinsics at the same time.  I think that's why
    the plural is there.  Also, consistently adding the plural allows you to
    add more capabilities to a flag that initially only had one intrinsic
    without changing the plurality (and thus backward compatibility).

    Regards,

    - Corey