8086046: escape analysis generates incorrect code as of B67
Summary: load bypasses arraycopy that sets the value after the ArrayCopyNode is expanded
Reviewed-by: kvn
--- a/hotspot/src/share/vm/opto/callnode.cpp Wed Jun 03 08:23:09 2015 +0000
+++ b/hotspot/src/share/vm/opto/callnode.cpp Fri Jun 12 14:10:17 2015 +0200
@@ -724,6 +724,26 @@
//
bool CallNode::may_modify(const TypeOopPtr *t_oop, PhaseTransform *phase) {
assert((t_oop != NULL), "sanity");
+ if (is_call_to_arraycopystub()) {
+ const TypeTuple* args = _tf->domain();
+ Node* dest = NULL;
+ // Stubs that can be called once an ArrayCopyNode is expanded have
+ // different signatures. Look for the second pointer argument,
+ // that is the destination of the copy.
+ for (uint i = TypeFunc::Parms, j = 0; i < args->cnt(); i++) {
+ if (args->field_at(i)->isa_ptr()) {
+ j++;
+ if (j == 2) {
+ dest = in(i);
+ break;
+ }
+ }
+ }
+ if (!dest->is_top() && may_modify_arraycopy_helper(phase->type(dest)->is_oopptr(), t_oop, phase)) {
+ return true;
+ }
+ return false;
+ }
if (t_oop->is_known_instance()) {
// The instance_id is set only for scalar-replaceable allocations which
// are not passed as arguments according to Escape Analysis.
@@ -909,6 +929,12 @@
return SafePointNode::Ideal(phase, can_reshape);
}
+bool CallNode::is_call_to_arraycopystub() const {
+ if (_name != NULL && strstr(_name, "arraycopy") != 0) {
+ return true;
+ }
+ return false;
+}
//=============================================================================
uint CallJavaNode::size_of() const { return sizeof(*this); }
@@ -1007,14 +1033,6 @@
//=============================================================================
-bool CallLeafNode::is_call_to_arraycopystub() const {
- if (_name != NULL && strstr(_name, "arraycopy") != 0) {
- return true;
- }
- return false;
-}
-
-
#ifndef PRODUCT
void CallLeafNode::dump_spec(outputStream *st) const {
st->print("# ");
@@ -1930,26 +1948,3 @@
return true;
}
-bool CallLeafNode::may_modify(const TypeOopPtr *t_oop, PhaseTransform *phase) {
- if (is_call_to_arraycopystub()) {
- const TypeTuple* args = _tf->domain();
- Node* dest = NULL;
- // Stubs that can be called once an ArrayCopyNode is expanded have
- // different signatures. Look for the second pointer argument,
- // that is the destination of the copy.
- for (uint i = TypeFunc::Parms, j = 0; i < args->cnt(); i++) {
- if (args->field_at(i)->isa_ptr()) {
- j++;
- if (j == 2) {
- dest = in(i);
- break;
- }
- }
- }
- if (!dest->is_top() && may_modify_arraycopy_helper(phase->type(dest)->is_oopptr(), t_oop, phase)) {
- return true;
- }
- return false;
- }
- return CallNode::may_modify(t_oop, phase);
-}
--- a/hotspot/src/share/vm/opto/callnode.hpp Wed Jun 03 08:23:09 2015 +0000
+++ b/hotspot/src/share/vm/opto/callnode.hpp Fri Jun 12 14:10:17 2015 +0200
@@ -565,13 +565,15 @@
address _entry_point; // Address of method being called
float _cnt; // Estimate of number of times called
CallGenerator* _generator; // corresponding CallGenerator for some late inline calls
+ const char *_name; // Printable name, if _method is NULL
CallNode(const TypeFunc* tf, address addr, const TypePtr* adr_type)
: SafePointNode(tf->domain()->cnt(), NULL, adr_type),
_tf(tf),
_entry_point(addr),
_cnt(COUNT_UNKNOWN),
- _generator(NULL)
+ _generator(NULL),
+ _name(NULL)
{
init_class_id(Class_Call);
}
@@ -630,6 +632,8 @@
virtual uint match_edge(uint idx) const;
+ bool is_call_to_arraycopystub() const;
+
#ifndef PRODUCT
virtual void dump_req(outputStream *st = tty) const;
virtual void dump_spec(outputStream *st) const;
@@ -683,7 +687,7 @@
virtual uint size_of() const; // Size is bigger
public:
CallStaticJavaNode(Compile* C, const TypeFunc* tf, address addr, ciMethod* method, int bci)
- : CallJavaNode(tf, addr, method, bci), _name(NULL) {
+ : CallJavaNode(tf, addr, method, bci) {
init_class_id(Class_CallStaticJava);
if (C->eliminate_boxing() && (method != NULL) && method->is_boxing_method()) {
init_flags(Flag_is_macro);
@@ -694,14 +698,14 @@
}
CallStaticJavaNode(const TypeFunc* tf, address addr, const char* name, int bci,
const TypePtr* adr_type)
- : CallJavaNode(tf, addr, NULL, bci), _name(name) {
+ : CallJavaNode(tf, addr, NULL, bci) {
init_class_id(Class_CallStaticJava);
// This node calls a runtime stub, which often has narrow memory effects.
_adr_type = adr_type;
_is_scalar_replaceable = false;
_is_non_escaping = false;
+ _name = name;
}
- const char *_name; // Runtime wrapper name
// Result of Escape Analysis
bool _is_scalar_replaceable;
@@ -754,13 +758,12 @@
public:
CallRuntimeNode(const TypeFunc* tf, address addr, const char* name,
const TypePtr* adr_type)
- : CallNode(tf, addr, adr_type),
- _name(name)
+ : CallNode(tf, addr, adr_type)
{
init_class_id(Class_CallRuntime);
+ _name = name;
}
- const char *_name; // Printable name, if _method is NULL
virtual int Opcode() const;
virtual void calling_convention( BasicType* sig_bt, VMRegPair *parm_regs, uint argcnt ) const;
@@ -785,8 +788,6 @@
#ifndef PRODUCT
virtual void dump_spec(outputStream *st) const;
#endif
- bool is_call_to_arraycopystub() const;
- virtual bool may_modify(const TypeOopPtr *t_oop, PhaseTransform *phase);
};
//------------------------------CallLeafNoFPNode-------------------------------
--- a/hotspot/src/share/vm/opto/memnode.cpp Wed Jun 03 08:23:09 2015 +0000
+++ b/hotspot/src/share/vm/opto/memnode.cpp Fri Jun 12 14:10:17 2015 +0200
@@ -108,11 +108,10 @@
#endif
-static bool membar_for_arraycopy_helper(const TypeOopPtr *t_oop, MergeMemNode* mm, PhaseTransform *phase) {
- if (mm->memory_at(Compile::AliasIdxRaw)->is_Proj()) {
- Node* n = mm->memory_at(Compile::AliasIdxRaw)->in(0);
- if ((n->is_ArrayCopy() && n->as_ArrayCopy()->may_modify(t_oop, phase)) ||
- (n->is_CallLeaf() && n->as_CallLeaf()->may_modify(t_oop, phase))) {
+static bool membar_for_arraycopy_helper(const TypeOopPtr *t_oop, Node* n, PhaseTransform *phase) {
+ if (n->is_Proj()) {
+ n = n->in(0);
+ if (n->is_Call() && n->as_Call()->may_modify(t_oop, phase)) {
return true;
}
}
@@ -121,16 +120,22 @@
static bool membar_for_arraycopy(const TypeOopPtr *t_oop, MemBarNode* mb, PhaseTransform *phase) {
Node* mem = mb->in(TypeFunc::Memory);
+
if (mem->is_MergeMem()) {
- return membar_for_arraycopy_helper(t_oop, mem->as_MergeMem(), phase);
- } else if (mem->is_Phi()) {
- // after macro expansion of an ArrayCopyNode we may have a Phi
- for (uint i = 1; i < mem->req(); i++) {
- if (mem->in(i) != NULL && mem->in(i)->is_MergeMem() && membar_for_arraycopy_helper(t_oop, mem->in(i)->as_MergeMem(), phase)) {
- return true;
+ Node* n = mem->as_MergeMem()->memory_at(Compile::AliasIdxRaw);
+ if (membar_for_arraycopy_helper(t_oop, n, phase)) {
+ return true;
+ } else if (n->is_Phi()) {
+ for (uint i = 1; i < n->req(); i++) {
+ if (n->in(i) != NULL) {
+ if (membar_for_arraycopy_helper(t_oop, n->in(i), phase)) {
+ return true;
+ }
+ }
}
}
}
+
return false;
}
--- /dev/null Thu Jan 01 00:00:00 1970 +0000
+++ b/hotspot/test/compiler/arraycopy/TestLoadBypassArrayCopy.java Fri Jun 12 14:10:17 2015 +0200
@@ -0,0 +1,71 @@
+/*
+ * Copyright (c) 2015, 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 8086046
+ * @summary load bypasses arraycopy that sets the value after the ArrayCopyNode is expanded
+ * @run main/othervm -XX:-BackgroundCompilation -XX:-UseOnStackReplacement -XX:CompileCommand=dontinline,TestLoadBypassArrayCopy::test_helper -XX:-TieredCompilation TestLoadBypassArrayCopy
+ *
+ */
+
+public class TestLoadBypassArrayCopy {
+
+ static long i;
+ static boolean test_helper() {
+ i++;
+ if ((i%10) == 0) {
+ return false;
+ }
+ return true;
+ }
+
+ static int test(int[] src, int len, boolean flag) {
+ int[] dest = new int[10];
+ int res = 0;
+ while (test_helper()) {
+ System.arraycopy(src, 0, dest, 0, len);
+ // predicate moved out of loop so control of following
+ // load is not the ArrayCopyNode. Otherwise, if the memory
+ // of the load is changed and the control is set to the
+ // ArrayCopyNode the graph is unschedulable and the test
+ // doesn't fail.
+ if (flag) {
+ }
+ // The memory of this load shouldn't bypass the arraycopy
+ res = dest[0];
+ }
+ return res;
+ }
+
+ static public void main(String[] args) {
+ int[] src = new int[10];
+ src[0] = 0x42;
+ for (int i = 0; i < 20000; i++) {
+ int res = test(src, 10, false);
+ if (res != src[0]) {
+ throw new RuntimeException("test failed");
+ }
+ }
+ }
+}