infernap12/jdk
1
0
Fork 0
mirror of https://github.com/openjdk/jdk.git synced 2026-07-28 11:53:09 +00:00
Code Issues Packages Projects Releases Wiki Activity

8341976: C2: use_mem_state != load->find_exact_control(load->in(0)) assert failure

Browse Source 
Reviewed-by: chagedorn, dfenacci
This commit is contained in:
Roland Westrelin 2025-04-08 15:52:05 +00:00
parent 58ff36f3bd
commit 4645ddbb6b
6 changed files with 117 additions and 39 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

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

@ -681,18 +681,21 @@ bool ArrayCopyNode::may_modify(const TypeOopPtr* t_oop, PhaseValues* phase) {
return CallNode::may_modify_arraycopy_helper(dest_t, t_oop, phase);
}
bool ArrayCopyNode::may_modify_helper(const TypeOopPtr* t_oop, Node* n, PhaseValues* phase, CallNode*& call) {
bool ArrayCopyNode::may_modify_helper(const TypeOopPtr* t_oop, Node* n, PhaseValues* phase, ArrayCopyNode*& ac) {
if (n != nullptr &&
n->is_Call() &&
n->as_Call()->may_modify(t_oop, phase) &&
(n->as_Call()->is_ArrayCopy() || n->as_Call()->is_call_to_arraycopystub())) {
call = n->as_Call();
n->is_ArrayCopy() &&
n->as_ArrayCopy()->may_modify(t_oop, phase)) {
ac = n->as_ArrayCopy();
return true;
}
return false;
}
bool ArrayCopyNode::may_modify(const TypeOopPtr* t_oop, MemBarNode* mb, PhaseValues* phase, ArrayCopyNode*& ac) {
if (mb->trailing_expanded_array_copy()) {
return true;
}
Node* c = mb->in(0);
BarrierSetC2* bs = BarrierSet::barrier_set()->barrier_set_c2();
@ -705,23 +708,19 @@ bool ArrayCopyNode::may_modify(const TypeOopPtr* t_oop, MemBarNode* mb, PhaseVal
for (uint i = 1; i < c->req(); i++) {
if (c->in(i) != nullptr) {
Node* n = c->in(i)->in(0);
if (may_modify_helper(t_oop, n, phase, call)) {
ac = call->isa_ArrayCopy();
if (may_modify_helper(t_oop, n, phase, ac)) {
assert(c == mb->in(0), "only for clone");
return true;
}
}
}
} else if (may_modify_helper(t_oop, c->in(0), phase, call)) {
ac = call->isa_ArrayCopy();
} else if (may_modify_helper(t_oop, c->in(0), phase, ac)) {
#ifdef ASSERT
bool use_ReduceInitialCardMarks = BarrierSet::barrier_set()->is_a(BarrierSet::CardTableBarrierSet) &&
static_cast<CardTableBarrierSetC2*>(bs)->use_ReduceInitialCardMarks();
assert(c == mb->in(0) || (ac != nullptr && ac->is_clonebasic() && !use_ReduceInitialCardMarks), "only for clone");
assert(c == mb->in(0) || (ac->is_clonebasic() && !use_ReduceInitialCardMarks), "only for clone");
#endif
return true;
} else if (mb->trailing_partial_array_copy()) {
return true;
}
return false;

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

@ -121,7 +121,7 @@ private:
BasicType copy_type, const Type* value_type, int count);
bool finish_transform(PhaseGVN *phase, bool can_reshape,
Node* ctl, Node *mem);
static bool may_modify_helper(const TypeOopPtr* t_oop, Node* n, PhaseValues* phase, CallNode*& call);
static bool may_modify_helper(const TypeOopPtr* t_oop, Node* n, PhaseValues* phase, ArrayCopyNode*& ac);
public:
static Node* load(BarrierSetC2* bs, PhaseGVN *phase, Node*& ctl, MergeMemNode* mem, Node* addr, const TypePtr* adr_type, const Type *type, BasicType bt);
private:

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

@ -111,7 +111,7 @@ private:
void expand_unlock_node(UnlockNode *unlock);
// More helper methods modeled after GraphKit for array copy
void insert_mem_bar(Node** ctrl, Node** mem, int opcode, Node* precedent = nullptr);
void insert_mem_bar(Node** ctrl, Node** mem, int opcode, int alias_idx, Node* precedent = nullptr);
Node* array_element_address(Node* ary, Node* idx, BasicType elembt);
Node* ConvI2L(Node* offset);
@ -171,7 +171,7 @@ private:
Node* src, Node* src_offset,
Node* dest, Node* dest_offset,
Node* copy_length, bool dest_uninitialized);
bool generate_unchecked_arraycopy(Node** ctrl, MergeMemNode** mem,
void generate_unchecked_arraycopy(Node** ctrl, MergeMemNode** mem,
const TypePtr* adr_type,
BasicType basic_elem_type,
bool disjoint_bases,

55
src/hotspot/share/opto/macroArrayCopy.cpp
View File

@ -36,8 +36,8 @@
#include "utilities/align.hpp"
#include "utilities/powerOfTwo.hpp"
void PhaseMacroExpand::insert_mem_bar(Node** ctrl, Node** mem, int opcode, Node* precedent) {
MemBarNode* mb = MemBarNode::make(C, opcode, Compile::AliasIdxBot, precedent);
void PhaseMacroExpand::insert_mem_bar(Node** ctrl, Node** mem, int opcode, int alias_idx, Node* precedent) {
MemBarNode* mb = MemBarNode::make(C, opcode, alias_idx, precedent);
mb->init_req(TypeFunc::Control, *ctrl);
mb->init_req(TypeFunc::Memory, *mem);
transform_later(mb);
@ -45,7 +45,14 @@ void PhaseMacroExpand::insert_mem_bar(Node** ctrl, Node** mem, int opcode, Node*
transform_later(*ctrl);
Node* mem_proj = new ProjNode(mb,TypeFunc::Memory);
transform_later(mem_proj);
*mem = mem_proj;
if (alias_idx == Compile::AliasIdxBot) {
*mem = mem_proj;
} else {
MergeMemNode* mm = (*mem)->clone()->as_MergeMem();
mm->set_memory_at(alias_idx, mem_proj);
transform_later(mm);
*mem = mm;
}
}
Node* PhaseMacroExpand::array_element_address(Node* ary, Node* idx, BasicType elembt) {
@ -667,16 +674,15 @@ Node* PhaseMacroExpand::generate_arraycopy(ArrayCopyNode *ac, AllocateArrayNode*
}
}
bool is_partial_array_copy = false;
if (!(*ctrl)->is_top()) {
// Generate the fast path, if possible.
Node* local_ctrl = *ctrl;
MergeMemNode* local_mem = MergeMemNode::make(mem);
transform_later(local_mem);
is_partial_array_copy = generate_unchecked_arraycopy(&local_ctrl, &local_mem,
adr_type, copy_type, disjoint_bases,
src, src_offset, dest, dest_offset,
ConvI2X(copy_length), acopy_to_uninitialized);
generate_unchecked_arraycopy(&local_ctrl, &local_mem,
adr_type, copy_type, disjoint_bases,
src, src_offset, dest, dest_offset,
ConvI2X(copy_length), acopy_to_uninitialized);
// Present the results of the fast call.
result_region->init_req(fast_path, local_ctrl);
@ -818,16 +824,23 @@ Node* PhaseMacroExpand::generate_arraycopy(ArrayCopyNode *ac, AllocateArrayNode*
// Do not let stores that initialize this object be reordered with
// a subsequent store that would make this object accessible by
// other threads.
insert_mem_bar(ctrl, &out_mem, Op_MemBarStoreStore);
assert(ac->_dest_type == TypeOopPtr::BOTTOM, "non escaping destination shouldn't have narrow slice");
insert_mem_bar(ctrl, &out_mem, Op_MemBarStoreStore, Compile::AliasIdxBot);
} else {
insert_mem_bar(ctrl, &out_mem, Op_MemBarCPUOrder);
int mem_bar_alias_idx = Compile::AliasIdxBot;
if (ac->_dest_type != TypeOopPtr::BOTTOM) {
// The graph was transformed under the assumption the ArrayCopy node only had an effect on a narrow slice. We can't
// insert a wide membar now that it's being expanded: a load that uses the input memory state of the ArrayCopy
// could then become anti dependent on the membar when it was not anti dependent on the ArrayCopy leading to a
// broken graph.
mem_bar_alias_idx = C->get_alias_index(ac->_dest_type->add_offset(Type::OffsetBot)->is_ptr());
}
insert_mem_bar(ctrl, &out_mem, Op_MemBarCPUOrder, mem_bar_alias_idx);
}
if (is_partial_array_copy) {
assert((*ctrl)->is_Proj(), "MemBar control projection");
assert((*ctrl)->in(0)->isa_MemBar(), "MemBar node");
(*ctrl)->in(0)->isa_MemBar()->set_trailing_partial_array_copy();
}
assert((*ctrl)->is_Proj(), "MemBar control projection");
assert((*ctrl)->in(0)->isa_MemBar(), "MemBar node");
(*ctrl)->in(0)->isa_MemBar()->set_trailing_expanded_array_copy();
_igvn.replace_node(_callprojs.fallthrough_memproj, out_mem);
if (_callprojs.fallthrough_ioproj != nullptr) {
@ -837,7 +850,7 @@ Node* PhaseMacroExpand::generate_arraycopy(ArrayCopyNode *ac, AllocateArrayNode*
#ifdef ASSERT
const TypeOopPtr* dest_t = _igvn.type(dest)->is_oopptr();
if (dest_t->is_known_instance() && !is_partial_array_copy) {
if (dest_t->is_known_instance()) {
ArrayCopyNode* ac = nullptr;
assert(ArrayCopyNode::may_modify(dest_t, (*ctrl)->in(0)->as_MemBar(), &_igvn, ac), "dependency on arraycopy lost");
assert(ac == nullptr, "no arraycopy anymore");
@ -1174,14 +1187,16 @@ Node* PhaseMacroExpand::generate_generic_arraycopy(Node** ctrl, MergeMemNode** m
}
// Helper function; generates the fast out-of-line call to an arraycopy stub.
bool PhaseMacroExpand::generate_unchecked_arraycopy(Node** ctrl, MergeMemNode** mem,
void PhaseMacroExpand::generate_unchecked_arraycopy(Node** ctrl, MergeMemNode** mem,
const TypePtr* adr_type,
BasicType basic_elem_type,
bool disjoint_bases,
Node* src, Node* src_offset,
Node* dest, Node* dest_offset,
Node* copy_length, bool dest_uninitialized) {
if ((*ctrl)->is_top()) return false;
if ((*ctrl)->is_top()) {
return;
}
Node* src_start = src;
Node* dest_start = dest;
@ -1226,9 +1241,7 @@ bool PhaseMacroExpand::generate_unchecked_arraycopy(Node** ctrl, MergeMemNode**
}
transform_later(*mem);
*ctrl = exit_block;
return true;
}
return false;
}
#undef XTOP
@ -1311,7 +1324,7 @@ void PhaseMacroExpand::expand_arraycopy_node(ArrayCopyNode *ac) {
// Do not let writes into the source float below the arraycopy.
{
Node* mem = ac->in(TypeFunc::Memory);
insert_mem_bar(&ctrl, &mem, Op_MemBarCPUOrder);
insert_mem_bar(&ctrl, &mem, Op_MemBarCPUOrder, Compile::AliasIdxBot);
merge_mem = MergeMemNode::make(mem);
transform_later(merge_mem);

6
src/hotspot/share/opto/memnode.hpp
View File

@ -1132,7 +1132,7 @@ class MemBarNode: public MultiNode {
LeadingStore,
TrailingLoadStore,
LeadingLoadStore,
TrailingPartialArrayCopy
TrailingExpandedArrayCopy
} _kind;
#ifdef ASSERT
@ -1169,8 +1169,8 @@ public:
bool trailing() const { return _kind == TrailingLoad || _kind == TrailingStore || _kind == TrailingLoadStore; }
bool leading() const { return _kind == LeadingStore || _kind == LeadingLoadStore; }
bool standalone() const { return _kind == Standalone; }
void set_trailing_partial_array_copy() { _kind = TrailingPartialArrayCopy; }
bool trailing_partial_array_copy() const { return _kind == TrailingPartialArrayCopy; }
void set_trailing_expanded_array_copy() { _kind = TrailingExpandedArrayCopy; }
bool trailing_expanded_array_copy() const { return _kind == TrailingExpandedArrayCopy; }
static void set_store_pair(MemBarNode* leading, MemBarNode* trailing);
static void set_load_store_pair(MemBarNode* leading, MemBarNode* trailing);

66
test/hotspot/jtreg/compiler/arraycopy/TestSunkLoadAntiDependency.java Normal file
View File

@ -0,0 +1,66 @@
/*
* Copyright (c) 2025, Red Hat, Inc. 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 8341976
* @summary C2: use_mem_state != load->find_exact_control(load->in(0)) assert failure
* @run main/othervm -XX:-BackgroundCompilation TestSunkLoadAntiDependency
* @run main TestSunkLoadAntiDependency
*/
public class TestSunkLoadAntiDependency {
private static volatile int volatileField;
public static void main(String[] args) {
int[] array = new int[100];
for (int i = 0; i < 20_000; i++) {
test1(array, 2);
inlined1(array, 100, 0, 100, array);
}
}
private static int test1(int[] src, int length) {
length = Integer.max(1, length);
int[] dst = new int[2];
int stop;
for (stop = 0; stop < 10; stop++) {
for (int i = 0; i < 10; i++) {
}
}
int start;
for (start = 0; start < 9; start++) {
for (int i = 0; i < 10; i++) {
}
}
inlined1(src, length, start, stop, dst);
return dst[0] + dst[1];
}
private static void inlined1(int[] src, int length, int start, int stop, int[] dst) {
for (int i = start; i < stop; i++) {
volatileField = 42;
System.arraycopy(src, 0, dst, 0, length);
}
}
}