# HG changeset patch # User roland # Date 1568728734 -7200 # Node ID b311681bc3f9ed12ae964a91d81ec9403211fb35 # Parent e4ce29f6094e6edfa001d1372301528c396ff94d 8231055: C2: arraycopy with same non escaping src and dest but different positions causes wrong execution Reviewed-by: thartmann, vlivanov diff -r e4ce29f6094e -r b311681bc3f9 src/hotspot/share/opto/macro.cpp --- a/src/hotspot/share/opto/macro.cpp Wed Sep 25 17:54:21 2019 +0800 +++ b/src/hotspot/share/opto/macro.cpp Tue Sep 17 15:58:54 2019 +0200 @@ -357,19 +357,38 @@ if (ac->modifies(offset, offset, &_igvn, true)) { assert(ac->in(ArrayCopyNode::Dest) == alloc->result_cast(), "arraycopy destination should be allocation's result"); uint shift = exact_log2(type2aelembytes(bt)); - 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* src_pos = ac->in(ArrayCopyNode::SrcPos); + Node* dest_pos = ac->in(ArrayCopyNode::DestPos); + const TypeInt* src_pos_t = _igvn.type(src_pos)->is_int(); + const TypeInt* dest_pos_t = _igvn.type(dest_pos)->is_int(); - Node* off = _igvn.transform(new AddXNode(MakeConX(offset), diff)); - Node* base = ac->in(ArrayCopyNode::Src); - Node* adr = _igvn.transform(new AddPNode(base, base, off)); - const TypePtr* adr_type = _igvn.type(base)->is_ptr()->add_offset(offset); - 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); + Node* adr = NULL; + const TypePtr* adr_type = NULL; + if (src_pos_t->is_con() && dest_pos_t->is_con()) { + intptr_t off = ((src_pos_t->get_con() - dest_pos_t->get_con()) << shift) + offset; + Node* base = ac->in(ArrayCopyNode::Src); + adr = _igvn.transform(new AddPNode(base, base, MakeConX(off))); + 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); } diff -r e4ce29f6094e -r b311681bc3f9 test/hotspot/jtreg/compiler/escapeAnalysis/TestSelfArrayCopy.java --- a/test/hotspot/jtreg/compiler/escapeAnalysis/TestSelfArrayCopy.java Wed Sep 25 17:54:21 2019 +0800 +++ b/test/hotspot/jtreg/compiler/escapeAnalysis/TestSelfArrayCopy.java Tue Sep 17 15:58:54 2019 +0200 @@ -23,7 +23,7 @@ /* * @test - * @bug 8229016 + * @bug 8229016 8231055 * @summary Test correct elimination of array allocation with arraycopy to itself. * @library /test/lib * @run main/othervm -Xbatch -XX:CompileCommand=compileonly,compiler.escapeAnalysis.TestSelfArrayCopy::test @@ -39,7 +39,7 @@ private static final int rI1 = Utils.getRandomInstance().nextInt(); private static final int rI2 = Utils.getRandomInstance().nextInt(); - private static int test() { + private static int test1() { // Non-escaping allocation Integer[] array = {rI1, rI2}; // Arraycopy with src == dst @@ -51,14 +51,40 @@ 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) { - int expected = rI1 + rI2; + int expected1 = rI1 + rI2; + int expected2 = rI1 + rI1; // Trigger compilation for (int i = 0; i < 20_000; ++i) { - int result = test(); - if (result != expected) { - throw new RuntimeException("Incorrect result: " + result + " != " + expected); + 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); } } + 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); + } } }