RFR: 8374725: C2: assert(x_ctrl == get_late_ctrl_with_anti_dep(x->as_Load(), early_ctrl, x_ctrl)) failed: anti-dependences were already checked

[Damon Fenacci]

On Wed, 14 Jan 2026 13:45:09 GMT, Roland Westrelin <roland at openjdk.org> wrote:

> `PhaseIdealLoop::dom_lca_for_get_late_ctrl_internal()` caches
> intermediate results in `_dom_lca_tags` when the late control is
> computed by `PhaseIdealLoop::get_late_ctrl()` for a node `n`: the code
> iterates over all uses of `n` potentially calling
> `PhaseIdealLoop::dom_lca_for_get_late_ctrl_internal()` multiple
> times. `_dom_lca_tags` is used to cache data that is specific to the
> lca computation for `n`. `_dom_lca_tags` is set to a tag that depends
> on `n` to mark the cached data as only valid during the lca
> computation for `n`.
> 
> `PhaseIdealLoop::try_sink_out_of_loop()` checks that all uses of a
> node are out of loop with
> `PhaseIdealLoop::ctrl_of_all_uses_out_of_loop()` which also needs to
> consider anti-dependences for `Load`s and also calls
> `PhaseIdealLoop::dom_lca_for_get_late_ctrl_internal()` through
> `PhaseIdealLoop::get_late_ctrl_with_anti_dep()`: this computes the
> late control for a node and one particular out of loop
> use. `_dom_lca_tags` values computed by an earlier
> `PhaseIdealLoop::get_late_ctrl()` should be ignored (because it
> computes the late control for a node and all its uses). To address
> that issue, the tag that's used by
> `PhaseIdealLoop::dom_lca_for_get_late_ctrl_internal()` is made
> different on each call from
> `PhaseIdealLoop::ctrl_of_all_uses_out_of_loop()` by incrementing
> `_dom_lca_tags_round`.
> 
> The issue here is that one `Load` node is input to a `Phi` twice. So
> the `Phi` is considered twice as a use of the node along 2 different
> paths. `PhaseIdealLoop::get_late_ctrl_with_anti_dep()` is called twice
> from `PhaseIdealLoop::ctrl_of_all_uses_out_of_loop()` but
> `_dom_lca_tags_round` is not incremented between the 2
> calls. `PhaseIdealLoop::dom_lca_for_get_late_ctrl_internal()` when
> called for the second `Phi` input uses incorrect cached data which, in
> turn, causes an incorrect computation.
> 
> The fix I propose is to make sure `_dom_lca_tags_round` is incremented
> for every call to `PhaseIdealLoop::get_late_ctrl_with_anti_dep()`.

Looks good to me too. Thanks a lot @rwestrel! (I just added a couple of very marginal nits)

src/hotspot/share/opto/loopopts.cpp line 1959:

> 1957:   if (n->is_Load()) {
> 1958:     // We can't reuse tags in PhaseIdealLoop::dom_lca_for_get_late_ctrl_internal() so make sure calls to
> 1959:     // get_late_ctrl_with_anti_dep() use their own tag

just to emphasise this change we could write "make sure each call... its own tag"

test/hotspot/jtreg/compiler/loopopts/TestSinkingLoadInputOfPhi.java line 40:

> 38:   static int iFld2 = 10;
> 39:   static void test() {
> 40:     long l1;

It is a reduced fuzzer test but I happened to notice that `l1` doesn't seem to be used...

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

Marked as reviewed by dfenacci (Committer).

PR Review: https://git.openjdk.org/jdk/pull/29231#pullrequestreview-3670996932
PR Review Comment: https://git.openjdk.org/jdk/pull/29231#discussion_r2698610998
PR Review Comment: https://git.openjdk.org/jdk/pull/29231#discussion_r2698618802