RFR: 8313672: C2: PhaseCCP does not correctly track analysis dependencies involving shift, convert, and mask [v11]

[Daniel Lundén]

On Wed, 8 Nov 2023 09:14:28 GMT, Emanuel Peter <epeter at openjdk.org> wrote:

>> Daniel Lundén has updated the pull request incrementally with one additional commit since the last revision:
>> 
>>   Add UnlockDiagnosticVMOptions to allow running new test on product build
>
> src/hotspot/share/opto/node.hpp line 1135:
> 
>> 1133:   // reaching a boundary node, defined by is_boundary.
>> 1134:   // Note: the function definition appears after the complete type definition
>> 1135:   // of Unique_Node_List.
> 
> Nit: generally we try to make the comments in "block" style. You could also make the lines a bit wider.

Are you referring to the line break between the first sentence and the `Note:`? I'm aligning on 80 columns, but will of course change to the agreed upon convention.

> src/hotspot/share/opto/node.hpp line 1142:
> 
>> 1140:   // Pattern: this (-> ConstraintCast)* -> non_cast
>> 1141:   // In other words: find all non_cast nodes such that
>> 1142:   // non_cast->uncast() == this.
> 
> same here

This is the original comment for `visit_uncasted_uses` (I haven't changed it). But, I'm fine with changing it to something better (see my question above as well).

> test/hotspot/jtreg/compiler/ccp/TestShiftConvertAndNotification.java line 41:
> 
>> 39:  * @summary Test CCP notification for value update of AndL through LShiftI and
>> 40:  *          ConvI2L (reduced set of flags).
>> 41:  * @run main/othervm compiler.ccp.TestShiftConvertAndNotification
> 
> Suggestion:
> 
>  * @run driver compiler.ccp.TestShiftConvertAndNotification
> 
> If you have no flags, you don't need to spawn a new VM. `driver` executes the tests in the driver vm.
> 
> https://openjdk.org/jtreg/faq.html#what-are-the-agentvm-and-othervm-modes

Thanks, I'll change it

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

PR Review Comment: https://git.openjdk.org/jdk/pull/16429#discussion_r1386798699
PR Review Comment: https://git.openjdk.org/jdk/pull/16429#discussion_r1386798952
PR Review Comment: https://git.openjdk.org/jdk/pull/16429#discussion_r1386800646