RFR: 8308504: C2: "malformed control flow" after JDK-8303466
Emanuel Peter
epeter at openjdk.org
Tue Jun 20 05:55:15 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 regression test (takes less than `1.5sec`). Tested up to tier6 and stress testing. **Running**
--------
**Details**
![example](https://github.com/openjdk/jdk/assets/32593061/92d5d801-1a41-4dfd-ab39-cd6b69527dcd)
621 CountedLoop (pre-loop)
622 Phi (tripcount, minint...7)
612 AddI (incr, int) ---> with patch type minint...4
650 CmpI / 702 Bool (cannot detect that tripcount < limit) ---> with patch it can detect it!
871 ConvL2I (loop limit, 8...maxint)
703 If (zero-trip-guard)
704 IfFalse (projection towards main-loop)
654 CastII (value of 612 AddI, if zero-trip-guard goes to main-loop, so value > limit, hence type 9...maxint)
809 CastII (remembers that tripcount cannot overflow, so has type minint...7)
(when the Opaque nodes vanish, then the two types create an empty range, and TOP starts corrupting the CFG)
-------------
Commit messages:
- notify counted loop incr
- added -XX:+UnlockDiagnosticVMOptions to test
- merge with master
- 8308504: C2: "malformed control flow" after JDK-8303466
Changes: https://git.openjdk.org/jdk/pull/14331/files
Webrev: https://webrevs.openjdk.org/?repo=jdk&pr=14331&range=00
Issue: https://bugs.openjdk.org/browse/JDK-8308504
Stats: 127 lines in 4 files changed: 105 ins; 1 del; 21 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