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

[Roberto Castañeda Lozano]

On Thu, 8 May 2025 11:04:26 GMT, Emanuel Peter <epeter at openjdk.org> wrote:

>> Roberto Castañeda Lozano has updated the pull request incrementally with nine additional commits since the last revision:
>> 
>>  - Generalize tests by removing requires annotation and adding local applyIf rules
>>  - Assert that we do not move control nodes
>>  - Extend comment about hoisting DecodeN inputs
>>  - Apply Emanuels suggestions to ensure_node_is_at_block_or_above
>>  - Rename auxiliary functions
>>  - Rename auxiliary functions
>>  - Clarify scope of move_into
>>  - Extend comment about MachTemp nodes
>>  - Extract and reuse legitimize_address test
>
> test/hotspot/jtreg/compiler/gcbarriers/TestImplicitNullChecks.java line 119:
> 
>> 117:                 testLoad(o);
>> 118:             } catch (NullPointerException e) { nullPointerException = true; }
>> 119:             Asserts.assertTrue(nullPointerException);
> 
> Suggestion:
> 
>             try {
>                 testLoad(o);
>                 throw new RuntimeException("Should have thrown NullPointerException");
>             } catch (NullPointerException e) { /* expected */}
> 
> Could be a shorter alternative. Up to you. Maybe there is a benefit to `Asserts.assertTrue` I am also not aware of?
> But totally optional, as your approach works anyway :)

I rather prefer the current version with `Asserts.assertTrue(nullPointerException)`, because it makes the test expectations more explicit (no need for an `/* expected */` comment or similar).

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

PR Review Comment: https://git.openjdk.org/jdk/pull/25066#discussion_r2087205670