RFR: 8292898: [vectorapi] Unify vector mask cast operation [v5]

Xiaohong Gong xgong at openjdk.org
Mon Oct 10 06:15:56 UTC 2022


On Mon, 10 Oct 2022 06:00:38 GMT, Jatin Bhateja <jbhateja at openjdk.org> wrote:

> > > > > > > Hi @jatin-bhateja , the IR test has been added. Could you please help to review again? Thanks a lot!
> > > > > > 
> > > > > > 
> > > > > > Some of the IR tests like testByte64ToLong512 are currently are failing on KNL due to following check https://github.com/openjdk/jdk/blob/master/src/hotspot/share/opto/vectorIntrinsics.cpp#L2484
> > > > > > since source and destination ideal types are different (TypeVect vs TypeVectMask), can you kindly change the feature check for relevant IR tests to avx512vl till we remove that limitation.
> > > > > 
> > > > > 
> > > > > Thanks for pointing out this issue. Sure, I will limit the feature check to "avx512vl" for all the 512 bits related casting. BTW, could you please show me how to run the test with KNL feature? So that I can have an internal test before pushing the changes. Thanks a lot!
> > > > 
> > > > 
> > > > Hi @jatin-bhateja , the test is updated. I tested it with `-XX:+UseKNLSetting` by adding the flag to
> > > > `TestFramework.runWithFlags()` in the main function, and tests pass. Could you please help to check whether it is ok for you?
> > > > Thanks!, we can also pass additional flag in JTREG_WHITELIST_FLAGS in TestFramework.java
> > > > Thanks a lot!
> > > 
> > > 
> > > Hi @XiaohongGong , Thanks for addressing my comments, test now passes on KNL platform. Newly introduced @WarmUp annotation in all the tests looks redundant since in NORMAL run-mode framework does the necessary warmup followed by compilation by "C2' (default compiler).
> > 
> > 
> > Hi @jatin-bhateja , thanks for looking at the changes again! Yes, you are right that the fromework has a default warmup (2000). But I'd like to keep the new added 10000 here, because I met the IR check failing issues when I wrote another IR test and set the warmup as 5000 before. To be honest,I don't know why it fails since the method is compiled by C2 but the compiler shows it lost some information, which made the expected IR not generated. So adding the larger warmup is safe for me. WDYT? Thanks!
> 
> Framework is using white box APIs to enqueue test methods to compile queues from which they will be picked up by respective compilers, so test method compilation here is agnostic to warmup invocation count, but warmup will ensure that some of the closed world assumptions needed for intrinsification are met.
> 
> Changes still looks good to me. Thanks!

I see. Thanks a lot for the clarification and reviewing!

-------------

PR: https://git.openjdk.org/jdk/pull/10192


More information about the hotspot-compiler-dev mailing list