8306042: C2: failed: Missed optimization opportunity in PhaseCCP (adding LShift->Cast->Add notification)

Reviewed-by: thartmann, chagedorn, kvn

src/hotspot/share/opto/castnode.hpp

@@ -69,6 +69,28 @@ public:
   static Node* make_cast_for_type(Node* c, Node* in, const Type* type, DependencyType dependency);
 
   Node* optimize_integer_cast(PhaseGVN* phase, BasicType bt);
+
+  // Visit all non-cast uses of the node, bypassing ConstraintCasts.
+  // Pattern: this (-> ConstraintCast)* -> non_cast
+  // In other words: find all non_cast nodes such that
+  // non_cast->uncast() == this.
+  template <typename Callback>
+  static void visit_uncasted_uses(const Node* n, Callback callback) {
+    ResourceMark rm;
+    Unique_Node_List internals;
+    internals.push((Node*)n); // start traversal
+    for (uint j = 0; j < internals.size(); ++j) {
+      Node* internal = internals.at(j); // for every internal
+      for (DUIterator_Fast kmax, k = internal->fast_outs(kmax); k < kmax; k++) {
+        Node* internal_use = internal->fast_out(k);
+        if (internal_use->is_ConstraintCast()) {
+          internals.push(internal_use); // traverse this cast also
+        } else {
+          callback(internal_use);
+        }
+      }
+    }
+  }
 };
 
 //------------------------------CastIINode-------------------------------------

src/hotspot/share/opto/phaseX.cpp

@@ -1760,31 +1760,16 @@ void PhaseIterGVN::add_users_to_worklist( Node *n ) {
     }
 
     // If changed Cast input, notify down for Phi and Sub - both do "uncast"
+    // Patterns:
+    // ConstraintCast+ -> Sub
+    // ConstraintCast+ -> Phi
     if (use->is_ConstraintCast()) {
-      for (DUIterator_Fast i2max, i2 = use->fast_outs(i2max); i2 < i2max; i2++) {
-        Node* u = use->fast_out(i2);
-        if (u->is_Phi() || u->is_Sub()) {
-          // Phi (.., CastII, ..) or Sub(Cast(x), x)
-          _worklist.push(u);
-        } else if (u->is_ConstraintCast()) {
-          // Follow cast-chains down to Sub: Sub( CastII(CastII(x)), x)
-          // This case is quite rare. Let's BFS-traverse casts, to find Subs:
-          ResourceMark rm;
-          Unique_Node_List casts;
-          casts.push(u); // start traversal
-          for (uint j = 0; j < casts.size(); ++j) {
-            Node* cast = casts.at(j); // for every cast
-            for (DUIterator_Fast kmax, k = cast->fast_outs(kmax); k < kmax; k++) {
-              Node* cast_use = cast->fast_out(k);
-              if (cast_use->is_ConstraintCast()) {
-                casts.push(cast_use); // traverse this cast also
-              } else if (cast_use->is_Sub()) {
-                _worklist.push(cast_use); // found Sub
-              }
-            }
-          }
+      auto push_phi_or_sub_uses_to_worklist = [&](Node* n){
+        if (n->is_Phi() || n->is_Sub()) {
+          _worklist.push(n);
         }
-      }
+      };
+      ConstraintCastNode::visit_uncasted_uses(use, push_phi_or_sub_uses_to_worklist);
     }
     // If changed LShift inputs, check RShift users for useless sign-ext
     if( use_op == Op_LShiftI ) {
@@ -1965,10 +1950,15 @@ void PhaseCCP::analyze() {
     _types.map(i, Type::TOP);
   }
 
+  // CCP worklist is placed on a local arena, so that we can allow ResourceMarks on "Compile::current()->resource_arena()".
+  // We also do not want to put the worklist on "Compile::current()->comp_arena()", as that one only gets de-allocated after
+  // Compile is over. The local arena gets de-allocated at the end of its scope.
+  ResourceArea local_arena(mtCompiler);
+  Unique_Node_List worklist(&local_arena);
+  DEBUG_ONLY(Unique_Node_List worklist_verify(&local_arena);)
+
   // Push root onto worklist
-  Unique_Node_List worklist;
   worklist.push(C->root());
-  DEBUG_ONLY(Unique_Node_List worklist_verify;)
 
   assert(_root_and_safepoints.size() == 0, "must be empty (unused)");
   _root_and_safepoints.push(C->root());
@@ -2149,17 +2139,18 @@ void PhaseCCP::push_load_barrier(Unique_Node_List& worklist, const BarrierSetC2*
 
 // AndI/L::Value() optimizes patterns similar to (v << 2) & 3 to zero if they are bitwise disjoint.
 // Add the AndI/L nodes back to the worklist to re-apply Value() in case the shift value changed.
+// Pattern: parent -> LShift (use) -> ConstraintCast* -> And
 void PhaseCCP::push_and(Unique_Node_List& worklist, const Node* parent, const Node* use) const {
   uint use_op = use->Opcode();
   if ((use_op == Op_LShiftI || use_op == Op_LShiftL)
       && use->in(2) == parent) { // is shift value (right-hand side of LShift)
-    for (DUIterator_Fast imax, i = use->fast_outs(imax); i < imax; i++) {
-      Node* and_node = use->fast_out(i);
-      uint and_node_op = and_node->Opcode();
-      if (and_node_op == Op_AndI || and_node_op == Op_AndL) {
-        push_if_not_bottom_type(worklist, and_node);
+    auto push_and_uses_to_worklist = [&](Node* n){
+      uint opc = n->Opcode();
+      if (opc == Op_AndI || opc == Op_AndL) {
+        push_if_not_bottom_type(worklist, n);
       }
-    }
+    };
+    ConstraintCastNode::visit_uncasted_uses(use, push_and_uses_to_worklist);
   }
 }
 

test/hotspot/jtreg/compiler/ccp/TestShiftCastAndNotification.java

@@ -0,0 +1,62 @@
+/*
+ * Copyright (c) 2023, Oracle and/or its affiliates. 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 8306042
+ * @summary CCP missed optimization opportunity. Due to missing notification through Casts.
+ * @library /test/lib
+ * @run main/othervm -Xcomp -XX:-TieredCompilation
+ *                   -XX:CompileOnly=compiler.ccp.TestShiftCastAndNotification::test
+ *                   compiler.ccp.TestShiftCastAndNotification
+ */
+
+package compiler.ccp;
+
+public class TestShiftCastAndNotification {
+    static int N;
+    static int iArrFld[] = new int[1];
+    static int test() {
+        int x = 1;
+        int sval = 4;
+        long useless[] = new long[N];
+        for (double d1 = 63; d1 > 2; d1 -= 2) {
+            for (double d2 = 3; 1 < d2; d2--) {
+                x <<= sval; // The LShiftI
+            }
+            // CastII probably somewhere in the loop structure
+            x &= 3; // The AndI
+            for (int i = 1; i < 3; i++) {
+                try {
+                    x = iArrFld[0];
+                    sval = 0;
+                } catch (ArithmeticException a_e) {
+                }
+            }
+        }
+        return x;
+    }
+    public static void main(String[] args) {
+        test();
+    }
+}