RFR: 8364757: Missing Store nodes caused by bad wiring in PhaseIdealLoop::insert_post_loop [v4]

[Benoît Maillard]

On Tue, 16 Sep 2025 08:44:25 GMT, Emanuel Peter <epeter at openjdk.org> wrote:

>> src/hotspot/share/opto/loopTransform.cpp line 1794:
>> 
>>> 1792:       Node* store = if_false->out(j)->isa_Store();
>>> 1793:       // We don't make changes if the memory input is in the loop body as well
>>> 1794:       if (store && !outer_loop->is_member(get_loop(get_ctrl(store->in(MemNode::Memory))))) {
>> 
>> Suggestion:
>> 
>>       if (store != nullptr && !outer_loop->is_member(get_loop(get_ctrl(store->in(MemNode::Memory))))) {
>> 
>> No implicit null or zero checks, see hotspot style guide ;)
>
> The loop nesting check looks a bit convoluted. Consider refactoring a little. Could you get rid of the `!` by swapping things around?
> `get_loop(get_ctrl(store->in(MemNode::Memory))))->is_member(outer_loop)`
> Does not look that much better either... hmm.

> No implicit null or zero checks, see hotspot style guide ;)

Missed that, thanks for the reminder!

> The loop nesting check looks a bit convoluted. Consider refactoring a little. Could you get rid of the ! by swapping things around?

I personally think it looks more intuitive with the `!`, but I agree it is a bit convoluted. I have added an intermediate variable to make it more readable.

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

PR Review Comment: https://git.openjdk.org/jdk/pull/27225#discussion_r2371622867