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

[Emanuel Peter]

On Wed, 26 Apr 2023 11:09:52 GMT, Christian Hagedorn <chagedorn at openjdk.org> wrote:

>> Emanuel Peter has updated the pull request incrementally with one additional commit since the last revision:
>> 
>>   corrected java whitespaces
>
> Looks good!

@chhagedorn @TobiHartmann Thanks for the reviews.
I could integrate now.

But I have one more discovery:
The reason why I cannot place the mentioned `ResoureMark` is that the `worklist` has its arena as the `Thread::current()->resource_area()` in some cases. One example is the `Unique_Node_List worklist` in `PhaseCCP::analyze`. So then in `PhaseCCP::push_and`, this `worklist` gets captured by the lambda, and passed to `ConstraintCastNode::visit_uncasted_uses`. If there is a `ResourceMark` inside it, and we re-allocate memory for the `worklist` because it grows beyond its current capacity, then we will de-allocate that memory when we exit the `ResourceMark`, and the `worklist` now points to de-allocated memory.

Without a `ResourceMark`, this can be avoided. But then we never release the memory for the `Unique_Node_List internals`. 

The second use of `ConstraintCastNode::visit_uncasted_uses` in `PhaseIterGVN::add_users_to_worklist` does not run into this issue, even if we have a `ResourceMark`. The reason is that there the `worklist` that is captured allocates from a separate arena, namely the `Compile::current()->comp_arena()`.

We could thus also consider having the `worklist` inside `PhaseCCP::analyze` allocate also from the `Compile::current()->comp_arena()`. One downside is that we never de-allocate memory in that arena until the end of `Compile`. But currently, there is also no `ResourceMark` upstream of `PhaseCCP::analyze`, all the way up to `Compile`. So this `worklist` never has its memory de-allocated anyway.

What is your opinion? Should I make such changes, or just integrate what I have now?

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

PR Comment: https://git.openjdk.org/jdk/pull/13611#issuecomment-1524868040