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

[Tobias Hartmann]

On Fri, 21 Mar 2025 22:47:30 GMT, Vladimir Ivanov <vlivanov at openjdk.org> wrote:

>> src/hotspot/share/opto/compile.cpp line 2050:
>> 
>>> 2048:           assert(is_scheduled_for_igvn_before == is_scheduled_for_igvn_after, "call node removed from IGVN list during inlining pass");
>>> 2049:           cg->call_node()->set_generator(cg);
>>> 2050:         }
>> 
>> I find this a bit hard to read. Wouldn't it be semantically equivalent to this?
>> 
>> 
>> if (is_scheduled_for_igvn_before == is_scheduled_for_igvn_after) {
>>   cg->call_node()->set_generator(cg);
>> } else {
>>   assert(false, "Some useful message");
>> }
>> 
>> 
>> We wouldn't have separate asserts for the two cases, but I think that's fine since one can easily figure it out from the boolean values.
>
> The difference is whether a call can be scheduled for a repeated inlining attempt in the future. 
> 
> `cg->call_node()->set_generator(cg)` reinitializes `cg` in `CallNode` and lets IGVN to submit it for incremental inlining during future passes.
> 
> The first check guards against a situation when the call node is already on IGVN list (so, it will be automatically rescheduled for inlining during the next IGVN pass causing an infinite loop in incremental inlining).
> 
> The second assert catches a suspicious situation when the call node disappears from IGVN worklist during failed inlining attempt. IMO it should not happens, hence the assert. But it is benign to allow repeated inlining in such case.

Okay, thanks for the background. If we keep both asserts, some comments explaining this would be helpful.

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

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