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:18:18 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 1739:
> 
>> 1737:   // never pop anything from the worklist, since that could result in applying
>> 1738:   // the callback for a use more than once (if it is at some point readded to
>> 1739:   // the worklist).
> 
> This is block style, but it would be ok to make lines longer

What's the convention? I'm used to aligning so that no line breaks 80 columns, and it looks like most comments in `node.hpp` follow this convention as well.

> src/hotspot/share/opto/node.hpp line 1744:
> 
>> 1742:   for (DUIterator_Fast kmax, k = fast_outs(kmax); k < kmax; k++) {
>> 1743:     worklist.push(fast_out(k));
>> 1744:   }
> 
> An alternative could be to just push "this", and then in the loop: `if (use == this) { continue; }`
> Would just save the code duplication.

True, but I do not like having a conditional in the loop that is redundant in all but the first iteration. I prefer the current solution, but I'm also OK with changing. Maybe there is a third even more elegant solution?

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

PR Review Comment: https://git.openjdk.org/jdk/pull/16429#discussion_r1386790562
PR Review Comment: https://git.openjdk.org/jdk/pull/16429#discussion_r1386787150