1berry/openjdk
1
0
Fork 0
Code Issues Packages Projects Releases Wiki Activity

8231055: C2: arraycopy with same non escaping src and dest but different positions causes wrong execution

Browse Source 
Reviewed-by: thartmann, vlivanov
This commit is contained in:
Roland Westrelin 2019-09-17 15:58:54 +02:00
parent 12c278c3e3
commit e2803ed999
2 changed files with 63 additions and 18 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

43
src/hotspot/share/opto/macro.cpp
View File

@ -357,19 +357,38 @@ Node* PhaseMacroExpand::make_arraycopy_load(ArrayCopyNode* ac, intptr_t offset,
if (ac->modifies(offset, offset, &_igvn, true)) { if (ac->modifies(offset, offset, &_igvn, true)) {
assert(ac->in(ArrayCopyNode::Dest) == alloc->result_cast(), "arraycopy destination should be allocation's result"); assert(ac->in(ArrayCopyNode::Dest) == alloc->result_cast(), "arraycopy destination should be allocation's result");
uint shift = exact_log2(type2aelembytes(bt)); uint shift = exact_log2(type2aelembytes(bt));
Node* diff = _igvn.transform(new SubINode(ac->in(ArrayCopyNode::SrcPos), ac->in(ArrayCopyNode::DestPos))); Node* src_pos = ac->in(ArrayCopyNode::SrcPos);
#ifdef _LP64 Node* dest_pos = ac->in(ArrayCopyNode::DestPos);
diff = _igvn.transform(new ConvI2LNode(diff)); const TypeInt* src_pos_t = _igvn.type(src_pos)->is_int();
#endif const TypeInt* dest_pos_t = _igvn.type(dest_pos)->is_int();
diff = _igvn.transform(new LShiftXNode(diff, intcon(shift)));
Node* off = _igvn.transform(new AddXNode(MakeConX(offset), diff)); Node* adr = NULL;
Node* base = ac->in(ArrayCopyNode::Src); const TypePtr* adr_type = NULL;
Node* adr = _igvn.transform(new AddPNode(base, base, off)); if (src_pos_t->is_con() && dest_pos_t->is_con()) {
const TypePtr* adr_type = _igvn.type(base)->is_ptr()->add_offset(offset); intptr_t off = ((src_pos_t->get_con() - dest_pos_t->get_con()) << shift) + offset;
if (ac->in(ArrayCopyNode::Src) == ac->in(ArrayCopyNode::Dest)) { Node* base = ac->in(ArrayCopyNode::Src);
// Don't emit a new load from src if src == dst but try to get the value from memory instead adr = _igvn.transform(new AddPNode(base, base, MakeConX(off)));
return value_from_mem(ac->in(TypeFunc::Memory), ctl, ft, ftype, adr_type->isa_oopptr(), alloc); adr_type = _igvn.type(base)->is_ptr()->add_offset(off);
if (ac->in(ArrayCopyNode::Src) == ac->in(ArrayCopyNode::Dest)) {
// Don't emit a new load from src if src == dst but try to get the value from memory instead
return value_from_mem(ac->in(TypeFunc::Memory), ctl, ft, ftype, adr_type->isa_oopptr(), alloc);
}
} else {
Node* diff = _igvn.transform(new SubINode(ac->in(ArrayCopyNode::SrcPos), ac->in(ArrayCopyNode::DestPos)));
#ifdef _LP64
diff = _igvn.transform(new ConvI2LNode(diff));
#endif
diff = _igvn.transform(new LShiftXNode(diff, intcon(shift)));
Node* off = _igvn.transform(new AddXNode(MakeConX(offset), diff));
Node* base = ac->in(ArrayCopyNode::Src);
adr = _igvn.transform(new AddPNode(base, base, off));
adr_type = _igvn.type(base)->is_ptr()->add_offset(Type::OffsetBot);
if (ac->in(ArrayCopyNode::Src) == ac->in(ArrayCopyNode::Dest)) {
// Non constant offset in the array: we can't statically
// determine the value
return NULL;
}
} }
res = LoadNode::make(_igvn, ctl, mem, adr, adr_type, type, bt, MemNode::unordered, LoadNode::UnknownControl); res = LoadNode::make(_igvn, ctl, mem, adr, adr_type, type, bt, MemNode::unordered, LoadNode::UnknownControl);
} }

38
test/hotspot/jtreg/compiler/escapeAnalysis/TestSelfArrayCopy.java
View File

@ -23,7 +23,7 @@
/* /*
* @test * @test
* @bug 8229016 * @bug 8229016 8231055
* @summary Test correct elimination of array allocation with arraycopy to itself. * @summary Test correct elimination of array allocation with arraycopy to itself.
* @library /test/lib * @library /test/lib
* @run main/othervm -Xbatch -XX:CompileCommand=compileonly,compiler.escapeAnalysis.TestSelfArrayCopy::test * @run main/othervm -Xbatch -XX:CompileCommand=compileonly,compiler.escapeAnalysis.TestSelfArrayCopy::test
@ -39,7 +39,7 @@ public class TestSelfArrayCopy {
private static final int rI1 = Utils.getRandomInstance().nextInt(); private static final int rI1 = Utils.getRandomInstance().nextInt();
private static final int rI2 = Utils.getRandomInstance().nextInt(); private static final int rI2 = Utils.getRandomInstance().nextInt();
private static int test() { private static int test1() {
// Non-escaping allocation // Non-escaping allocation
Integer[] array = {rI1, rI2}; Integer[] array = {rI1, rI2};
// Arraycopy with src == dst // Arraycopy with src == dst
@ -51,14 +51,40 @@ public class TestSelfArrayCopy {
return array[0] + array[1]; return array[0] + array[1];
} }
private static int test2() {
// Non-escaping allocation
Integer[] array = {rI1, rI2};
// Arraycopy with src == dst
System.arraycopy(array, 0, array, 1, 1);
if (b) {
// Uncommon trap
System.out.println(array[0]);
}
return array[0] + array[1];
}
public static void main(String[] args) { public static void main(String[] args) {
int expected = rI1 + rI2; int expected1 = rI1 + rI2;
int expected2 = rI1 + rI1;
// Trigger compilation // Trigger compilation
for (int i = 0; i < 20_000; ++i) { for (int i = 0; i < 20_000; ++i) {
int result = test(); int result = test1();
if (result != expected) { if (result != expected1) {
throw new RuntimeException("Incorrect result: " + result + " != " + expected); throw new RuntimeException("Incorrect result: " + result + " != " + expected1);
} }
result = test2();
if (result != expected2) {
throw new RuntimeException("Incorrect result: " + result + " != " + expected2);
}
}
b = true;
int result = test1();
if (result != expected1) {
throw new RuntimeException("Incorrect result: " + result + " != " + expected1);
}
result = test2();
if (result != expected2) {
throw new RuntimeException("Incorrect result: " + result + " != " + expected2);
} }
} }
} }