[lworld+vector] RFR: Merge lworld [v2]

[Xiaohong Gong]

On Tue, 26 Sep 2023 03:37:33 GMT, Xiaohong Gong <xgong at openjdk.org> wrote:

>> Jatin Bhateja has updated the pull request with a new target base due to a merge or a rebase. The pull request now contains 14 commits:
>> 
>>  - jcheck failure resolution, whitespace removal
>>  - Remove multifield related special handling from C1
>>  - Merge branch 'lworld' of http://github.com/openjdk/valhalla into merge_lworld
>>  - 8314980: [lworld+vector] consider scalarization conditions during ciMultiField creation.
>>    
>>    Reviewed-by: xgong
>>  - Merge lworld
>>    
>>    Reviewed-by: jbhateja
>>  - 8314628: [lworld+vector] validation regression fixes and cleanups.
>>    
>>    Reviewed-by: xgong
>>  - 8311610: [lworld+vector] Clean-up of vector allocation in class VectorSupport
>>    
>>    Reviewed-by: jbhateja
>>  - 8311080: [lworld+vector] Fix jdk build failures with different options
>>    
>>    Reviewed-by: jbhateja
>>  - Merge lworld
>>    
>>    Co-authored-by: Xiaohong Gong <xgong at openjdk.org>
>>    Reviewed-by: xgong
>>  - 8307715: Integrate VectorMask/Shuffle with value/primitive classes
>>    
>>    Reviewed-by: jbhateja
>>  - ... and 4 more: https://git.openjdk.org/valhalla/compare/0263bd93...fa27d069
>
> Hi @jatin-bhateja , 
> 
> Sorry for my late reply to this PR (just noticed it today)! 
> 
> Regarding to the removing of special handling for multifields in C1, it may cause a regression in test `jdk/incubator/vector/VectorRuns.java` and `jdk/incubator/vector/VectorHash.java`. 
> 
> Here is the main log: 
> 
> WARNING: Using incubator modules: jdk.incubator.vector
> java.lang.AssertionError: 1024 72
>         at VectorRuns.assertEquals(VectorRuns.java:51)
>         at VectorRuns.main(VectorRuns.java:46)
>         at java.base/jdk.internal.reflect.DirectMethodHandleAccessor.invoke(DirectMethodHandleAccessor.java:103)
>         at java.base/java.lang.reflect.Method.invoke(Method.java:582)
>         at com.sun.javatest.regtest.agent.MainWrapper$MainTask.run(MainWrapper.java:138)
>         at java.base/java.lang.Thread.run(Thread.java:1570)
> 
> JavaTest Message: Test threw exception: java.lang.AssertionError: 1024 72
> JavaTest Message: shutting down test
> 
> STATUS:Failed.`main' threw exception: java.lang.AssertionError: 1024 72
> 
> 
> I guess the `nof_nonstatic_fields` in the `ciInstanceKlass.cpp` does not contain all the multifields, although https://github.com/openjdk/valhalla/blob/lworld%2Bvector/src/hotspot/share/ci/ciEnv.cpp#L1774 returns `false ` for c1. Because the ci instance is singleton and shared between c1 and c2. So if the instance klass is created and the fields are initialized in c2 compiler thread, its nonstatic fields are just like c2 in c1 which is expected to be vectorized. 
> 
> Could you please look at this issue? Running with `"-XX:TieredStopAtLevel=3"` or adding back the original handling in c1 can make these two tests pass. Thanks!
> 
> Best Regards,
> Xiaohong

> Thanks @XiaohongGong , I wanted to give a second though to it. ci model is shared b/w compilers and should ideally be free from specializations, but as you know we have purposefully moved scalarization check upfront to reduce the complexity from field query API. As discussed earlier I am working on max species support and will soon be creating a PR. It will enable us to run entire JTREG suite and uncover regressions.

Yes, I'm also revisiting this part to check whether there are other side effect of current ci design, and I found this tricky issue. Maybe it's better to not consider the multifields in ci stage except some info passed to compiler. And compiler can choose to do the special handling itself. It's not only to c1 compiler, but also the identity class. We may also think about the scene that the `MultiField` is used in an identity class, although it does not have the usage now.

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

PR Comment: https://git.openjdk.org/valhalla/pull/929#issuecomment-1734910607