RFR: 8308504: C2: "malformed control flow" after JDK-8303466 [v2]

Emanuel Peter epeter at openjdk.org
Mon Jul 10 13:20:23 UTC 2023


> This is another case where imprecise type computation leads to corrupted control flow.
> 
> The loop incr `AddI` does never overflow at runtime, this is guaranteed by the loop limit check (we ensure that adding the incr value to the limit would not overflow). However, the type so far did often overflow, and returns type `int`. The loop  tripcount `Phi` does not have such type overflow. So we had cases where the Phi had type `minint...7`, but the AddI had type `int`, since it adds `-3`, which makes the lower bound underflow (even though the runtime value never would). If we had known that an underflow is impossible because of the limit check, then we could have had type `minint...4` for the incr AddI.
> 
> Since JDK-8303466, the type of the limit can now not overflow and is more precise. In the example, the limit is `8...maxint`. The CastII after the zero-trip-guard thus concludes that the type must be `9...maxint` (we have a decrement loop).
> 
> Since the main-loop knows that the trip-count Phi has type `minint...7`, we get an empty range (intersection of `9...maxint` and `minint...7`), and the main-loop (in this case one of the assertion predicates) start to corrode away because of TOP inputs. This creates malformed control flow, we have an if with only one projection.
> 
> **Why did this arise with JDK-8303466?** We made the loop-limit type more precise, forbidding overflow. So before that change I think the loop limit just had a type would have overfown, and returned `int`. That would have not allowed the main-loop internals to detect that it could never be reached. The main-loop would not have died at all, but it would still never have been entered.
> 
> **Solution** Since the AddI (incr) of the tripcount Phi cannot overflow/underflow, **we should also prevent the type of the incr from overflowing/underflowing**. That means that it basically has the same type as the Phi, if not even a bit better. And this means that the type of the incr used to compare against the loop-limit is consistent with the type information in the main-loop.
> 
> I also had to improve the notification in IGVN, I encountered a case where a incr node was replaced via Identity, so the new incr node needed its type to be tightened (no overflow).
> 
> **Risk** Just as JDK-8303466, this fix here makes types more precise. That can always mean that somewhere else we also need more precise types than we currently have. It is difficult to know what such bugs are still lurking for us.
> 
> **Testing** Attached one regre...

Emanuel Peter has updated the pull request incrementally with one additional commit since the last revision:

  revert bad fix

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

Changes:
  - all: https://git.openjdk.org/jdk/pull/14331/files
  - new: https://git.openjdk.org/jdk/pull/14331/files/a3e8da27..e6b1424b

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

  Stats: 73 lines in 3 files changed: 8 ins; 51 del; 14 mod
  Patch: https://git.openjdk.org/jdk/pull/14331.diff
  Fetch: git fetch https://git.openjdk.org/jdk.git pull/14331/head:pull/14331

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


More information about the hotspot-compiler-dev mailing list