RFR: 8315920: C2: "control input must dominate current control" assert failure [v3]

Roland Westrelin roland at openjdk.org
Mon Sep 25 13:46:12 UTC 2023


> The assert fires during range check eliminationin code that was added
> recently. It catches a bug that's been going unnoticed. The `array[0]`
> load of the test case ends up on the exit of the loop above it. After
> pre/main/post loops are created for this loop (the inner loop is
> optimized out), the load is at a region merging control from the just
> created pre/main/post loops but its control (as returned by
> `PhaseIdealLoop::get_ctrl()`) is not updated by pre/main/post loops
> creation code so it's still the exit projection of one of the
> loops. This causes the assert to fire (the load has bad control) when
> RC runs next in the same loop opts pass.
> 
> The current logic for pre/main/post loops creation does update the
> control of nodes on an exit projection but only if the node is
> referenced from some other node in the loop body.
> 
> I think a similar bug with a node assigned control at the exit but not
> pinned is likely to exist. So I doubt simply going over nodes pinned
> at exits during pre/main/post and updating their control is good
> enough. What I propose instead is to `lazy_replace()` the projection
> by the region so that, for all nodes that are assigned the exit as
> control, `get_ctrl()` will then return the region. It's not entirely
> straightforward to implement because the `lazy_replace()` mechanism
> assumes the control being replaced is dead which is not the case
> here. That's why in the patch the exit projection is cloned so the
> existing one is dead and can be used in `lazy_replace()`. This also
> makes the calls to `sink_use()` redundant.

Roland Westrelin has updated the pull request with a new target base due to a merge or a rebase. The incremental webrev excludes the unrelated changes brought in by the merge/rebase. The pull request contains seven additional commits since the last revision:

 - comments
 - Merge branch 'master' into JDK-8315920
 - Update src/hotspot/share/opto/loopopts.cpp
   
   Co-authored-by: Christian Hagedorn <christian.hagedorn at oracle.com>
 - Update src/hotspot/share/opto/loopopts.cpp
   
   Co-authored-by: Tobias Hartmann <tobias.hartmann at oracle.com>
 - Update src/hotspot/share/opto/loopopts.cpp
   
   Co-authored-by: Tobias Hartmann <tobias.hartmann at oracle.com>
 - Update src/hotspot/share/opto/loopopts.cpp
   
   Co-authored-by: Tobias Hartmann <tobias.hartmann at oracle.com>
 - fix & test

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

Changes:
  - all: https://git.openjdk.org/jdk/pull/15720/files
  - new: https://git.openjdk.org/jdk/pull/15720/files/a0845847..321a7244

Webrevs:
 - full: https://webrevs.openjdk.org/?repo=jdk&pr=15720&range=02
 - incr: https://webrevs.openjdk.org/?repo=jdk&pr=15720&range=01-02

  Stats: 59218 lines in 1737 files changed: 15049 ins; 12221 del; 31948 mod
  Patch: https://git.openjdk.org/jdk/pull/15720.diff
  Fetch: git fetch https://git.openjdk.org/jdk.git pull/15720/head:pull/15720

PR: https://git.openjdk.org/jdk/pull/15720


More information about the hotspot-compiler-dev mailing list