8147645: get_ctrl_no_update() code is wrong
authorroland
Wed, 03 Feb 2016 10:58:50 +0100
changeset 35759 85d3873f87b4
parent 35757 0eeda480b926
child 35760 00d17cc49814
8147645: get_ctrl_no_update() code is wrong Summary: Array.fill intrinsification code doesn't mark replaced control as dead Reviewed-by: kvn
hotspot/src/share/vm/opto/loopTransform.cpp
hotspot/src/share/vm/opto/loopnode.cpp
hotspot/src/share/vm/opto/loopnode.hpp
hotspot/src/share/vm/opto/split_if.cpp
hotspot/test/compiler/loopopts/TestArraysFillDeadControl.java
--- a/hotspot/src/share/vm/opto/loopTransform.cpp	Wed Feb 03 09:09:56 2016 +0100
+++ b/hotspot/src/share/vm/opto/loopTransform.cpp	Wed Feb 03 10:58:50 2016 +0100
@@ -3048,7 +3048,7 @@
   // state of the loop.  It's safe in this case to replace it with the
   // result_mem.
   _igvn.replace_node(store->in(MemNode::Memory), result_mem);
-  _igvn.replace_node(exit, result_ctrl);
+  lazy_replace(exit, result_ctrl);
   _igvn.replace_node(store, result_mem);
   // Any uses the increment outside of the loop become the loop limit.
   _igvn.replace_node(head->incr(), head->limit());
--- a/hotspot/src/share/vm/opto/loopnode.cpp	Wed Feb 03 09:09:56 2016 +0100
+++ b/hotspot/src/share/vm/opto/loopnode.cpp	Wed Feb 03 10:58:50 2016 +0100
@@ -755,8 +755,8 @@
     set_loop(iff2, get_loop(iffalse));
 
     // Lazy update of 'get_ctrl' mechanism.
-    lazy_replace_proj( iffalse, iff2 );
-    lazy_replace_proj( iftrue,  ift2 );
+    lazy_replace(iffalse, iff2);
+    lazy_replace(iftrue,  ift2);
 
     // Swap names
     iffalse = iff2;
--- a/hotspot/src/share/vm/opto/loopnode.hpp	Wed Feb 03 09:09:56 2016 +0100
+++ b/hotspot/src/share/vm/opto/loopnode.hpp	Wed Feb 03 10:58:50 2016 +0100
@@ -693,13 +693,18 @@
   }
 
 private:
-  Node *get_ctrl_no_update( Node *i ) const {
+  Node *get_ctrl_no_update_helper(Node *i) const {
+    assert(has_ctrl(i), "should be control, not loop");
+    return (Node*)(((intptr_t)_nodes[i->_idx]) & ~1);
+  }
+
+  Node *get_ctrl_no_update(Node *i) const {
     assert( has_ctrl(i), "" );
-    Node *n = (Node*)(((intptr_t)_nodes[i->_idx]) & ~1);
+    Node *n = get_ctrl_no_update_helper(i);
     if (!n->in(0)) {
       // Skip dead CFG nodes
       do {
-        n = (Node*)(((intptr_t)_nodes[n->_idx]) & ~1);
+        n = get_ctrl_no_update_helper(n);
       } while (!n->in(0));
       n = find_non_split_ctrl(n);
     }
@@ -721,22 +726,15 @@
   // from old_node to new_node to support the lazy update.  Reference
   // replaces loop reference, since that is not needed for dead node.
 public:
-  void lazy_update( Node *old_node, Node *new_node ) {
-    assert( old_node != new_node, "no cycles please" );
-    //old_node->set_req( 1, new_node /*NO DU INFO*/ );
-    // Nodes always have DU info now, so re-use the side array slot
-    // for this node to provide the forwarding pointer.
-    _nodes.map( old_node->_idx, (Node*)((intptr_t)new_node + 1) );
+  void lazy_update(Node *old_node, Node *new_node) {
+    assert(old_node != new_node, "no cycles please");
+    // Re-use the side array slot for this node to provide the
+    // forwarding pointer.
+    _nodes.map(old_node->_idx, (Node*)((intptr_t)new_node + 1));
   }
-  void lazy_replace( Node *old_node, Node *new_node ) {
-    _igvn.replace_node( old_node, new_node );
-    lazy_update( old_node, new_node );
-  }
-  void lazy_replace_proj( Node *old_node, Node *new_node ) {
-    assert( old_node->req() == 1, "use this for Projs" );
-    _igvn.hash_delete(old_node); // Must hash-delete before hacking edges
-    old_node->add_req( NULL );
-    lazy_replace( old_node, new_node );
+  void lazy_replace(Node *old_node, Node *new_node) {
+    _igvn.replace_node(old_node, new_node);
+    lazy_update(old_node, new_node);
   }
 
 private:
--- a/hotspot/src/share/vm/opto/split_if.cpp	Wed Feb 03 09:09:56 2016 +0100
+++ b/hotspot/src/share/vm/opto/split_if.cpp	Wed Feb 03 10:58:50 2016 +0100
@@ -472,7 +472,7 @@
 
     // Replace in the graph with lazy-update mechanism
     new_iff->set_req(0, new_iff); // hook self so it does not go dead
-    lazy_replace_proj( ifp, ifpx );
+    lazy_replace(ifp, ifpx);
     new_iff->set_req(0, region);
 
     // Record bits for later xforms
--- /dev/null	Thu Jan 01 00:00:00 1970 +0000
+++ b/hotspot/test/compiler/loopopts/TestArraysFillDeadControl.java	Wed Feb 03 10:58:50 2016 +0100
@@ -0,0 +1,53 @@
+/*
+ * Copyright (c) 2016, 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 8147645
+ * @summary Array.fill intrinsification code doesn't mark replaced control as dead
+ * @run main/othervm  -XX:-TieredCompilation -XX:CompileCommand=dontinline,TestArraysFillDeadControl::dont_inline TestArraysFillDeadControl
+ *
+ */
+
+import java.util.Arrays;
+
+public class TestArraysFillDeadControl {
+
+    static void dont_inline() {
+    }
+
+    static int i = 1;
+
+    public static void main(String[] args) {
+        for (int j = 0; j < 200000; j++) {
+            int[] a = new int[2];
+            int b = i;
+
+            Arrays.fill(a, 1);
+            Arrays.fill(a, 1+b);
+
+            dont_inline();
+        }
+    }
+}