RFR: 8359121: C2: Region added by vectorizedMismatch intrinsic can survive as a dead node after IGVN

Marc Chevalier mchevalier at openjdk.org
Wed Jun 11 13:02:42 UTC 2025


In `bool LibraryCallKit::inline_vectorizedMismatch()` the region created at:

https://github.com/openjdk/jdk/blob/0582bd290d5a8b6344ae7ada36492cc2f33df050/src/hotspot/share/opto/library_call.cpp#L6502

may have only one input, and be a copy (that is no self-loop) and a single input. It is thus safe to remove. Yet, in the reproducer case, the node is short-circuited, but stays in the graph after IGVN.

Left, after Parsing/before IGVN; right, after IGVN:
<img src="https://github.com/user-attachments/assets/2fc0e817-ccf5-4586-81dc-ea9daef21274" width="400">  <img src="https://github.com/user-attachments/assets/6032f54e-75ba-49a1-b00e-ac8053d9f592" width="400">
On the left, the ⚠ is there because the Region doesn't have a self loop, which is expected for copies. On the right, it still doesn't have a self-loop, but IGV is also complaining the Region has no successor.

This transformation comes from `IfNode::Ideal`, that calls `IfNode::Ideal_common`, that calls `Node::remove_dead_region`,  that shortcuts a trivial Region input:
https://github.com/openjdk/jdk/blob/0582bd290d5a8b6344ae7ada36492cc2f33df050/src/hotspot/share/opto/node.cpp#L1480-L1484

Yet, the Region node is never enqueued for IGVN, so stays in the graph.  This is not nice because it gives both:
- a control node (50 Proj) has 2 successors,
- a control node (46 Region) has 0 successors.

While at the end of IGVN, we could expect the graph to be cleaned up. There are couple ways of doing, each being enough by itself:
1. explicitly record for IGVN the region node in `LibraryCallKit::inline_vectorizedMismatch`, and not hoping it would be collected by another consequence
2. change `Node::remove_dead_region` to use `set_req_X` instead of `set_req`, so that if the Region goes dead, it will be removed.
3. not introduce the region node in `LibraryCallKit::inline_vectorizedMismatch` if we are going to have only one path, and thus avoid the problem entirely.

The solution 3. is really not easy and would require quite some code restructuring for simply saving removing a node. After discussing with @chhagedorn, we concluded that the solution 1. was probably the best:
- usually, functions similar to `inline_vectorizedMismatch` call `record_for_igvn` themselves,
- unlike what I assumed at first seeing `remove_dead_region`, it's not a general problem at all: I couldn't find another case without using `inline_vectorizedMismatch` where the Region is put aside, and not entirely disconnected quickly after, but the Region is always processed in the same IGVN when `remove_dead_region` makes it dead.

And then, we find that:
<img src="https://github.com/user-attachments/assets/32f4d934-6849-43bf-915f-0710436a22c5" width="400">

This was found because it makes a check of JDK-8350864 to fail. My plan is to add this structural invariant check to the test once the flag is integrated, as for now, the test need manual inspection to see a difference. It's also not really possible to write an IR test with it: the Region node is not reachable by under (use to def traversal from Root), so the printout of the graph doesn't show the Region node, even if the `50 Proj` above is indeed printed to have 2 outputs, only the `57 If` is actually printed.

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

Commit messages:
 - Add test
 - Record region and co for IGVN in inline_vectorizedMismatch

Changes: https://git.openjdk.org/jdk/pull/25749/files
  Webrev: https://webrevs.openjdk.org/?repo=jdk&pr=25749&range=00
  Issue: https://bugs.openjdk.org/browse/JDK-8359121
  Stats: 58 lines in 2 files changed: 58 ins; 0 del; 0 mod
  Patch: https://git.openjdk.org/jdk/pull/25749.diff
  Fetch: git fetch https://git.openjdk.org/jdk.git pull/25749/head:pull/25749

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


More information about the graal-dev mailing list