Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

8242453: [lworld] C2 compilation fails with assert 'correct memory chain' #134

Closed
wants to merge 1 commit into from
Closed
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions src/hotspot/share/opto/library_call.cpp
Original file line number Diff line number Diff line change
@@ -5035,8 +5035,8 @@ bool LibraryCallKit::inline_arraycopy() {
if (has_src && has_dest && can_emit_guards) {
BasicType src_elem = top_src->klass()->as_array_klass()->element_type()->basic_type();
BasicType dest_elem = top_dest->klass()->as_array_klass()->element_type()->basic_type();
if (src_elem == T_ARRAY) src_elem = T_OBJECT;
if (dest_elem == T_ARRAY) dest_elem = T_OBJECT;
if (is_reference_type(src_elem)) src_elem = T_OBJECT;
if (is_reference_type(dest_elem)) dest_elem = T_OBJECT;

if (src_elem == dest_elem && src_elem == T_OBJECT) {
// If both arrays are object arrays then having the exact types
6 changes: 3 additions & 3 deletions src/hotspot/share/opto/macro.hpp
Original file line number Diff line number Diff line change
@@ -191,9 +191,9 @@ class PhaseMacroExpand : public Phase {
Node* src, Node* src_offset,
Node* dest, Node* dest_offset,
Node* copy_length, bool dest_uninitialized);
const TypePtr* adjust_parameters_for_vt(const TypeAryPtr* top_dest, Node*& src_offset,
Node*& dest_offset, Node*& length, BasicType& dest_elem,
Node*& dest_length);
const TypePtr* adjust_for_flat_array(const TypeAryPtr* top_dest, Node*& src_offset,
Node*& dest_offset, Node*& length, BasicType& dest_elem,
Node*& dest_length);
void expand_arraycopy_node(ArrayCopyNode *ac);

void expand_subtypecheck_node(SubTypeCheckNode *check);
10 changes: 5 additions & 5 deletions src/hotspot/share/opto/macroArrayCopy.cpp
Original file line number Diff line number Diff line change
@@ -1155,9 +1155,9 @@ void PhaseMacroExpand::generate_unchecked_arraycopy(Node** ctrl, MergeMemNode**
finish_arraycopy_call(call, ctrl, mem, adr_type);
}

const TypePtr* PhaseMacroExpand::adjust_parameters_for_vt(const TypeAryPtr* top_dest, Node*& src_offset,
Node*& dest_offset, Node*& length, BasicType& dest_elem,
Node*& dest_length) {
const TypePtr* PhaseMacroExpand::adjust_for_flat_array(const TypeAryPtr* top_dest, Node*& src_offset,
Node*& dest_offset, Node*& length, BasicType& dest_elem,
Node*& dest_length) {
assert(top_dest->klass()->is_flat_array_klass(), "inconsistent");
int elem_size = ((ciFlatArrayKlass*)top_dest->klass())->element_byte_size();
if (elem_size >= 8) {
@@ -1225,7 +1225,7 @@ void PhaseMacroExpand::expand_arraycopy_node(ArrayCopyNode *ac) {

const TypePtr* adr_type = NULL;
if (dest_elem == T_INLINE_TYPE) {
adr_type = adjust_parameters_for_vt(top_dest, src_offset, dest_offset, length, dest_elem, dest_length);
adr_type = adjust_for_flat_array(top_dest, src_offset, dest_offset, length, dest_elem, dest_length);
} else {
adr_type = dest_type->is_oopptr()->add_offset(Type::OffsetBot);
if (ac->_dest_type != TypeOopPtr::BOTTOM) {
@@ -1429,7 +1429,7 @@ void PhaseMacroExpand::expand_arraycopy_node(ArrayCopyNode *ac) {
Node* dest_length = alloc != NULL ? alloc->in(AllocateNode::ALength) : NULL;

if (dest_elem == T_INLINE_TYPE) {
adr_type = adjust_parameters_for_vt(top_dest, src_offset, dest_offset, length, dest_elem, dest_length);
adr_type = adjust_for_flat_array(top_dest, src_offset, dest_offset, length, dest_elem, dest_length);
} else if (ac->_dest_type != TypeOopPtr::BOTTOM) {
adr_type = ac->_dest_type->add_offset(Type::OffsetBot)->is_ptr();
} else {
7 changes: 6 additions & 1 deletion src/hotspot/share/opto/memnode.cpp
Original file line number Diff line number Diff line change
@@ -222,6 +222,11 @@ Node *MemNode::optimize_memory_chain(Node *mchain, const TypePtr *t_adr, Node *l
// clone the Phi with our address type
result = mphi->split_out_instance(t_adr, igvn);
} else {
if (t->isa_aryptr()) {
// In the case of a flattened inline type array, each field has its own slice.
// TODO This should be re-evaluated with JDK-8251039
t = t->is_aryptr()->with_field_offset(t_adr->is_aryptr()->field_offset().get());
}
assert(phase->C->get_alias_index(t) == phase->C->get_alias_index(t_adr), "correct memory chain");
}
}
@@ -962,7 +967,7 @@ Node* LoadNode::can_see_arraycopy_value(Node* st, PhaseGVN* phase) const {
addp->set_req(AddPNode::Address, src);

const TypeAryPtr* ary_t = phase->type(in(MemNode::Address))->isa_aryptr();
BasicType ary_elem = ary_t->klass()->as_array_klass()->element_type()->basic_type();
BasicType ary_elem = ary_t->klass()->as_array_klass()->element_type()->basic_type();
uint header = arrayOopDesc::base_offset_in_bytes(ary_elem);
uint shift = exact_log2(type2aelembytes(ary_elem));
if (ary_t->klass()->is_flat_array_klass()) {
Original file line number Diff line number Diff line change
@@ -886,12 +886,18 @@ public MyValue2.ref test29(MyValue2.ref[] src) {

@DontCompile
public void test29_verifier(boolean warmup) {
MyValue2.ref[] src = new MyValue2.ref[10];
MyValue2.ref[] src1 = new MyValue2.ref[10];
MyValue2.val[] src2 = new MyValue2.val[10];
for (int i = 0; i < 10; ++i) {
src[i] = MyValue2.createWithFieldsInline(rI, (i % 2) == 0);
src1[i] = MyValue2.createWithFieldsInline(rI, (i % 2) == 0);
src2[i] = MyValue2.createWithFieldsInline(rI, (i % 2) == 0);
}
MyValue2.ref v = test29(src1);
Asserts.assertEQ(src1[0].hash(), v.hash());
if (!warmup) {
v = test29(src2);
Asserts.assertEQ(src2[0].hash(), v.hash());
}
MyValue2.ref v = test29(src);
Asserts.assertEQ(src[0].hash(), v.hash());
}

// non escaping allocation with uncommon trap that needs
@@ -907,12 +913,18 @@ public MyValue2.ref test30(MyValue2.ref[] src, boolean flag) {

@DontCompile
public void test30_verifier(boolean warmup) {
MyValue2.ref[] src = new MyValue2.ref[10];
MyValue2.ref[] src1 = new MyValue2.ref[10];
MyValue2.val[] src2 = new MyValue2.val[10];
for (int i = 0; i < 10; ++i) {
src[i] = MyValue2.createWithFieldsInline(rI, (i % 2) == 0);
src1[i] = MyValue2.createWithFieldsInline(rI, (i % 2) == 0);
src2[i] = MyValue2.createWithFieldsInline(rI, (i % 2) == 0);
}
MyValue2.ref v = test30(src1, !warmup);
Asserts.assertEQ(src1[0].hash(), v.hash());
if (!warmup) {
v = test30(src2, true);
Asserts.assertEQ(src2[0].hash(), v.hash());
}
MyValue2.ref v = test30(src, !warmup);
Asserts.assertEQ(src[0].hash(), v.hash());
}

// non escaping allocation with memory phi
@@ -2812,7 +2824,6 @@ public void test107_verifier(boolean warmup) {
test107();
}


@Test
@Warmup(10000)
public Object test108(MyValue1.ref[] src, boolean flag) {
@@ -2828,4 +2839,33 @@ public void test108_verifier(boolean warmup) {
test108(src, !warmup);
}

// Test LoadNode::can_see_arraycopy_value optimization
@Test()
public static void test109() {
MyValue1[] src = new MyValue1[1];
MyValue1.ref[] dst = new MyValue1.ref[1];
src[0] = testValue1;
System.arraycopy(src, 0, dst, 0, 1);
Asserts.assertEquals(src[0], dst[0]);
}

@DontCompile
public void test109_verifier(boolean warmup) {
test109();
}

// Same as test109 but with Object destination array
@Test()
public static void test110() {
MyValue1[] src = new MyValue1[1];
Object[] dst = new Object[1];
src[0] = testValue1;
System.arraycopy(src, 0, dst, 0, 1);
Asserts.assertEquals(src[0], dst[0]);
}

@DontCompile
public void test110_verifier(boolean warmup) {
test110();
}
}