RFR: 8302459: Missing late inline cleanup causes compiler/vectorapi/VectorLogicalOpIdentityTest.java IR failure [v3]

[Damon Fenacci]

On Fri, 7 Mar 2025 20:41:50 GMT, Vladimir Ivanov <vlivanov at openjdk.org> wrote:

>> Thanks a lot for having a look @iwanowww!
>> 
>> I took me a while to recollect it too (and I remember having a hard time figuring out if that could be an issue back then 😉). Anyway the concern, as you said, was that there might be an infinite loop between IGVN and incremental inlining (presumably because during incremental inlining the call node could potentially slip back into the working list, right?).
>> If that is the root of the problem, the issue would only exist in the `false -> true` case. In the (potential) `true -> false` case the call node wouldn't be scheduled for IGVN in the next round, so there wouldn't be any loop. Maybe we could even transform the statement into something like:
>> 
>> if (C->igvn_worklist()->member(cg->call_node()) && is_scheduled_for_igvn_before) {
>>   cg->call_node()->set_generator(cg);
>> }
>> 
>> What do you think?
>
> So, since current logic for generic (non-MH case) case conservatively assumes that any change in inputs may benefit inlining (and unconditionally schedules such call nodes for another inlining attempt during IGVN), we want to avoid the situation when call node gets scheduled for IGVN during failed inlining attempt. 
> 
> I'd shape it as follows:
> 
> if (!is_scheduled_for_igvn_before && is_scheduled_for_igvn_after) { // avoid potential infinite loop
>   assert(false, "scheduled for IGVN during inlining attempt"); 
> } else {
>   assert(is_scheduled_for_igvn_before == is_scheduled_for_igvn_after, "interesting"); // removed from IGVN list during inlining pass?
>   cg->call_node()->set_generator(cg); // wait for another opportunity
> }

Fair enough. I reshaped it following your suggestion. Thanks!

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

PR Review Comment: https://git.openjdk.org/jdk/pull/21682#discussion_r1986949416