Skip to content

Commit 4445fa8

Browse files
EgorBoCopilotCopilot
authored
Remove more write-barriers (#128238)
1. `GT_STORE_BLK` now uses the same address analysis as `GT_STOREIND` --------- Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
1 parent e00aafd commit 4445fa8

3 files changed

Lines changed: 48 additions & 9 deletions

File tree

src/coreclr/jit/assertionprop.cpp

Lines changed: 45 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -986,7 +986,7 @@ const Compiler::AssertionDsc& Compiler::optGetAssertion(AssertionIndex assertInd
986986
return assertion;
987987
}
988988

989-
ValueNum Compiler::optConservativeNormalVN(GenTree* tree)
989+
ValueNum Compiler::optConservativeNormalVN(const GenTree* tree)
990990
{
991991
if (optLocalAssertionProp)
992992
{
@@ -3879,9 +3879,10 @@ GenTree* Compiler::optAssertionProp_BlockStore(ASSERT_VALARG_TP assertions, GenT
38793879
{
38803880
assert(store->OperIs(GT_STORE_BLK));
38813881

3882-
bool didZeroObjProp = optZeroObjAssertionProp(store->Data(), assertions);
3883-
bool didNonNullProp = optNonNullAssertionProp_Ind(assertions, store);
3884-
if (didZeroObjProp || didNonNullProp)
3882+
bool didZeroObjProp = optZeroObjAssertionProp(store->Data(), assertions);
3883+
bool didNonNullProp = optNonNullAssertionProp_Ind(assertions, store);
3884+
bool didWriteBarrierProp = optWriteBarrierAssertionProp_StoreBlk(assertions, store);
3885+
if (didZeroObjProp || didNonNullProp || didWriteBarrierProp)
38853886
{
38863887
return optAssertionProp_Update(store, store, stmt);
38873888
}
@@ -5196,7 +5197,7 @@ static GCInfo::WriteBarrierForm GetWriteBarrierForm(Compiler* comp, ValueNum vn)
51965197
// Boxed static - always on the heap
51975198
return GCInfo::WriteBarrierForm::WBF_BarrierUnchecked;
51985199
}
5199-
if (funcApp.m_func == VNFunc(GT_ADD))
5200+
if (funcApp.m_func == VNF_ADD)
52005201
{
52015202
// Check arguments of the GT_ADD
52025203
// To make it conservative, we require one of the arguments to be a constant, e.g.:
@@ -5261,7 +5262,7 @@ bool Compiler::optWriteBarrierAssertionProp_StoreInd(ASSERT_VALARG_TP assertions
52615262
return ValueNumStore::VNVisit::Abort;
52625263
};
52635264

5264-
if (vnStore->VNVisitReachingVNs(value->gtVNPair.GetConservative(), vnVisitor) == ValueNumStore::VNVisit::Continue)
5265+
if (vnStore->VNVisitReachingVNs(optConservativeNormalVN(value), vnVisitor) == ValueNumStore::VNVisit::Continue)
52655266
{
52665267
barrierType = GCInfo::WriteBarrierForm::WBF_NoBarrier;
52675268
}
@@ -5270,7 +5271,7 @@ bool Compiler::optWriteBarrierAssertionProp_StoreInd(ASSERT_VALARG_TP assertions
52705271
{
52715272
// NOTE: we might want to inspect indirs with GTF_IND_TGT_HEAP flag as well - what if we can prove
52725273
// that they actually need no barrier? But that comes with a TP regression.
5273-
barrierType = GetWriteBarrierForm(this, addr->gtVNPair.GetConservative());
5274+
barrierType = GetWriteBarrierForm(this, optConservativeNormalVN(addr));
52745275
}
52755276

52765277
JITDUMP("Trying to determine the exact type of write barrier for STOREIND [%d06]: ", dspTreeID(indir));
@@ -5291,6 +5292,43 @@ bool Compiler::optWriteBarrierAssertionProp_StoreInd(ASSERT_VALARG_TP assertions
52915292
return false;
52925293
}
52935294

5295+
//------------------------------------------------------------------------
5296+
// optWriteBarrierAssertionProp_StoreBlk: The STORE_BLK counterpart of
5297+
// optWriteBarrierAssertionProp_StoreInd. For block stores that contain GC
5298+
// pointers, attempt to prove via VN/assertion analysis that the destination
5299+
// address is not on the GC heap (or always on the heap).
5300+
//
5301+
// Arguments:
5302+
// assertions - Active assertions
5303+
// store - The STORE_BLK node
5304+
//
5305+
// Return Value:
5306+
// Whether the exact type of write barrier was determined and marked on the STOREBLK node.
5307+
//
5308+
bool Compiler::optWriteBarrierAssertionProp_StoreBlk(ASSERT_VALARG_TP assertions, GenTreeBlk* store)
5309+
{
5310+
if (optLocalAssertionProp || !store->GetLayout()->HasGCPtr() ||
5311+
((store->gtFlags & (GTF_IND_TGT_NOT_HEAP | GTF_IND_TGT_HEAP)) != 0))
5312+
{
5313+
return false;
5314+
}
5315+
5316+
GCInfo::WriteBarrierForm barrierType = GetWriteBarrierForm(this, optConservativeNormalVN(store->Addr()));
5317+
if (barrierType == GCInfo::WriteBarrierForm::WBF_NoBarrier)
5318+
{
5319+
JITDUMP("Add GTF_IND_TGT_NOT_HEAP to STORE_BLK [%06d]: ", dspTreeID(store));
5320+
store->gtFlags |= GTF_IND_TGT_NOT_HEAP;
5321+
return true;
5322+
}
5323+
if (barrierType == GCInfo::WriteBarrierForm::WBF_BarrierUnchecked)
5324+
{
5325+
JITDUMP("Add GTF_IND_TGT_HEAP to STORE_BLK [%06d]: ", dspTreeID(store));
5326+
store->gtFlags |= GTF_IND_TGT_HEAP;
5327+
return true;
5328+
}
5329+
return false;
5330+
}
5331+
52945332
/*****************************************************************************
52955333
*
52965334
* Given a tree consisting of a call and a set of available assertions, we

src/coreclr/jit/compiler.h

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -9041,7 +9041,7 @@ class Compiler
90419041
AssertionIndex optFindComplementary(AssertionIndex assertionIndex);
90429042
void optMapComplementary(AssertionIndex assertionIndex, AssertionIndex index);
90439043

9044-
ValueNum optConservativeNormalVN(GenTree* tree);
9044+
ValueNum optConservativeNormalVN(const GenTree* tree);
90459045

90469046
ssize_t optCastConstantSmall(ssize_t iconVal, var_types smallType);
90479047

@@ -9099,6 +9099,7 @@ class Compiler
90999099
GenTree* optNonNullAssertionProp_Call(ASSERT_VALARG_TP assertions, GenTreeCall* call);
91009100
bool optNonNullAssertionProp_Ind(ASSERT_VALARG_TP assertions, GenTree* indir);
91019101
bool optWriteBarrierAssertionProp_StoreInd(ASSERT_VALARG_TP assertions, GenTreeStoreInd* indir);
9102+
bool optWriteBarrierAssertionProp_StoreBlk(ASSERT_VALARG_TP assertions, GenTreeBlk* store);
91029103

91039104
enum class AssertVisit
91049105
{

src/coreclr/jit/gentree.h

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -8134,7 +8134,7 @@ struct GenTreeBlk : public GenTreeIndir
81348134

81358135
bool IsOnHeapAndContainsReferences()
81368136
{
8137-
return ContainsReferences() && !Addr()->OperIs(GT_LCL_ADDR);
8137+
return ContainsReferences() && !Addr()->OperIs(GT_LCL_ADDR) && ((gtFlags & GTF_IND_TGT_NOT_HEAP) == 0);
81388138
}
81398139

81408140
bool IsZeroingGcPointersOnHeap()

0 commit comments

Comments
 (0)