8136469: OptimizeStringConcat fails on pre-sized StringBuilder shapes
authorthartmann
Tue, 12 Jan 2016 12:55:37 +0100
changeset 35552 be1bff8945dc
parent 35551 36ef3841fb34
child 35553 fa41da206b95
child 35555 06982bbaaa22
8136469: OptimizeStringConcat fails on pre-sized StringBuilder shapes Summary: Cut off dead if branch already during parsing (GVN). Reviewed-by: shade, kvn, roland
hotspot/src/share/vm/opto/ifnode.cpp
hotspot/src/share/vm/opto/macro.cpp
hotspot/src/share/vm/opto/stringopts.cpp
--- a/hotspot/src/share/vm/opto/ifnode.cpp	Tue Jan 12 12:55:09 2016 +0100
+++ b/hotspot/src/share/vm/opto/ifnode.cpp	Tue Jan 12 12:55:37 2016 +0100
@@ -1530,13 +1530,16 @@
 Node* IfProjNode::Identity(PhaseGVN* phase) {
   // Can only optimize if cannot go the other way
   const TypeTuple *t = phase->type(in(0))->is_tuple();
-  if (t == TypeTuple::IFNEITHER ||
-      // kill dead branch first otherwise the IfNode's control will
-      // have 2 control uses (the IfNode that doesn't go away because
-      // it still has uses and this branch of the
-      // If). Node::has_special_unique_user() will cause this node to
-      // be reprocessed once the dead branch is killed.
-      (always_taken(t) && in(0)->outcnt() == 1)) {
+  if (t == TypeTuple::IFNEITHER || (always_taken(t) &&
+       // During parsing (GVN) we don't remove dead code aggressively.
+       // Cut off dead branch and let PhaseRemoveUseless take care of it.
+      (!phase->is_IterGVN() ||
+       // During IGVN, first wait for the dead branch to be killed.
+       // Otherwise, the IfNode's control will have two control uses (the IfNode
+       // that doesn't go away because it still has uses and this branch of the
+       // If) which breaks other optimizations. Node::has_special_unique_user()
+       // will cause this node to be reprocessed once the dead branch is killed.
+       in(0)->outcnt() == 1))) {
     // IfNode control
     return in(0)->in(0);
   }
--- a/hotspot/src/share/vm/opto/macro.cpp	Tue Jan 12 12:55:09 2016 +0100
+++ b/hotspot/src/share/vm/opto/macro.cpp	Tue Jan 12 12:55:37 2016 +0100
@@ -244,7 +244,7 @@
   } else {
     // G1 pre/post barriers
     assert(p2x->outcnt() <= 2, "expects 1 or 2 users: Xor and URShift nodes");
-    // It could be only one user, URShift node, in Object.clone() instrinsic
+    // It could be only one user, URShift node, in Object.clone() intrinsic
     // but the new allocation is passed to arraycopy stub and it could not
     // be scalar replaced. So we don't check the case.
 
--- a/hotspot/src/share/vm/opto/stringopts.cpp	Tue Jan 12 12:55:09 2016 +0100
+++ b/hotspot/src/share/vm/opto/stringopts.cpp	Tue Jan 12 12:55:37 2016 +0100
@@ -822,11 +822,10 @@
             }
           } else if (ctrl->is_IfTrue()) { // null checks, class checks
             iff = ctrl->in(0)->as_If();
-            assert(iff->is_If(), "must be if");
             // Verify that the other arm is an uncommon trap
             Node* otherproj = iff->proj_out(1 - ctrl->as_Proj()->_con);
             CallStaticJavaNode* call = otherproj->unique_out()->isa_CallStaticJava();
-            assert(strcmp(call->_name, "uncommon_trap") == 0, "must be uncommond trap");
+            assert(strcmp(call->_name, "uncommon_trap") == 0, "must be uncommon trap");
             ctrl = iff->in(0);
           } else {
             break;
@@ -914,6 +913,13 @@
       BoolNode* b = iff->in(1)->isa_Bool();
 
       if (b == NULL) {
+#ifndef PRODUCT
+        if (PrintOptimizeStringConcat) {
+          tty->print_cr("unexpected input to IfNode");
+          iff->in(1)->dump();
+          tty->cr();
+        }
+#endif
         fail = true;
         break;
       }