RFR: 8306042: C2: failed: Missed optimization opportunity in PhaseCCP (adding LShift->Cast->Add notification) [v4]

[Emanuel Peter]

On Fri, 28 Apr 2023 18:56:05 GMT, Vladimir Kozlov <kvn at openjdk.org> wrote:

>> Emanuel Peter has updated the pull request incrementally with one additional commit since the last revision:
>> 
>>   CCP worklist on local arena
>
> src/hotspot/share/opto/phaseX.cpp line 1961:
> 
>> 1959:   // Push root onto worklist
>> 1960:   worklist.push(C->root());
>> 1961:   DEBUG_ONLY(Unique_Node_List worklist_verify;)
> 
> Should you put `worklist_verify` to `local_arena` too?

@vnkozlov I could do that, but it is not required. The CCP `worklist` gets passed downward much farther, including the  graph walks in `push_child_nodes_to_worklist`. So there it is nice to be able to have `ResourceMarks`. But `worklist_verify` is only modified directly in `PhaseCCP::analyze` and one layer deeper in `PhaseCCP::verify_analyze`. So I would never expect a `ResourceMark` to mess with re-allocation.

So at this point the only reason to add `worklist_verify` to `local_arena` is to ensure it is de-allocated afterward. We could also use a `ResourceMark` and leave it on `Thread::current()->resource_area()`.

Anyway, I'll just move it to `local_arena`, after all it is `DEBUG_ONLY`.

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

PR Review Comment: https://git.openjdk.org/jdk/pull/13611#discussion_r1182123382