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

[Daniel Lundén]

On Thu, 2 Nov 2023 16:39:51 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:
>> 
>>   Fix typo in visit_uses comment
>
> src/hotspot/share/opto/node.hpp line 1744:
> 
>> 1742:         internals.push(internal_use); // traverse this also
>> 1743:       } else {
>> 1744:         callback(internal_use);
> 
> uses may now have the callback invoked multiple times, for example if we have something like this:
> 
> 
>             this
>              |
>    +---------+--------+
>    |                  |
>    x                  y
>    |                  |
>    +--------+ +-------+
>             | |
>             use
> 
> This could not happen with Cast nodes (I think), but now that we generalize the function we should improve the logic, such that callback only gets called once.

Yes, it is true that we sometimes may visit nodes multiple times. However, is it worth the extra bookkeeping overhead to ensure uses are visited only once? At least for the current applications of `visit_uses`, it's not unsound to (quite rarely, I think) add a node twice or more times to worklists.

I'm fine with either option, let me know what you think.

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

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