RFR: 8307084: C2: Vectorized drain loop is not executed for some small trip counts [v4]

[Fei Gao]

On Thu, 15 Jan 2026 15:04:30 GMT, Fei Gao <fgao at openjdk.org> wrote:

>> Also: how sure are you that the backedge `main_phi->in(LoopNode::LoopBackControl)` is the same as the the value after main `iv_after_main`?
>> What if we did some split-through-phi action at some point? Example:
>> 
>> x = ...
>> LOOP:
>>     x = op(x);
>>     // x now serves as exit value and backedge value
>>     exit check;
>>     goto LOOP;
>> 
>> If we split `op` through the LOOP Phi, we get:
>> 
>> x = ...
>> x = op(x);
>> LOOP:
>>     // the exit value is the phi
>>     exit check;
>>     x = op(x);
>>     // x after op is the backedge
>>     goto LOOP;
>> 
>> 
>> I'm not sure this currently ever happens, but what if it did?
>
>> * If we cover all phis: we should probably change the naming of the variables to indicate that it is any `phi` and not just the `iv`.
>> 
>> What do you think?
> 
> Yes, we’re looking at all variables that change as the loop iterates, not just the trip counter. How about renaming it to `main_exit_value` or `value_after_main_loop`?

> Also: how sure are you that the backedge `main_phi->in(LoopNode::LoopBackControl)` is the same as the the value after main `iv_after_main`? What if we did some split-through-phi action at some point? Example:
> 
> ```
> x = ...
> LOOP:
>     x = op(x);
>     // x now serves as exit value and backedge value
>     exit check;
>     goto LOOP;
> ```
> 
> If we split `op` through the LOOP Phi, we get:
> 
> ```
> x = ...
> x = op(x);
> LOOP:
>     // the exit value is the phi
>     exit check;
>     x = op(x);
>     // x after op is the backedge
>     goto LOOP;
> ```
> 
> I'm not sure this currently ever happens, but what if it did?

Great question—and yes, that’s exactly why this logic is a bit more involved.

If I understand correctly, in the second scenario—after a split-through-phi—the value we need after the main loop is not the original x directly. Instead, we also need to apply an additional op(x) so that the resulting value can serve as the input to the drain loop.


x = ...
y = op(x);
MAIN_LOOP:
    // the exit value is the phi
    exit check;
    y = op(x);
    // x after op is the backedge
    goto MAIN_LOOP;

MAIN_EXIT:
    y = op(x);  // the newly created op(x) outside the main-loop body

DRAIN_LOOP:
    // the exit value is the phi
    exit check;
    y = op(x);
    // x after op is the backedge
    goto DRAIN_LOOP;



Let's walk through the code step by step.


  Node* drain_input = nullptr;
  Node* iv_after_main = main_phi->in(LoopNode::LoopBackControl);

At this point, we do **not** assume that `iv_after_main` is already the correct “after-main” value.

  if (get_ctrl(iv_after_main) != main_backedge_ctrl) {
    drain_input = find_merge_phi_for_vectorized_drain(iv_after_main, main_merge_region);
  }

We first check whether `main_phi->in(LoopNode::LoopBackControl)` is pinned in the backedge block.

1. If `get_ctrl(iv_after_main) != main_backedge_ctrl`, then `main_phi->in(LoopNode::LoopBackControl)` is **not** in the backedge block. It should be **either in the main loop body or in the main exit block.**

- If it is still in the main loop body, we won’t be able to find a valid merge phi via its uses, so **`drain_input` remains nullptr**.

- If it is already in the main exit block, we can find an existing valid merge phi via its uses—this corresponds to the first scenario you described, **`drain_input` won't be nullptr**.

2. If `get_ctrl(iv_after_main) == main_backedge_ctrl`, then the `main_phi->in(LoopNode::LoopBackControl)` is pinned in the backedge block. In this case, especially in the presence of transformations like split-through-phi (your second scenario), it clearly cannot be treated as the correct “after-main” value, and **`drain_input` will also remain nullptr**.


  if (drain_input == nullptr) {
    iv_after_main = clone_up_backedge_goo(main_backedge_ctrl, main_merge_region->in(2),
                                          iv_after_main, visited, clones);
    drain_input = PhiNode::make(main_merge_region, iv_after_main);
  }

When `drain_input` is still nullptr, we enter this path and call `clone_up_backedge_goo()`. This helper recursively walks and clones the input chain of `main_phi->in(LoopNode::LoopBackControl)` as needed, producing a version of the value that is legal at the main-exit control (`main_merge_region->in(2)`), which is the newly created op(x) outside the main-loop body.

At this point, the returned `iv_after_main` represents the correct value to use after the main loop, even if split-through-phi or similar transformations have occurred. We can then safely create (or later reuse) a merge phi based on this value.

Does this address your concern?

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

PR Review Comment: https://git.openjdk.org/jdk/pull/22629#discussion_r2695018811