8349479: C2: when a Type node becomes dead, make CFG path that uses it unreachable

rwestrel · rwestrel · commit bcac42aabce5 · 2025-04-10T07:04:15.000Z
Reviewed-by: chagedorn, vlivanov
diff --git a/src/hotspot/cpu/aarch64/aarch64.ad b/src/hotspot/cpu/aarch64/aarch64.ad
@@ -16297,7 +16297,8 @@ instruct ShouldNotReachHere() %{
 
   ins_encode %{
     if (is_reachable()) {
-      __ stop(_halt_reason);
+      const char* str = __ code_string(_halt_reason);
+      __ stop(str);
     }
   %}
 
diff --git a/src/hotspot/cpu/arm/arm.ad b/src/hotspot/cpu/arm/arm.ad
@@ -8992,7 +8992,8 @@ instruct ShouldNotReachHere( )
   format %{ "ShouldNotReachHere" %}
   ins_encode %{
     if (is_reachable()) {
-      __ stop(_halt_reason);
+      const char* str = __ code_string(_halt_reason);
+      __ stop(str);
     }
   %}
   ins_pipe(tail_call);
diff --git a/src/hotspot/cpu/ppc/ppc.ad b/src/hotspot/cpu/ppc/ppc.ad
@@ -14699,7 +14699,8 @@ instruct ShouldNotReachHere() %{
   format %{ "ShouldNotReachHere" %}
   ins_encode %{
     if (is_reachable()) {
-      __ stop(_halt_reason);
+      const char* str = __ code_string(_halt_reason);
+      __ stop(str);
     }
   %}
   ins_pipe(pipe_class_default);
diff --git a/src/hotspot/cpu/riscv/riscv.ad b/src/hotspot/cpu/riscv/riscv.ad
@@ -10893,7 +10893,8 @@ instruct ShouldNotReachHere() %{
 
   ins_encode %{
     if (is_reachable()) {
-      __ stop(_halt_reason);
+      const char* str = __ code_string(_halt_reason);
+      __ stop(str);
     }
   %}
 
diff --git a/src/hotspot/cpu/s390/s390.ad b/src/hotspot/cpu/s390/s390.ad
@@ -10080,7 +10080,8 @@ instruct ShouldNotReachHere() %{
   format %{ "ILLTRAP; ShouldNotReachHere" %}
   ins_encode %{
     if (is_reachable()) {
-      __ stop(_halt_reason);
+      const char* str = __ code_string(_halt_reason);
+      __ stop(str);
     }
   %}
   ins_pipe(pipe_class_dummy);
diff --git a/src/hotspot/cpu/x86/x86.ad b/src/hotspot/cpu/x86/x86.ad
@@ -2874,7 +2874,8 @@ instruct ShouldNotReachHere() %{
   format %{ "stop\t# ShouldNotReachHere" %}
   ins_encode %{
     if (is_reachable()) {
-      __ stop(_halt_reason);
+      const char* str = __ code_string(_halt_reason);
+      __ stop(str);
     }
   %}
   ins_pipe(pipe_slow);
diff --git a/src/hotspot/share/nmt/nativeCallStackPrinter.cpp b/src/hotspot/share/nmt/nativeCallStackPrinter.cpp
@@ -44,10 +44,7 @@ void NativeCallStackPrinter::print_stack(const NativeCallStack* stack) const {
     if (created) {
       stringStream ss(4 * K);
       stack->print_frame(&ss, pc);
-      const size_t len = ss.size();
-      char* store = NEW_ARENA_ARRAY(&_text_storage, char, len + 1);
-      memcpy(store, ss.base(), len + 1);
-      (*cached_frame_text) = store;
+      (*cached_frame_text) = ss.as_string(&_text_storage);
     }
     _out->print_raw_cr(*cached_frame_text);
   }
diff --git a/src/hotspot/share/opto/c2_globals.hpp b/src/hotspot/share/opto/c2_globals.hpp
@@ -820,6 +820,12 @@
   product(bool, UseStoreStoreForCtor, true, DIAGNOSTIC,                     \
           "Use StoreStore barrier instead of Release barrier at the end "   \
           "of constructors")                                                \
+                                                                            \
+  develop(bool, KillPathsReachableByDeadTypeNode, true,                     \
+          "When a Type node becomes top, make paths where the node is "     \
+          "used dead by replacing them with a Halt node. Turning this off " \
+          "could corrupt the graph in rare cases and should be used with "  \
+          "care.")                                                          \
 
 // end of C2_FLAGS
 
diff --git a/src/hotspot/share/opto/castnode.cpp b/src/hotspot/share/opto/castnode.cpp
@@ -96,8 +96,14 @@ const Type* ConstraintCastNode::Value(PhaseGVN* phase) const {
 //------------------------------Ideal------------------------------------------
 // Return a node which is more "ideal" than the current node.  Strip out
 // control copies
-Node *ConstraintCastNode::Ideal(PhaseGVN *phase, bool can_reshape) {
-  return (in(0) && remove_dead_region(phase, can_reshape)) ? this : nullptr;
+Node* ConstraintCastNode::Ideal(PhaseGVN* phase, bool can_reshape) {
+  if (in(0) != nullptr && remove_dead_region(phase, can_reshape)) {
+    return this;
+  }
+  if (in(1) != nullptr && phase->type(in(1)) != Type::TOP) {
+    return TypeNode::Ideal(phase, can_reshape);
+  }
+  return nullptr;
 }
 
 uint ConstraintCastNode::hash() const {
diff --git a/src/hotspot/share/opto/connode.hpp b/src/hotspot/share/opto/connode.hpp
@@ -46,6 +46,10 @@ class ConNode : public TypeNode {
   virtual const RegMask &out_RegMask() const { return RegMask::Empty; }
   virtual const RegMask &in_RegMask(uint) const { return RegMask::Empty; }
 
+  virtual Node* Ideal(PhaseGVN* phase, bool can_reshape) {
+    return Node::Ideal(phase, can_reshape);
+  }
+
   // Polymorphic factory method:
   static ConNode* make(const Type *t);
 };
diff --git a/src/hotspot/share/opto/convertnode.cpp b/src/hotspot/share/opto/convertnode.cpp
@@ -689,7 +689,14 @@ bool Compile::push_thru_add(PhaseGVN* phase, Node* z, const TypeInteger* tz, con
 
 
 //------------------------------Ideal------------------------------------------
-Node *ConvI2LNode::Ideal(PhaseGVN *phase, bool can_reshape) {
+Node* ConvI2LNode::Ideal(PhaseGVN* phase, bool can_reshape) {
+  if (in(1) != nullptr && phase->type(in(1)) != Type::TOP) {
+    Node* progress = TypeNode::Ideal(phase, can_reshape);
+    if (progress != nullptr) {
+      return progress;
+    }
+  }
+
   const TypeLong* this_type = this->type()->is_long();
   if (can_reshape && !phase->C->post_loop_opts_phase()) {
     // makes sure we run ::Value to potentially remove type assertion after loop opts
@@ -791,7 +798,14 @@ const Type* ConvL2INode::Value(PhaseGVN* phase) const {
 //------------------------------Ideal------------------------------------------
 // Return a node which is more "ideal" than the current node.
 // Blow off prior masking to int
-Node *ConvL2INode::Ideal(PhaseGVN *phase, bool can_reshape) {
+Node* ConvL2INode::Ideal(PhaseGVN* phase, bool can_reshape) {
+  if (in(1) != nullptr && phase->type(in(1)) != Type::TOP) {
+    Node* progress = TypeNode::Ideal(phase, can_reshape);
+    if (progress != nullptr) {
+      return progress;
+    }
+  }
+
   Node *andl = in(1);
   uint andl_op = andl->Opcode();
   if( andl_op == Op_AndL ) {
diff --git a/src/hotspot/share/opto/movenode.cpp b/src/hotspot/share/opto/movenode.cpp
@@ -90,6 +90,10 @@ Node *CMoveNode::Ideal(PhaseGVN *phase, bool can_reshape) {
       phase->type(in(IfTrue))    == Type::TOP) {
     return nullptr;
   }
+  Node* progress = TypeNode::Ideal(phase, can_reshape);
+  if (progress != nullptr) {
+    return progress;
+  }
 
   // Check for Min/Max patterns. This is called before constants are pushed to the right input, as that transform can
   // make BoolTests non-canonical.
diff --git a/src/hotspot/share/opto/node.cpp b/src/hotspot/share/opto/node.cpp
@@ -3076,3 +3076,78 @@ const Type* TypeNode::Value(PhaseGVN* phase) const { return _type; }
 uint TypeNode::ideal_reg() const {
   return _type->ideal_reg();
 }
+
+void TypeNode::make_path_dead(PhaseIterGVN* igvn, PhaseIdealLoop* loop, Node* ctrl_use, uint j, const char* phase_str) {
+  Node* c = ctrl_use->in(j);
+  if (igvn->type(c) != Type::TOP) {
+    igvn->replace_input_of(ctrl_use, j, igvn->C->top());
+    create_halt_path(igvn, c, loop, phase_str);
+  }
+}
+
+// This Type node is dead. It could be because the type that it captures and the type of the node computed from its
+// inputs do not intersect anymore. That node has some uses along some control flow paths. Those control flow paths must
+// be unreachable as using a dead value makes no sense. For the Type node to capture a narrowed down type, some control
+// flow construct must guard the Type node (an If node usually). When the Type node becomes dead, the guard usually
+// constant folds and the control flow that leads to the Type node becomes unreachable. There are cases where that
+// doesn't happen, however. They are handled here by following uses of the Type node until a CFG or a Phi to find dead
+// paths. The dead paths are then replaced by a Halt node.
+void TypeNode::make_paths_from_here_dead(PhaseIterGVN* igvn, PhaseIdealLoop* loop, const char* phase_str) {
+  Unique_Node_List wq;
+  wq.push(this);
+  for (uint i = 0; i < wq.size(); ++i) {
+    Node* n = wq.at(i);
+    for (DUIterator_Fast kmax, k = n->fast_outs(kmax); k < kmax; k++) {
+      Node* u = n->fast_out(k);
+      if (u->is_CFG()) {
+        assert(!u->is_Region(), "Can't reach a Region without going through a Phi");
+        make_path_dead(igvn, loop, u, 0, phase_str);
+      } else if (u->is_Phi()) {
+        Node* r = u->in(0);
+        assert(r->is_Region() || r->is_top(), "unexpected Phi's control");
+        if (r->is_Region()) {
+          for (uint j = 1; j < u->req(); ++j) {
+            if (u->in(j) == n) {
+              make_path_dead(igvn, loop, r, j, phase_str);
+            }
+          }
+        }
+      } else {
+        wq.push(u);
+      }
+    }
+  }
+}
+
+void TypeNode::create_halt_path(PhaseIterGVN* igvn, Node* c, PhaseIdealLoop* loop, const char* phase_str) const {
+  Node* frame = new ParmNode(igvn->C->start(), TypeFunc::FramePtr);
+  if (loop == nullptr) {
+    igvn->register_new_node_with_optimizer(frame);
+  } else {
+    loop->register_new_node(frame, igvn->C->start());
+  }
+
+  stringStream ss;
+  ss.print("dead path discovered by TypeNode during %s", phase_str);
+
+  Node* halt = new HaltNode(c, frame, ss.as_string(igvn->C->comp_arena()));
+  if (loop == nullptr) {
+    igvn->register_new_node_with_optimizer(halt);
+  } else {
+    loop->register_control(halt, loop->ltree_root(), c);
+  }
+  igvn->add_input_to(igvn->C->root(), halt);
+}
+
+Node* TypeNode::Ideal(PhaseGVN* phase, bool can_reshape) {
+  if (KillPathsReachableByDeadTypeNode && can_reshape && Value(phase) == Type::TOP) {
+    PhaseIterGVN* igvn = phase->is_IterGVN();
+    Node* top = igvn->C->top();
+    ResourceMark rm;
+    make_paths_from_here_dead(igvn, nullptr, "igvn");
+    return top;
+  }
+
+  return Node::Ideal(phase, can_reshape);
+}
+
diff --git a/src/hotspot/share/opto/node.hpp b/src/hotspot/share/opto/node.hpp
@@ -155,6 +155,7 @@ class ParsePredicateNode;
 class PCTableNode;
 class PhaseCCP;
 class PhaseGVN;
+class PhaseIdealLoop;
 class PhaseIterGVN;
 class PhaseRegAlloc;
 class PhaseTransform;
@@ -2044,12 +2045,17 @@ class TypeNode : public Node {
     init_class_id(Class_Type);
   }
   virtual const Type* Value(PhaseGVN* phase) const;
+  virtual Node* Ideal(PhaseGVN* phase, bool can_reshape);
   virtual const Type *bottom_type() const;
   virtual       uint  ideal_reg() const;
+
+  void make_path_dead(PhaseIterGVN* igvn, PhaseIdealLoop* loop, Node* ctrl_use, uint j, const char* phase_str);
 #ifndef PRODUCT
   virtual void dump_spec(outputStream *st) const;
   virtual void dump_compact_spec(outputStream *st) const;
 #endif
+  void make_paths_from_here_dead(PhaseIterGVN* igvn, PhaseIdealLoop* loop, const char* phase_str);
+  void create_halt_path(PhaseIterGVN* igvn, Node* c, PhaseIdealLoop* loop, const char* phase_str) const;
 };
 
 #include "opto/opcodes.hpp"
diff --git a/src/hotspot/share/opto/phaseX.cpp b/src/hotspot/share/opto/phaseX.cpp
@@ -1835,6 +1835,11 @@ void PhaseCCP::analyze() {
       set_type(n, new_type);
       push_child_nodes_to_worklist(worklist, n);
     }
+    if (KillPathsReachableByDeadTypeNode && n->is_Type() && new_type == Type::TOP) {
+      // Keep track of Type nodes to kill CFG paths that use Type
+      // nodes that become dead.
+      _maybe_top_type_nodes.push(n);
+    }
   }
   DEBUG_ONLY(verify_analyze(worklist_verify);)
 }
@@ -2065,6 +2070,18 @@ Node *PhaseCCP::transform( Node *n ) {
   // track all visited nodes, so that we can remove the complement
   Unique_Node_List useful;
 
+  if (KillPathsReachableByDeadTypeNode) {
+    for (uint i = 0; i < _maybe_top_type_nodes.size(); ++i) {
+      Node* type_node = _maybe_top_type_nodes.at(i);
+      if (type(type_node) == Type::TOP) {
+        ResourceMark rm;
+        type_node->as_Type()->make_paths_from_here_dead(this, nullptr, "ccp");
+      }
+    }
+  } else {
+    assert(_maybe_top_type_nodes.size() == 0, "we don't need type nodes");
+  }
+
   // Initialize the traversal.
   // This CCP pass may prove that no exit test for a loop ever succeeds (i.e. the loop is infinite). In that case,
   // the logic below doesn't follow any path from Root to the loop body: there's at least one such path but it's proven
diff --git a/src/hotspot/share/opto/phaseX.hpp b/src/hotspot/share/opto/phaseX.hpp
@@ -608,6 +608,7 @@ class PhaseIterGVN : public PhaseGVN {
 // Should be replaced with combined CCP & GVN someday.
 class PhaseCCP : public PhaseIterGVN {
   Unique_Node_List _root_and_safepoints;
+  Unique_Node_List _maybe_top_type_nodes;
   // Non-recursive.  Use analysis to transform single Node.
   virtual Node* transform_once(Node* n);
 
diff --git a/src/hotspot/share/utilities/ostream.cpp b/src/hotspot/share/utilities/ostream.cpp
@@ -437,6 +437,13 @@ char* stringStream::as_string(bool c_heap) const {
   return copy;
 }
 
+char* stringStream::as_string(Arena* arena) const {
+  char* copy = NEW_ARENA_ARRAY(arena, char, _written + 1);
+  ::memcpy(copy, _buffer, _written);
+  copy[_written] = '\0';  // terminating null
+  return copy;
+}
+
 stringStream::~stringStream() {
   if (!_is_fixed && _buffer != _small_buffer) {
     FREE_C_HEAP_ARRAY(char, _buffer);
diff --git a/src/hotspot/share/utilities/ostream.hpp b/src/hotspot/share/utilities/ostream.hpp
@@ -270,6 +270,7 @@ class stringStream : public outputStream {
   bool is_empty() const { return _buffer[0] == '\0'; }
   // Copy to a resource, or C-heap, array as requested
   char* as_string(bool c_heap = false) const;
+  char* as_string(Arena* arena) const;
 };
 
 class fileStream : public outputStream {
diff --git a/test/hotspot/jtreg/compiler/c2/TestGuardOfCastIIDoesntFold.java b/test/hotspot/jtreg/compiler/c2/TestGuardOfCastIIDoesntFold.java
@@ -0,0 +1,73 @@
+/*
+ * Copyright (c) 2025, Red Hat, Inc. All rights reserved.
+ * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
+ *
+ * This code is free software; you can redistribute it and/or modify it
+ * under the terms of the GNU General Public License version 2 only, as
+ * published by the Free Software Foundation.
+ *
+ * This code is distributed in the hope that it will be useful, but WITHOUT
+ * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
+ * FITNESS FOR A PARTICULAR PURPOSE.  See the GNU General Public License
+ * version 2 for more details (a copy is included in the LICENSE file that
+ * accompanied this code).
+ *
+ * You should have received a copy of the GNU General Public License version
+ * 2 along with this work; if not, write to the Free Software Foundation,
+ * Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA.
+ *
+ * Please contact Oracle, 500 Oracle Parkway, Redwood Shores, CA 94065 USA
+ * or visit www.oracle.com if you need additional information or have any
+ * questions.
+ */
+
+/*
+ * @test
+ * @bug 8349479
+ * @summary C2: when a Type node becomes dead, make CFG path that uses it unreachable
+ * @run main/othervm -XX:-TieredCompilation -XX:-BackgroundCompilation -XX:-UseOnStackReplacement
+ *                   -XX:CompileCommand=dontinline,TestGuardOfCastIIDoesntFold::notInlined
+ *                   TestGuardOfCastIIDoesntFold
+ * @run main TestGuardOfCastIIDoesntFold
+ */
+
+public class TestGuardOfCastIIDoesntFold {
+    private static volatile int volatileField;
+
+    public static void main(String[] args) {
+        for (int i = 0; i < 20_000; i++) {
+            test1(1, 0, 0, false);
+            helper1(1, 0, 0, true);
+            helper2(0, 0);
+        }
+    }
+
+    private static int test1(int i, int j, int k, boolean flag) {
+        int l;
+        for (l = 0; l < 10; l++) {
+        }
+        j = helper2(j, l);
+        return helper1(i, j, k, flag);
+    }
+
+    private static int helper2(int j, int l) {
+        if (l == 10) {
+            j = Integer.MAX_VALUE-1;
+        }
+        return j;
+    }
+
+    private static int helper1(int i, int j, int k, boolean flag) {
+        if (flag) {
+            k = Integer.max(k, -2);
+            int[] array = new int[i + k];
+            notInlined(array);
+            return array[j];
+        }
+        volatileField = 42;
+        return volatileField;
+    }
+
+    private static void notInlined(int[] array) {
+    }
+}
diff --git a/test/hotspot/jtreg/compiler/predicates/assertion/TestAssertionPredicates.java b/test/hotspot/jtreg/compiler/predicates/assertion/TestAssertionPredicates.java

Original file line number	Diff line number	Diff line change
`@@ -16297,7 +16297,8 @@ instruct ShouldNotReachHere() %{`
`16297`	`16297`
`16298`	`16298`	`ins_encode %{`
`16299`	`16299`	`if (is_reachable()) {`
`16300`		`- __ stop(_halt_reason);`
	`16300`	`+ const char* str = __ code_string(_halt_reason);`
	`16301`	`+ __ stop(str);`
`16301`	`16302`	`}`
`16302`	`16303`	`%}`
`16303`	`16304`
Original file line number	Diff line number	Diff line change
`@@ -10893,7 +10893,8 @@ instruct ShouldNotReachHere() %{`
`10893`	`10893`
`10894`	`10894`	`ins_encode %{`
`10895`	`10895`	`if (is_reachable()) {`
`10896`		`- __ stop(_halt_reason);`
	`10896`	`+ const char* str = __ code_string(_halt_reason);`
	`10897`	`+ __ stop(str);`
`10897`	`10898`	`}`
`10898`	`10899`	`%}`
`10899`	`10900`
Original file line number	Diff line number	Diff line change
`@@ -44,10 +44,7 @@ void NativeCallStackPrinter::print_stack(const NativeCallStack* stack) const {`
`44`	`44`	`if (created) {`
`45`	`45`	`stringStream ss(4 * K);`
`46`	`46`	`stack->print_frame(&ss, pc);`
`47`		`- const size_t len = ss.size();`
`48`		`- char* store = NEW_ARENA_ARRAY(&_text_storage, char, len + 1);`
`49`		`- memcpy(store, ss.base(), len + 1);`
`50`		`- (*cached_frame_text) = store;`
	`47`	`+ (*cached_frame_text) = ss.as_string(&_text_storage);`
`51`	`48`	`}`
`52`	`49`	`_out->print_raw_cr(*cached_frame_text);`
`53`	`50`	`}`