RFR(S): 8247732: validate user-input intrinsic_ids in ControlIntrinsic

[Liu, Xin]

Hi, Nils, 

Could you take a look at this PR:
https://github.com/openjdk/jdk/pull/1179

It should be the last patch of ControlIntrinsic, which add validation logic and regression test. 

I just renew it from the webrev.  Because global.hpp has changed since then, I modify slightly to make sure it works with the new macros. 
This is non-trivial because the end-users have multiple approaches to use Disable/ControlIntrinsic.  Now it conforms to the spec you gave me. 
https://bugs.openjdk.java.net/browse/JDK-8247732?focusedCommentId=14362773&page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel#comment-14362773

I add a few tests to cover different use cases.  The only case that doesn't cover is invalid input directly from vmflag.   Here it is:
$java -XX:+UnlockDiagnosticVMOptions -XX:ControlIntrinsic=+_dtan,+_hello,+_max -version
Unrecognized intrinsic detected in ControlIntrinsic: _hello
Error: Could not create the Java Virtual Machine.
Error: A fatal exception has occurred. Program will exit.

Thanks,
--lx


On 8/3/20, 11:41 PM, "hotspot-runtime-dev on behalf of Liu, Xin" <hotspot-runtime-dev-retn at openjdk.java.net on behalf of xxinliu at amazon.com> wrote:

    hi, Nils, 

    Tobias would like to keep the parser behavior consistency.  I think it means that the hotspot need to suppress the warning if the intrinsic_id doesn't exists in compiler directive.  
    eg. -XX:CompileCommand=option,<pattern>,ControlIntrinsic=-_nonexist.

    What do you think about it? 

    Here is the latest webrev:
    http://cr.openjdk.java.net/~xliu/8247732/01/webrev/

    thanks,
    --lx

    ________________________________________
    From: Tobias Hartmann <tobias.hartmann at oracle.com>
    Sent: Friday, July 24, 2020 2:52 AM
    To: Liu, Xin; Nils Eliasson; hotspot-compiler-dev at openjdk.java.net; hotspot-runtime-dev
    Subject: RE: [EXTERNAL] RFR(S): 8247732: validate user-input intrinsic_ids in ControlIntrinsic

    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.



    Hi Liu,

    On 23.07.20 18:02, Liu, Xin wrote:
    > That is my intention too, but CompilerOracle doesn't exit JVM when it encounters parsing errors.
    > It just exacts information from CompileCommand as many as possible. That makes sense because compiler "directives" are supposed to be optional for program execution.
    >
    > I do put the error message in parser's errorbuf.  I set a flag "exit_on_error" to quit JVM after it dumps parser errors. yes, I treat undefined intrinsics as fatal errors.
    > This behavior is from Nils comment: "I want to see an error on startup if the user has specified unknown intrinsic names."  It is also consistent with JVM option -XX:ControlIntrinsic=.

    Okay, thanks for the explanation! I would prefer consistency in error handling of compiler
    directives, i.e., handle all parser failures the same way. But I leave it to Nils to decide.

    Best regards,
    Tobias