8376220: C2: Refactor the logic to in MemNode::find_previous_store

Reviewed-by: rcastanedalo, bmaillard
This commit is contained in:
Quan Anh Mai 2026-02-21 11:52:50 +00:00
parent d4dc5e49c1
commit a35945ae06
2 changed files with 256 additions and 30 deletions

View File

@ -44,6 +44,7 @@
#include "opto/mempointer.hpp"
#include "opto/mulnode.hpp"
#include "opto/narrowptrnode.hpp"
#include "opto/opcodes.hpp"
#include "opto/phaseX.hpp"
#include "opto/regalloc.hpp"
#include "opto/regmask.hpp"
@ -552,6 +553,18 @@ Node::DomResult MemNode::maybe_all_controls_dominate(Node* dom, Node* sub) {
bool MemNode::detect_ptr_independence(Node* p1, AllocateNode* a1,
Node* p2, AllocateNode* a2,
PhaseTransform* phase) {
// Trivial case: Non-overlapping values. Be careful, we can cast a raw pointer to an oop (e.g. in
// the allocation pattern) so joining the types only works if both are oops. join may also give
// an incorrect result when both pointers are nullable and the result is supposed to be
// TypePtr::NULL_PTR, so we exclude that case.
const Type* p1_type = p1->bottom_type();
const Type* p2_type = p2->bottom_type();
if (p1_type->isa_oopptr() && p2_type->isa_oopptr() &&
(!p1_type->maybe_null() || !p2_type->maybe_null()) &&
p1_type->join(p2_type)->empty()) {
return true;
}
// Attempt to prove that these two pointers cannot be aliased.
// They may both manifestly be allocations, and they should differ.
// Or, if they are not both allocations, they can be distinct constants.
@ -690,6 +703,20 @@ Node* MemNode::find_previous_store(PhaseValues* phase) {
Node* base = AddPNode::Ideal_base_and_offset(adr, phase, offset);
AllocateNode* alloc = AllocateNode::Ideal_allocation(base);
const TypePtr* adr_type = this->adr_type();
if (adr_type == nullptr) {
// This means the access is dead
return phase->C->top();
} else if (adr_type->base() == TypePtr::AnyPtr) {
assert(adr_type->ptr() == TypePtr::Null, "MemNode should never access a wide memory");
// Give up, this will upset Compile::get_alias_index
return nullptr;
}
int alias_idx = phase->C->get_alias_index(adr_type);
assert(alias_idx != Compile::AliasIdxTop, "must not be a dead node");
assert(alias_idx != Compile::AliasIdxBot || !phase->C->do_aliasing(), "must not be a very wide access");
if (offset == Type::OffsetBot)
return nullptr; // cannot unalias unless there are precise offsets
@ -709,15 +736,32 @@ Node* MemNode::find_previous_store(PhaseValues* phase) {
Node* st_adr = mem->in(MemNode::Address);
intptr_t st_offset = 0;
Node* st_base = AddPNode::Ideal_base_and_offset(st_adr, phase, st_offset);
if (st_base == nullptr)
break; // inscrutable pointer
// For raw accesses it's not enough to prove that constant offsets don't intersect.
// We need the bases to be the equal in order for the offset check to make sense.
if ((adr_maybe_raw || check_if_adr_maybe_raw(st_adr)) && st_base != base) {
if (st_base == nullptr) {
// inscrutable pointer
break;
}
// If the bases are the same and the offsets are the same, it seems that this is the exact
// store we are looking for, the caller will check if the type of the store matches using
// MemNode::can_see_stored_value
if (st_base == base && st_offset == offset) {
return mem; // (b) found the store that this access observes
}
// If it is provable that the memory accessed by mem does not overlap the memory accessed by
// this, we may walk past mem.
// For raw accesses, 2 accesses are independent if they have the same base and the offsets
// say that they do not overlap.
// For heap accesses, 2 accesses are independent if either the bases are provably different
// at runtime or the offsets say that the accesses do not overlap.
if ((adr_maybe_raw || check_if_adr_maybe_raw(st_adr)) && st_base != base) {
// Raw accesses can only be provably independent if they have the same base
break;
}
// If the offsets say that the accesses do not overlap, then it is provable that mem and this
// do not overlap. For example, a LoadI from Object+8 is independent from a StoreL into
// Object+12, no matter what the bases are.
if (st_offset != offset && st_offset != Type::OffsetBot) {
const int MAX_STORE = MAX2(BytesPerLong, (int)MaxVectorSize);
assert(mem->as_Store()->memory_size() <= MAX_STORE, "");
@ -730,25 +774,25 @@ Node* MemNode::find_previous_store(PhaseValues* phase) {
// in the same sequence of RawMem effects. We sometimes initialize
// a whole 'tile' of array elements with a single jint or jlong.)
mem = mem->in(MemNode::Memory);
continue; // (a) advance through independent store memory
continue; // (a) advance through the independent store
}
}
if (st_base != base &&
detect_ptr_independence(base, alloc,
st_base,
AllocateNode::Ideal_allocation(st_base),
phase)) {
// Success: The bases are provably independent.
// Same base and overlapping offsets, it seems provable that the accesses overlap, give up
if (st_base == base) {
break;
}
// Try to prove that 2 different base nodes at compile time are different values at runtime
bool known_independent = false;
if (detect_ptr_independence(base, alloc, st_base, AllocateNode::Ideal_allocation(st_base), phase)) {
known_independent = true;
}
if (known_independent) {
mem = mem->in(MemNode::Memory);
continue; // (a) advance through independent store memory
continue; // (a) advance through the independent store
}
// (b) At this point, if the bases or offsets do not agree, we lose,
// since we have not managed to prove 'this' and 'mem' independent.
if (st_base == base && st_offset == offset) {
return mem; // let caller handle steps (c), (d)
}
} else if (mem->is_Proj() && mem->in(0)->is_Initialize()) {
InitializeNode* st_init = mem->in(0)->as_Initialize();
AllocateNode* st_alloc = st_init->allocation();
@ -769,7 +813,6 @@ Node* MemNode::find_previous_store(PhaseValues* phase) {
// The bases are provably independent: Either they are
// manifestly distinct allocations, or else the control
// of this load dominates the store's allocation.
int alias_idx = phase->C->get_alias_index(adr_type());
if (alias_idx == Compile::AliasIdxRaw) {
mem = st_alloc->in(TypeFunc::Memory);
} else {
@ -792,6 +835,9 @@ Node* MemNode::find_previous_store(PhaseValues* phase) {
}
// Found an arraycopy that may affect that load
return mem;
} else if (mem->is_MergeMem()) {
mem = mem->as_MergeMem()->memory_at(alias_idx);
continue;
} else if (addr_t != nullptr && addr_t->is_known_instance_field()) {
// Can't use optimize_simple_memory_chain() since it needs PhaseGVN.
if (mem->is_Proj() && mem->in(0)->is_Call()) {
@ -817,10 +863,6 @@ Node* MemNode::find_previous_store(PhaseValues* phase) {
// we are looking for.
return mem;
}
} else if (mem->is_MergeMem()) {
int alias_idx = phase->C->get_alias_index(adr_type());
mem = mem->as_MergeMem()->memory_at(alias_idx);
continue; // (a) advance through independent MergeMem memory
}
}
@ -1843,7 +1885,6 @@ Node *LoadNode::Ideal(PhaseGVN *phase, bool can_reshape) {
Node* ctrl = in(MemNode::Control);
Node* address = in(MemNode::Address);
bool progress = false;
bool addr_mark = ((phase->type(address)->isa_oopptr() || phase->type(address)->isa_narrowoop()) &&
phase->type(address)->is_ptr()->offset() == oopDesc::mark_offset_in_bytes());
@ -1856,7 +1897,7 @@ Node *LoadNode::Ideal(PhaseGVN *phase, bool can_reshape) {
(depends_only_on_test() || has_unknown_control_dependency())) {
ctrl = ctrl->in(0);
set_req(MemNode::Control,ctrl);
progress = true;
return this;
}
intptr_t ignore = 0;
@ -1870,7 +1911,7 @@ Node *LoadNode::Ideal(PhaseGVN *phase, bool can_reshape) {
&& all_controls_dominate(base, phase->C->start())) {
// A method-invariant, non-null address (constant or 'this' argument).
set_req(MemNode::Control, nullptr);
progress = true;
return this;
}
}
@ -1943,6 +1984,10 @@ Node *LoadNode::Ideal(PhaseGVN *phase, bool can_reshape) {
// the alias index stuff. So instead, peek through Stores and IFF we can
// fold up, do so.
Node* prev_mem = find_previous_store(phase);
if (prev_mem != nullptr && prev_mem->is_top()) {
// find_previous_store returns top when the access is dead
return prev_mem;
}
if (prev_mem != nullptr) {
Node* value = can_see_arraycopy_value(prev_mem, phase);
if (value != nullptr) {
@ -1961,7 +2006,11 @@ Node *LoadNode::Ideal(PhaseGVN *phase, bool can_reshape) {
}
}
return progress ? this : nullptr;
if (!can_reshape) {
phase->record_for_igvn(this);
}
return nullptr;
}
// Helper to recognize certain Klass fields which are invariant across
@ -3565,6 +3614,10 @@ Node* StoreNode::Identity(PhaseGVN* phase) {
Node* prev_mem = find_previous_store(phase);
// Steps (a), (b): Walk past independent stores to find an exact match.
if (prev_mem != nullptr) {
if (prev_mem->is_top()) {
// find_previous_store returns top when the access is dead
return prev_mem;
}
Node* prev_val = can_see_stored_value(prev_mem, phase);
if (prev_val != nullptr && prev_val == val) {
// prev_val and val might differ by a cast; it would be good

View File

@ -0,0 +1,173 @@
/*
* Copyright (c) 2026, Oracle and/or its affiliates. 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.
*/
package compiler.c2.gvn;
import compiler.lib.ir_framework.*;
import jdk.internal.misc.Unsafe;
import jdk.test.lib.Asserts;
/*
* @test
* @bug 8376220
* @summary Tests that memory accesses can be elided when the compiler can see the value at the
* accessed memory location by walking the memory graph.
* @modules java.base/jdk.internal.misc
* @library /test/lib /
* @run driver ${test.main.class}
*/
public class TestFindStore {
private static final Unsafe U = Unsafe.getUnsafe();
static class P {
int v;
int u;
}
static final long V_OFFSET = U.objectFieldOffset(P.class, "v");
static final long U_OFFSET = U.objectFieldOffset(P.class, "u");
static class C1 extends P {}
static class C2 extends P {}
public static void main(String[] args) {
TestFramework.runWithFlags("--add-exports=java.base/jdk.internal.misc=ALL-UNNAMED");
}
@Run(test = {"testLoad", "testStore", "testLoadDependent1", "testLoadDependent2", "testLoadArray",
"testLoadArrayOverlap", "testLoadIndependentAliasClasses", "testLoadMismatched",
"testLoadArrayCopy", "testLoadArrayCopyUnknownLength"})
public void run() {
C1 c1 = new C1();
C2 c2 = new C2();
int[] a1 = new int[1000];
int[] a2 = new int[1000];
Asserts.assertEQ(0, testLoad(c1, c2, 0, 1));
Asserts.assertEQ(0, testStore(c2, 0).v);
Asserts.assertEQ(0, testLoadDependent1(c1, c1, 0, 1));
Asserts.assertEQ(1, testLoadDependent1(c2, c1, 0, 1));
Asserts.assertEQ(1, testLoadDependent2(c1, c1, 0, 1));
Asserts.assertEQ(0, testLoadDependent2(c2, c1, 0, 1));
Asserts.assertEQ(0, testLoadArray(a1, a2, 0, 1));
Asserts.assertEQ(0, testLoadArrayOverlap(a1, a2, 2, 0, 1));
Asserts.assertEQ(0, testLoadArrayOverlap(a1, a1, 0, 0, 1));
Asserts.assertEQ(1, testLoadArrayOverlap(a1, a1, 2, 0, 1));
Asserts.assertEQ(0, testLoadIndependentAliasClasses(c1, 0, 1));
Asserts.assertNE(0, testLoadMismatched(c1, 0, -1));
Asserts.assertEQ(1, testLoadArrayCopy(a1, a2, 1));
a1[2] = 0;
Asserts.assertEQ(1, testLoadArrayCopyUnknownLength(a1, a2, 100, 1));
a1[2] = 0;
Asserts.assertEQ(0, testLoadArrayCopyUnknownLength(a1, a2, 2, 1));
}
@Test
@IR(failOn = IRNode.LOAD)
static int testLoad(C1 c1, C2 c2, int v1, int v2) {
// c1 and c2 are provably independent
c1.v = v1;
c2.v = v2;
return c1.v;
}
@Test
@IR(counts = {IRNode.STORE, "1"}, phase = CompilePhase.BEFORE_MACRO_EXPANSION)
static C1 testStore(C2 c2, int v2) {
C1 c1 = new C1();
c2.v = v2;
c1.v = 0;
return c1;
}
@Test
@IR(counts = {IRNode.LOAD, "1"})
static int testLoadDependent1(P p, C1 c1, int v, int v1) {
// It cannot be proved that p and c1 are independent
c1.v = v1;
p.v = v;
return c1.v;
}
@Test
@IR(counts = {IRNode.LOAD, "1"})
static int testLoadDependent2(P p, C1 c1, int v, int v1) {
// It cannot be proved that p and c1 are independent
p.v = v;
c1.v = v1;
return p.v;
}
@Test
@IR(failOn = IRNode.LOAD)
static int testLoadArray(int[] a1, int[] a2, int v1, int v2) {
// a1[2] and a2[1] are provably independent
a1[2] = v1;
a2[1] = v2;
return a1[2];
}
@Test
@IR(counts = {IRNode.LOAD, "1"})
static int testLoadArrayOverlap(int[] a1, int[] a2, int idx, int v1, int v2) {
// Cannot prove that a1[2] and a2[idx] are independent
a1[2] = v1;
a2[idx] = v2;
return a1[2];
}
@Test
@IR(failOn = IRNode.LOAD)
static int testLoadIndependentAliasClasses(P p, int v, int u) {
p.v = v;
p.u = u;
return p.v;
}
@Test
@IR(counts = {IRNode.LOAD, "1"})
static int testLoadMismatched(P p, int v1, int v2) {
p.v = v1;
U.putIntUnaligned(p, (V_OFFSET + U_OFFSET) / 2, v2);
return p.v;
}
@Test
@IR(failOn = IRNode.LOAD, applyIf = {"ArrayCopyLoadStoreMaxElem", "<100"})
static int testLoadArrayCopy(int[] a1, int[] a2, int v) {
a2[2] = v;
// Should be large so the compiler does not just transform it into a couple of loads and stores
System.arraycopy(a2, 0, a1, 0, 100);
return a1[2];
}
@Test
@IR(counts = {IRNode.LOAD, "1"})
static int testLoadArrayCopyUnknownLength(int[] a1, int[] a2, int len, int v) {
a2[2] = v;
// Cannot determine if this overwrites a1[2]
System.arraycopy(a2, 0, a1, 0, len);
return a1[2];
}
}