RFR: 8263227: C2: inconsistent spilling due to dead nodes in exception block [v2]

Mon Apr 12 08:37:23 UTC 2021

On Thu, 8 Apr 2021 18:20:13 GMT, Vladimir Kozlov <kvn at openjdk.org> wrote:

> I am not sure that no other nodes were inserted in between users.

Isn't this enforced by local scheduling? From `PhaseCFG::select()`:

https://github.com/openjdk/jdk/blob/b1ebf82269fa85bed859ffafacd59ed000f22bd0/src/hotspot/share/opto/lcm.cpp#L477-L478

https://github.com/openjdk/jdk/blob/b1ebf82269fa85bed859ffafacd59ed000f22bd0/src/hotspot/share/opto/lcm.cpp#L516-L523

`PhaseCFG::call_catch_cleanup()` runs right after local scheduling, and it doesn't reorder the cloned instructions before reaching the dead code elimination loop, so I don't see any place where other nodes might be inserted in between projections of the same node. I checked this with an assertion in `PhaseCFG::verify()` (commits 8d96589 and 808451d), and the following tests run without assertion failures:

- hs-tier1-5 (windows-x64, linux-x64, linux-aarch64, and macosx-x64) with `+VerifyRegisterAllocator`
- hs-tier6-9 (linux-x64) with `+VerifyRegisterAllocator`
- hs-tier1-3 (windows-x64, linux-x64, linux-aarch64, and macosx-x64) with `+VerifyRegisterAllocator`, `+StressGCM`, and `+StressLCM` (5 repetitions)

Based on above analysis, code comments in `lcm.cpp`, and test results, I suggest to treat the fact that projections are scheduled next to their parent nodes as an invariant, and exploit the invariant as in this (updated) change -- but I am fine with the more defensive alternative you propose if you still prefer that.

> I also noticed wrong code stile in for() statements near your change. Please, fix them too.

Done.

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

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