6973329: C2 with Zero based COOP produces code with broken anti-dependency on x86
authorkvn
Wed, 11 Aug 2010 10:48:20 -0700
changeset 6188 95ea4d66089a
parent 6187 4fa7845f7c14
child 6190 2ce445f727d8
child 6268 9e96dde307e9
6973329: C2 with Zero based COOP produces code with broken anti-dependency on x86 Summary: Recompile without subsuming loads if RA try to clone a node with anti_dependence. Reviewed-by: never
hotspot/src/share/vm/includeDB_compiler2
hotspot/src/share/vm/opto/lcm.cpp
hotspot/src/share/vm/opto/reg_split.cpp
hotspot/test/compiler/6973329/Test.java
--- a/hotspot/src/share/vm/includeDB_compiler2	Wed Aug 11 05:51:21 2010 -0700
+++ b/hotspot/src/share/vm/includeDB_compiler2	Wed Aug 11 10:48:20 2010 -0700
@@ -911,6 +911,7 @@
 reg_split.cpp                           addnode.hpp
 reg_split.cpp                           allocation.inline.hpp
 reg_split.cpp                           callnode.hpp
+reg_split.cpp                           c2compiler.hpp
 reg_split.cpp                           cfgnode.hpp
 reg_split.cpp                           chaitin.hpp
 reg_split.cpp                           loopnode.hpp
--- a/hotspot/src/share/vm/opto/lcm.cpp	Wed Aug 11 05:51:21 2010 -0700
+++ b/hotspot/src/share/vm/opto/lcm.cpp	Wed Aug 11 10:48:20 2010 -0700
@@ -113,7 +113,8 @@
     if( !m->is_Mach() ) continue;
     MachNode *mach = m->as_Mach();
     was_store = false;
-    switch( mach->ideal_Opcode() ) {
+    int iop = mach->ideal_Opcode();
+    switch( iop ) {
     case Op_LoadB:
     case Op_LoadUS:
     case Op_LoadD:
@@ -155,6 +156,12 @@
     default:                    // Also check for embedded loads
       if( !mach->needs_anti_dependence_check() )
         continue;               // Not an memory op; skip it
+      if( must_clone[iop] ) {
+        // Do not move nodes which produce flags because
+        // RA will try to clone it to place near branch and
+        // it will cause recompilation, see clone_node().
+        continue;
+      }
       {
         // Check that value is used in memory address in
         // instructions with embedded load (CmpP val1,(val2+off)).
@@ -957,6 +964,8 @@
     Block *sb = _succs[i];
     // Clone the entire area; ignoring the edge fixup for now.
     for( uint j = end; j > beg; j-- ) {
+      // It is safe here to clone a node with anti_dependence
+      // since clones dominate on each path.
       Node *clone = _nodes[j-1]->clone();
       sb->_nodes.insert( 1, clone );
       bbs.map(clone->_idx,sb);
--- a/hotspot/src/share/vm/opto/reg_split.cpp	Wed Aug 11 05:51:21 2010 -0700
+++ b/hotspot/src/share/vm/opto/reg_split.cpp	Wed Aug 11 10:48:20 2010 -0700
@@ -271,6 +271,32 @@
   return maxlrg;
 }
 
+//------------------------------clone_node----------------------------
+// Clone node with anti dependence check.
+Node* clone_node(Node* def, Block *b, Compile* C) {
+  if (def->needs_anti_dependence_check()) {
+#ifdef ASSERT
+    if (Verbose) {
+      tty->print_cr("RA attempts to clone node with anti_dependence:");
+      def->dump(-1); tty->cr();
+      tty->print_cr("into block:");
+      b->dump();
+    }
+#endif
+    if (C->subsume_loads() == true && !C->failing()) {
+      // Retry with subsume_loads == false
+      // If this is the first failure, the sentinel string will "stick"
+      // to the Compile object, and the C2Compiler will see it and retry.
+      C->record_failure(C2Compiler::retry_no_subsuming_loads());
+    } else {
+      // Bailout without retry
+      C->record_method_not_compilable("RA Split failed: attempt to clone node with anti_dependence");
+    }
+    return 0;
+  }
+  return def->clone();
+}
+
 //------------------------------split_Rematerialize----------------------------
 // Clone a local copy of the def.
 Node *PhaseChaitin::split_Rematerialize( Node *def, Block *b, uint insidx, uint &maxlrg, GrowableArray<uint> splits, int slidx, uint *lrg2reach, Node **Reachblock, bool walkThru ) {
@@ -298,8 +324,8 @@
     }
   }
 
-  Node *spill = def->clone();
-  if (C->check_node_count(NodeLimitFudgeFactor, out_of_nodes)) {
+  Node *spill = clone_node(def, b, C);
+  if (spill == NULL || C->check_node_count(NodeLimitFudgeFactor, out_of_nodes)) {
     // Check when generating nodes
     return 0;
   }
@@ -834,13 +860,13 @@
               // The effect of this clone is to drop the node out of the block,
               // so that the allocator does not see it anymore, and therefore
               // does not attempt to assign it a register.
-              def = def->clone();
+              def = clone_node(def, b, C);
+              if (def == NULL || C->check_node_count(NodeLimitFudgeFactor, out_of_nodes)) {
+                return 0;
+              }
               _names.extend(def->_idx,0);
               _cfg._bbs.map(def->_idx,b);
               n->set_req(inpidx, def);
-              if (C->check_node_count(NodeLimitFudgeFactor, out_of_nodes)) {
-                return 0;
-              }
               continue;
             }
 
--- /dev/null	Thu Jan 01 00:00:00 1970 +0000
+++ b/hotspot/test/compiler/6973329/Test.java	Wed Aug 11 10:48:20 2010 -0700
@@ -0,0 +1,63 @@
+/*
+ * Copyright (c) 2010, 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 6973329
+ * @summary C2 with Zero based COOP produces code with broken anti-dependency on x86
+ *
+ * @run main/othervm -Xbatch -Xcomp -XX:CompileOnly=Test Test
+ */
+
+class A {
+  A next;
+  int n;
+
+  public int get_n() {
+    return n+1;
+  }
+}
+public class Test {
+
+  A a;
+
+  void test (A new_next) {
+    A prev_next = a.next;
+    a.next = new_next;
+    if (prev_next == null) {
+      a.n = a.get_n();
+    }
+  }
+
+  public static void main(String args[]) {
+    Test t = new Test();
+    t.a = new A();
+    t.a.n = 1;
+    t.test(new A());
+    if (t.a.n != 2) {
+      System.out.println("Wrong value: " + t.a.n + " expected: 2");
+      System.exit(97);
+    }
+  }
+}
+