infernap12/jdk
1
0
Fork 0
mirror of https://github.com/openjdk/jdk.git synced 2026-04-11 23:48:38 +00:00
Code Issues Packages Projects Releases Wiki Activity

8380158: C2: compiler/c2/TestGVNCrash.java asserts with adr and adr_type must agree

Browse Source 
Reviewed-by: chagedorn, dfenacci
This commit is contained in:
Roland Westrelin 2026-04-09 09:35:13 +00:00
parent 61b2508224
commit 60115c0bbf
3 changed files with 109 additions and 27 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

67
src/hotspot/share/opto/arraycopynode.cpp
View File

@ -258,6 +258,19 @@ Node* ArrayCopyNode::try_clone_instance(PhaseGVN *phase, bool can_reshape, int c
return mem;
}
// We may have narrowed the type of base because this runs with PhaseIterGVN::_delay_transform true, explicitly
// update the type of the AddP so it's consistent with its base and load() picks the right memory slice.
Node* ArrayCopyNode::make_and_transform_addp(PhaseGVN* phase, Node* base, Node* offset) {
return make_and_transform_addp(phase, base, base, offset);
}
Node* ArrayCopyNode::make_and_transform_addp(PhaseGVN* phase, Node* base, Node* ptr, Node* offset) {
assert(phase->is_IterGVN() == nullptr || phase->is_IterGVN()->delay_transform(), "helper method when delay transform is set");
Node* addp = phase->transform(AddPNode::make_with_base(base, ptr, offset));
phase->set_type(addp, addp->Value(phase));
return addp;
}
bool ArrayCopyNode::prepare_array_copy(PhaseGVN *phase, bool can_reshape,
Node*& adr_src,
Node*& base_src,
@ -332,12 +345,11 @@ bool ArrayCopyNode::prepare_array_copy(PhaseGVN *phase, bool can_reshape,
Node* dest_scale = phase->transform(new LShiftXNode(dest_offset, phase->intcon(shift)));
adr_src = phase->transform(AddPNode::make_with_base(base_src, src_scale));
adr_dest = phase->transform(AddPNode::make_with_base(base_dest, dest_scale));
adr_src = phase->transform(AddPNode::make_with_base(base_src, adr_src, phase->MakeConX(header)));
adr_dest = phase->transform(AddPNode::make_with_base(base_dest, adr_dest, phase->MakeConX(header)));
adr_src = make_and_transform_addp(phase, base_src, src_scale);
adr_dest = make_and_transform_addp(phase, base_dest, dest_scale);
adr_src = make_and_transform_addp(phase, base_src, adr_src, phase->MakeConX(header));
adr_dest = make_and_transform_addp(phase, base_dest, adr_dest, phase->MakeConX(header));
copy_type = dest_elem;
} else {
assert(ary_src != nullptr, "should be a clone");
@ -355,8 +367,8 @@ bool ArrayCopyNode::prepare_array_copy(PhaseGVN *phase, bool can_reshape,
return false;
}
adr_src = phase->transform(AddPNode::make_with_base(base_src, src_offset));
adr_dest = phase->transform(AddPNode::make_with_base(base_dest, dest_offset));
adr_src = make_and_transform_addp(phase, base_src, src_offset);
adr_dest = make_and_transform_addp(phase, base_dest, dest_offset);
// The address is offsetted to an aligned address where a raw copy would start.
// If the clone copy is decomposed into load-stores - the address is adjusted to
@ -366,8 +378,8 @@ bool ArrayCopyNode::prepare_array_copy(PhaseGVN *phase, bool can_reshape,
int diff = arrayOopDesc::base_offset_in_bytes(elem) - offset;
assert(diff >= 0, "clone should not start after 1st array element");
if (diff > 0) {
adr_src = phase->transform(AddPNode::make_with_base(base_src, adr_src, phase->MakeConX(diff)));
adr_dest = phase->transform(AddPNode::make_with_base(base_dest, adr_dest, phase->MakeConX(diff)));
adr_src = make_and_transform_addp(phase, base_src, adr_src, phase->MakeConX(diff));
adr_dest = make_and_transform_addp(phase, base_dest, adr_dest, phase->MakeConX(diff));
}
copy_type = elem;
value_type = ary_src->elem();
@ -429,12 +441,8 @@ Node* ArrayCopyNode::array_copy_forward(PhaseGVN *phase,
store(bs, phase, forward_ctl, mm, adr_dest, atp_dest, v, value_type, copy_type);
for (int i = 1; i < count; i++) {
Node* off = phase->MakeConX(type2aelembytes(copy_type) * i);
Node* next_src = phase->transform(AddPNode::make_with_base(base_src,adr_src,off));
// We may have narrowed the type of next_src right before calling this method but because this runs with
// PhaseIterGVN::_delay_transform true, explicitly update the type of the AddP so it's consistent with its
// base and load() picks the right memory slice.
phase->set_type(next_src, next_src->Value(phase));
Node* next_dest = phase->transform(AddPNode::make_with_base(base_dest,adr_dest,off));
Node* next_src = make_and_transform_addp(phase, base_src,adr_src,off);
Node* next_dest = make_and_transform_addp(phase, base_dest,adr_dest,off);
// Same as above
phase->set_type(next_dest, next_dest->Value(phase));
v = load(bs, phase, forward_ctl, mm, next_src, atp_src, value_type, copy_type);
@ -473,13 +481,8 @@ Node* ArrayCopyNode::array_copy_backward(PhaseGVN *phase,
if (count > 0) {
for (int i = count-1; i >= 1; i--) {
Node* off = phase->MakeConX(type2aelembytes(copy_type) * i);
Node* next_src = phase->transform(AddPNode::make_with_base(base_src,adr_src,off));
// We may have narrowed the type of next_src right before calling this method but because this runs with
// PhaseIterGVN::_delay_transform true, explicitly update the type of the AddP so it's consistent with its
// base and store() picks the right memory slice.
phase->set_type(next_src, next_src->Value(phase));
Node* next_dest = phase->transform(AddPNode::make_with_base(base_dest,adr_dest,off));
phase->set_type(next_dest, next_dest->Value(phase));
Node* next_src = make_and_transform_addp(phase, base_src,adr_src,off);
Node* next_dest = make_and_transform_addp(phase, base_dest,adr_dest,off);
Node* v = load(bs, phase, backward_ctl, mm, next_src, atp_src, value_type, copy_type);
store(bs, phase, backward_ctl, mm, next_dest, atp_dest, v, value_type, copy_type);
}
@ -618,21 +621,31 @@ Node *ArrayCopyNode::Ideal(PhaseGVN *phase, bool can_reshape) {
phase->set_type(src, phase->type(src)->join_speculative(atp_src));
phase->set_type(dest, phase->type(dest)->join_speculative(atp_dest));
// Control flow is going to be created, it's easier to do with _delay_transform set to true.
// prepare_array_copy() doesn't build control flow, but it creates AddP nodes. The src/dest type possibly gets
// narrowed above. If a newly created AddP node is commoned with a pre-existing one, then the type narrowing is lost.
// Setting _delay_transform before prepare_array_copy() guarantees this doesn't happen.
if (can_reshape) {
assert(!phase->is_IterGVN()->delay_transform(), "cannot delay transforms");
phase->is_IterGVN()->set_delay_transform(true);
}
if (!prepare_array_copy(phase, can_reshape,
adr_src, base_src, adr_dest, base_dest,
copy_type, value_type, disjoint_bases)) {
assert(adr_src == nullptr, "no node can be left behind");
assert(adr_dest == nullptr, "no node can be left behind");
if (can_reshape) {
assert(phase->is_IterGVN()->delay_transform(), "cannot delay transforms");
phase->is_IterGVN()->set_delay_transform(false);
}
return nullptr;
}
Node* in_mem = in(TypeFunc::Memory);
if (can_reshape) {
assert(!phase->is_IterGVN()->delay_transform(), "cannot delay transforms");
phase->is_IterGVN()->set_delay_transform(true);
}
Node* backward_ctl = phase->C->top();
Node* forward_ctl = phase->C->top();
array_copy_test_overlap(phase, can_reshape, disjoint_bases, count, forward_ctl, backward_ctl);

4
src/hotspot/share/opto/arraycopynode.hpp
View File

@ -104,6 +104,10 @@ private:
static const TypePtr* get_address_type(PhaseGVN* phase, const TypePtr* atp, Node* n);
Node* try_clone_instance(PhaseGVN *phase, bool can_reshape, int count);
Node* make_and_transform_addp(PhaseGVN* phase, Node* base, Node* offset);
Node* make_and_transform_addp(PhaseGVN* phase, Node* base, Node* ptr, Node* offset);
bool prepare_array_copy(PhaseGVN *phase, bool can_reshape,
Node*& adr_src, Node*& base_src, Node*& adr_dest, Node*& base_dest,
BasicType& copy_type, const Type*& value_type, bool& disjoint_bases);

65
test/hotspot/jtreg/compiler/arraycopy/TestACNonEscapingSrcBadAddPBaseType.java Normal file
View File

@ -0,0 +1,65 @@
/*
* Copyright (c) 2026 IBM Corporation. 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 8380158
* @summary C2: compiler/c2/TestGVNCrash.java asserts with adr and adr_type must agree
* @run main/othervm -XX:-TieredCompilation -XX:-UseOnStackReplacement -XX:-BackgroundCompilation
* -XX:CompileOnly=compiler.arraycopy.TestACNonEscapingSrcBadAddPBaseType::test1
* -XX:+UnlockDiagnosticVMOptions -XX:+StressIGVN -XX:StressSeed=946074051 ${test.main.class}
* @run main/othervm -XX:-TieredCompilation -XX:-UseOnStackReplacement -XX:-BackgroundCompilation
* -XX:CompileOnly=compiler.arraycopy.TestACNonEscapingSrcBadAddPBaseType::test1
* -XX:+UnlockDiagnosticVMOptions -XX:+StressIGVN ${test.main.class}
* @run main ${test.main.class}
*/
package compiler.arraycopy;
public class TestACNonEscapingSrcBadAddPBaseType {
private static volatile int volatileField;
public static void main(String[] args) {
int[] dst = new int[2];
for (int i = 0; i < 20_000; i++) {
test1(dst);
}
}
private static void test1(int[] dst) {
int[] array = new int[2];
A a = new A(array);
volatileField = 42;
int[] src = a.src;
System.arraycopy(src, 0, dst, 0, src.length);
System.arraycopy(src, 0, dst, 0, src.length);
}
private static class A {
private final int[] src;
public A(int[] src) {
this.src = src;
}
}
}