RFR: 8345067: C2: enable implicit null checks for ZGC reads [v5]

[Roberto Castañeda Lozano]

On Tue, 27 May 2025 07:46:22 GMT, Roberto Castañeda Lozano <rcastanedalo at openjdk.org> wrote:

>> Thanks for implementing it and thanks for the ping. It basically works on PPC64, but one IR rule is failing:
>> 
>> Failed IR Rules (1) of Methods (1)
>> ----------------------------------
>> 1) Method "static java.lang.Object compiler.gcbarriers.TestImplicitNullChecks.testLoadVolatile(compiler.gcbarriers.TestImplicitNullChecks$OuterWithVolatileField)" - [Failed IR rules: 1]:
>>    * @IR rule 1: "@compiler.lib.ir_framework.IR(phase={FINAL_CODE}, applyIfPlatformAnd={}, applyIfCPUFeatureOr={}, counts={"_#NULL_CHECK#_", "1"}, applyIfPlatformOr={}, applyIfPlatform={"aarch64", "false"}, failOn={}, applyIfOr={"UseZGC", "true", "UseG1GC", "true"}, applyIfCPUFeatureAnd={}, applyIf={}, applyIfCPUFeature={}, applyIfAnd={}, applyIfNot={})"
>>      > Phase "Final Code":
>>        - counts: Graph contains wrong number of nodes:
>>          * Constraint 1: "(\d+(\s){2}(NullCheck.*)+(\s){2}===.*)"
>>            - Failed comparison: [found] 0 = 1 [given]
>>            - No nodes matched!
>> 
>> 
>> This is probably because PPC64 uses a membar_volatile before volatile load, so the graph looks differently:
>> 
>>          33  Prolog  ===  [[ ]]   [2380000000033]
>>           9  MachProj  === 10  [[ 8 ]] #0/unmatched !jvms: TestImplicitNullChecks::testLoadVolatile @ bci:-1 (line 85)
>>  R3      11  MachProj  === 10  [[ 8 26 ]] #5  Oop:compiler/gcbarriers/TestImplicitNullChecks$OuterWithVolatileField * !jvms: TestImplicitNullChecks::testLoadVolatile @ bci:-1 (line 85)
>>          12  MachProj  === 10  [[ 4 17 ]] #1/unmatched !jvms: TestImplicitNullChecks::testLoadVolatile @ bci:-1 (line 85)
>>          13  MachProj  === 10  [[ 4 21 ]] #2/unmatched  Memory: @BotPTR *+bot, idx=Bot; !jvms: TestImplicitNullChecks::testLoadVolatile @ bci:-1 (line 85)
>>  R1      14  MachProj  === 10  [[ 4 2 17 ]] #3 !jvms: TestImplicitNullChecks::testLoadVolatile @ bci:-1 (line 85)
>>          15  MachProj  === 10  [[ 4 17 ]] #4 !jvms: TestImplicitNullChecks::testLoadVolatile @ bci:-1 (line 85)
>>           0  Con  === 10  [[ ]]  #top
>>           8  zeroCheckP_reg_imm0  === 9 11  [[ 7 22 ]] P=0.000001, C=-1.000000 !jvms: TestImplicitNullChecks::testLoadVolatile @ bci:1 (line 85)
>> 
>> BB#002:
>>          31  Region  === 31 22  [[ 31 21 26 ]] 
>>          21  membar_volatile  === 31 0 13 0 0  [[ 20 23 ]]  !jvms: TestImplicitNullChecks::testLoadVolatile @ bci:1 (line 85)
>>          20  MachProj  === 21  [[ 19 ]] #0/unmatched !jvms: TestImplicitNullChecks::testLoadVolatile @ bci:1 (line 85)
>>          23  MachProj  === 21 ...
>
>> I guess it's not worth stepping over the memory barrier. Disabling this rule for PPC64 should be ok, too.
> 
> Thanks for testing and reporting @TheRealMDoerr, I agree that it would be too much complexity for little return. I disabled the rule for PPC64 (commit fdf34f90). Please let me know if that works as expected.

> @robcasloz : Hi, Thanks for the ping! I performed tier1-3 tests on linux-riscv64 platform, result is good. The new test `test/hotspot/jtreg/compiler/gcbarriers/TestImplicitNullChecks.java` also pass when running with G1 and ZGC using fastdebug build.

@RealFYang Thanks for testing and reporting!

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

PR Comment: https://git.openjdk.org/jdk/pull/25066#issuecomment-2911518019