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

[Vladimir Ivanov]

On Thu, 20 Mar 2025 13:05:19 GMT, Tobias Hartmann <thartmann at openjdk.org> wrote:

>> Damon Fenacci has updated the pull request incrementally with two additional commits since the last revision:
>> 
>>  - JDK-8302459: refactor helper method
>>  - JDK-8302459: reshape infinite loop check
>
> 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.

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

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