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

[王超]

On Thu, 19 Aug 2021 11:20:23 GMT, Christian Hagedorn <chagedorn at openjdk.org> wrote:

> Given that we could fix this bug without a bail out of loop predication, either by my suggested fix or something else, I'd say that this check should never be taken. And if it is taken it indicates a missed range check elimination opportunity. But you're right, we cannot really prove that there are currently no other such cases. I would therefore suggest to either turn your bailout code into an assertion check or keep your bailout but add an `assert(false)` to catch a missed case in the future to further investigate. If we will at some point face a non-fixable case we can still remove the assertion code again. But of course, all of this only applies if we can fix the current bug without bailing out of loop predication.

Thank you very much! I have make the bailing out code into an assertion, and include your suggested fix in this patch.

The first thing I worry about is: change the control of load will lead to wrong code sequence of different nodes. But during the tests, I found no problems. For two different loads, if a `volatile` load `l1` is before another normal load `l2`, the later load `l2` will have a memory dependency on the `acquire memory bar` node of  the `volatile` load `l1`, so the final code sequence will be guaranteed. For two normal loads, the disorder has no problem. So I can not find a scenario which leads to the wrong code generation of your suggested fix, please remind me if you find something, and I will keep on testing.



   

I have run tests with your suggested fix, and find no problem.

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

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