RFR: 8272562: C2: assert(false) failed: Bad graph detected in build_loop_late [v2]

Roland Westrelin roland at openjdk.java.net
Mon Sep 27 13:57:56 UTC 2021


> A counted loop has an array access:
> 
> (LoadI (AddP (AddP (ConvI2L (CastII (AddI (Phi ...)))))))
> 
> that is
> 
> array[iv - 1]
> 
> The Phi is the iv Phi. The ConvI2L/CastII are from a range check
> and capture type: 0..maxint-1 The loop is unrolled once, the
> LoadI is cloned.
> 
> array[iv - 1]
> array[iv]
> 
> The first LoadI is only used out of loop and is sunk with 2
> clones. One of the clones is on the IfFalse branch of a test
> for iv != 0.
> 
> (LoadI (AddP (AddP (ConvI2L (CastII (AddI (CastII ...)))))))
> 
> The second CastII pins the nodes out of the loop. The ConvI2L and
> CastII are pushed thru the AddI (for -1). As a result the ConvI2L
> has type:
> 1..maxint-1
> 
> The CastII, because it has the same input as the input for iv != 0,
> becomes 0 which is not part of 1..maxint-1. The ConvI2L and all its
> uses including the LoadI become top. The use of the LoadI is a Phi
> that is transformed into its remaining input and the graph is broken.
> 
> The root cause is that the loop body initially contains:
> 
> if (iv - 1 >=u array.length) { // range check
>   trap();
> }
> 
> if (iv == 0) {
>   // path where nodes are sunk later on
> }
> 
> And obviously if iv - 1 >= 0 then iv == 0 is always false but c2 fails
> to prove it. I tried to implement a simple fix for this issue but
> while it fixes this bug, I couldn't convince myself that it was robust
> enough.
> 
> So instead I propose following the suggestion Christian and Vladimir I.
> made in:
> 
> https://github.com/openjdk/jdk/pull/5199
> 
> that is to more generally exclude cast nodes from sinking as a
> workaround for now.
> 
> I've been looking for a more general solution to this problem and I
> have a prototype that fixes this failure but is a lot more
> complicated. I'll revisit this workaround when it's ready.

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

  conservative fix

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

Changes:
  - all: https://git.openjdk.java.net/jdk/pull/5716/files
  - new: https://git.openjdk.java.net/jdk/pull/5716/files/76f60e86..7324e96a

Webrevs:
 - full: https://webrevs.openjdk.java.net/?repo=jdk&pr=5716&range=01
 - incr: https://webrevs.openjdk.java.net/?repo=jdk&pr=5716&range=00-01

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

PR: https://git.openjdk.java.net/jdk/pull/5716


More information about the hotspot-compiler-dev mailing list