RFR: 8312980: C2: "malformed control flow" created during incremental inlining [v7]

[Roland Westrelin]

> When `testHelper()` is late inlined, `o` is recorded in the replaced
> nodes because of the cast to `B` in that method. When late inlining
> finishes, c2 tries to replace `o` with the resulting `CheckCastPP` to
> `B` for uses dominated by the call. ``test` and `testHelper2` have 2
> type checks for A: there are 2 `CheckCastPP` nodes pinned at 2
> different controls (one is dominated by the `testHelper()` call, one
> is not), a single `CmpP` that's shared by the 2 type checks (one of
> the type check is dominated, the other is not). To check if a
> replacement is legal, for each use of the node to be replaced, the
> code in `ReplacedNodes::apply()` follow uses and uses of uses until it
> finds a node that's a CFG node or is pinned. It then uses the IGVN
> heuristics to figure out if the CFG is dominated by the call or
> not. If it finds a CFG node that is not dominated, then that use is
> skipped.
> 
> What happens here is that `ReplacedNodes::apply()` checks the control
> input for each `CheckCastPP` and finds one to be dominated and the
> other not. So it performs the replacement only for the one that's
> dominated. For the shared `CmpP`, it follows uses until it reaches the
> `If` nodes of the type checks. One is dominated. The other is not. So
> it declares `CmpP` to not be dominated and skips it.
> 
> When IGVN runs next, the `CheckCastPP` that had its input replaced now
> casts a `B` to a `A` which results to top but the check that `o` is of
> type `A` that guards the type check still tests that `o` of type
> `Object` is a `B`. That check doesn't constant fold. Replaced nodes
> introduced an inconsistency. What we would have needed, is for both
> the `CmpP` and `CheckCastPP` inputs to be changed. But that wasn't
> possible because the `CmpP` is shared.
> 
> The fix I propose is a tweak to `ReplacedNodes::apply()` so it goes
> depth first over the use of the node to be replaced (let's call it N)
> and the use of its uses. When it hits a node that's pinned or a CFG,
> it checks if it is dominated. If that is the case, the chain of use
> that leads from N is cloned and the clone of the use of N gets the
> improved input. That way if a node on that path is shared and used in
> some CFG path that's not dominated, it is unaffected. We then let IGVN
> cleans up extraneous clones.

Roland Westrelin has updated the pull request incrementally with one additional commit since the last revision:

  Update src/hotspot/share/opto/replacednodes.cpp
  
  Co-authored-by: Emanuel Peter <emanuel.peter at oracle.com>

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

Changes:
  - all: https://git.openjdk.org/jdk/pull/15905/files
  - new: https://git.openjdk.org/jdk/pull/15905/files/609aa398..eb56721a

Webrevs:
 - full: https://webrevs.openjdk.org/?repo=jdk&pr=15905&range=06
 - incr: https://webrevs.openjdk.org/?repo=jdk&pr=15905&range=05-06

  Stats: 1 line in 1 file changed: 0 ins; 0 del; 1 mod
  Patch: https://git.openjdk.org/jdk/pull/15905.diff
  Fetch: git fetch https://git.openjdk.org/jdk.git pull/15905/head:pull/15905

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