Skip to content
Merged
Show file tree
Hide file tree
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
39 changes: 30 additions & 9 deletions src/coreclr/jit/assertionprop.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -5207,9 +5207,10 @@ bool Compiler::optNonNullAssertionProp_Ind(ASSERT_VALARG_TP assertions, GenTree*
// Return Value:
// Exact type of write barrier required for the given address.
//
static GCInfo::WriteBarrierForm GetWriteBarrierForm(ValueNumStore* vnStore, ValueNum vn)
static GCInfo::WriteBarrierForm GetWriteBarrierForm(Compiler* comp, ValueNum vn)
{
const var_types type = vnStore->TypeOfVN(vn);
ValueNumStore* vnStore = comp->vnStore;
const var_types type = vnStore->TypeOfVN(vn);
if (type == TYP_REF)
{
return GCInfo::WriteBarrierForm::WBF_BarrierUnchecked;
Expand Down Expand Up @@ -5250,18 +5251,38 @@ static GCInfo::WriteBarrierForm GetWriteBarrierForm(ValueNumStore* vnStore, Valu
//
if (vnStore->IsVNConstantNonHandle(funcApp.m_args[0]))
{
return GetWriteBarrierForm(vnStore, funcApp.m_args[1]);
return GetWriteBarrierForm(comp, funcApp.m_args[1]);
}
if (vnStore->IsVNConstantNonHandle(funcApp.m_args[1]))
{
return GetWriteBarrierForm(vnStore, funcApp.m_args[0]);
return GetWriteBarrierForm(comp, funcApp.m_args[0]);
}
}
}
if (funcApp.m_func == VNF_InitVal)
{
unsigned lclNum = vnStore->CoercedConstantValue<unsigned>(funcApp.m_args[0]);
assert(lclNum != BAD_VAR_NUM);
CORINFO_CLASS_HANDLE srcCls = NO_CLASS_HANDLE;

// TODO:
// * addr is ByRefLike - NoBarrier (https://github.com/dotnet/runtime/issues/9512)
//
if (comp->compMethodHasRetVal() && (lclNum == comp->info.compRetBuffArg))
{
// See if the address is in current method's return buffer
// while the return type is a byref-like type.
srcCls = comp->info.compMethodInfo->args.retTypeClass;
}
else if (lclNum == comp->info.compThisArg)
{
// Same for implicit "this" parameter
assert(!comp->info.compIsStatic);
srcCls = comp->info.compClassHnd;
}

if ((srcCls != NO_CLASS_HANDLE) && comp->eeIsByrefLike(srcCls))
{
return GCInfo::WriteBarrierForm::WBF_NoBarrier;
}
}
}
return GCInfo::WriteBarrierForm::WBF_BarrierUnknown;
}

Expand Down Expand Up @@ -5315,7 +5336,7 @@ bool Compiler::optWriteBarrierAssertionProp_StoreInd(ASSERT_VALARG_TP assertions
{
// NOTE: we might want to inspect indirs with GTF_IND_TGT_HEAP flag as well - what if we can prove
// that they actually need no barrier? But that comes with a TP regression.
barrierType = GetWriteBarrierForm(vnStore, addr->gtVNPair.GetConservative());
barrierType = GetWriteBarrierForm(this, addr->gtVNPair.GetConservative());
}

JITDUMP("Trying to determine the exact type of write barrier for STOREIND [%d06]: ", dspTreeID(indir));
Expand Down
3 changes: 1 addition & 2 deletions src/coreclr/jit/codegenarm.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -811,8 +811,7 @@ void CodeGen::genCodeForCpObj(GenTreeBlk* cpObjNode)
sourceIsLocal = true;
}

bool dstOnStack =
dstAddr->gtSkipReloadOrCopy()->OperIs(GT_LCL_ADDR) || cpObjNode->GetLayout()->IsStackOnly(compiler);
bool dstOnStack = cpObjNode->IsAddressNotOnHeap(compiler);

#ifdef DEBUG
assert(!dstAddr->isContained());
Expand Down
3 changes: 1 addition & 2 deletions src/coreclr/jit/codegenarm64.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -3665,8 +3665,7 @@ void CodeGen::genCodeForCpObj(GenTreeBlk* cpObjNode)
sourceIsLocal = true;
}

bool dstOnStack =
dstAddr->gtSkipReloadOrCopy()->OperIs(GT_LCL_ADDR) || cpObjNode->GetLayout()->IsStackOnly(compiler);
bool dstOnStack = cpObjNode->IsAddressNotOnHeap(compiler);

#ifdef DEBUG
assert(!dstAddr->isContained());
Expand Down
3 changes: 1 addition & 2 deletions src/coreclr/jit/codegenloongarch64.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -2233,8 +2233,7 @@ void CodeGen::genCodeForCpObj(GenTreeBlk* cpObjNode)
sourceIsLocal = true;
}

bool dstOnStack =
dstAddr->gtSkipReloadOrCopy()->OperIs(GT_LCL_ADDR) || cpObjNode->GetLayout()->IsStackOnly(compiler);
bool dstOnStack = cpObjNode->IsAddressNotOnHeap(compiler);

#ifdef DEBUG
assert(!dstAddr->isContained());
Expand Down
3 changes: 1 addition & 2 deletions src/coreclr/jit/codegenriscv64.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -2153,8 +2153,7 @@ void CodeGen::genCodeForCpObj(GenTreeBlk* cpObjNode)
sourceIsLocal = true;
}

bool dstOnStack =
dstAddr->gtSkipReloadOrCopy()->OperIs(GT_LCL_ADDR) || cpObjNode->GetLayout()->IsStackOnly(compiler);
bool dstOnStack = cpObjNode->IsAddressNotOnHeap(compiler);

#ifdef DEBUG
assert(!dstAddr->isContained());
Expand Down
3 changes: 1 addition & 2 deletions src/coreclr/jit/codegenxarch.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -4218,8 +4218,7 @@ void CodeGen::genCodeForCpObj(GenTreeBlk* cpObjNode)
GenTree* dstAddr = cpObjNode->Addr();
GenTree* source = cpObjNode->Data();
var_types srcAddrType = TYP_BYREF;
bool dstOnStack =
dstAddr->gtSkipReloadOrCopy()->OperIs(GT_LCL_ADDR) || cpObjNode->GetLayout()->IsStackOnly(compiler);
bool dstOnStack = cpObjNode->IsAddressNotOnHeap(compiler);

// If the GenTree node has data about GC pointers, this means we're dealing
// with CpObj, so this requires special logic.
Expand Down
20 changes: 20 additions & 0 deletions src/coreclr/jit/gentree.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -18244,6 +18244,26 @@ bool GenTree::isIndir() const
return OperGet() == GT_IND || OperGet() == GT_STOREIND;
}

bool GenTreeIndir::IsAddressNotOnHeap(Compiler* comp)
{
if (OperIs(GT_STOREIND, GT_STORE_BLK) && ((gtFlags & GTF_IND_TGT_NOT_HEAP) != 0))
{
return true;
}

if (HasBase() && Base()->gtSkipReloadOrCopy()->OperIs(GT_LCL_ADDR))
{
return true;
}

if (OperIs(GT_STORE_BLK) && AsBlk()->GetLayout()->IsStackOnly(comp))
{
return true;
}

return false;
}

bool GenTreeIndir::HasBase()
{
return Base() != nullptr;
Expand Down
1 change: 1 addition & 0 deletions src/coreclr/jit/gentree.h
Original file line number Diff line number Diff line change
Expand Up @@ -7941,6 +7941,7 @@ struct GenTreeIndir : public GenTreeOp
return gtOp2;
}

bool IsAddressNotOnHeap(Compiler* comp);
bool HasBase();
bool HasIndex();
GenTree* Base();
Expand Down
9 changes: 8 additions & 1 deletion src/coreclr/jit/importer.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -10607,7 +10607,14 @@ void Compiler::impImportBlockCode(BasicBlock* block)
op1 = impPopStack().val; // Ptr
assertImp(varTypeIsStruct(op2));

op1 = gtNewStoreValueNode(layout, op1, op2, impPrefixFlagsToIndirFlags(prefixFlags));
GenTreeFlags indirFlags = impPrefixFlagsToIndirFlags(prefixFlags);
if (eeIsByrefLike(resolvedToken.hClass))
{
indirFlags |= GTF_IND_TGT_NOT_HEAP;
}

op1 = gtNewStoreValueNode(layout, op1, op2, indirFlags);

op1 = impStoreStruct(op1, CHECK_SPILL_ALL);
goto SPILL_APPEND;
}
Expand Down
2 changes: 1 addition & 1 deletion src/coreclr/jit/lowerarmarch.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -762,7 +762,7 @@ void Lowering::LowerBlockStore(GenTreeBlk* blkNode)
{
// No write barriers are needed on the stack.
// If the layout contains a byref, then we know it must live on the stack.
if (dstAddr->OperIs(GT_LCL_ADDR) || layout->IsStackOnly(comp))
if (blkNode->IsAddressNotOnHeap(comp))
{
// If the size is small enough to unroll then we need to mark the block as non-interruptible
// to actually allow unrolling. The generated code does not report GC references loaded in the
Expand Down
2 changes: 1 addition & 1 deletion src/coreclr/jit/lowerloongarch64.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -362,7 +362,7 @@ void Lowering::LowerBlockStore(GenTreeBlk* blkNode)
{
// No write barriers are needed on the stack.
// If the layout contains a byref, then we know it must live on the stack.
if (dstAddr->OperIs(GT_LCL_ADDR) || layout->IsStackOnly(comp))
if (blkNode->IsAddressNotOnHeap(comp))
{
// If the size is small enough to unroll then we need to mark the block as non-interruptible
// to actually allow unrolling. The generated code does not report GC references loaded in the
Expand Down
2 changes: 1 addition & 1 deletion src/coreclr/jit/lowerriscv64.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -311,7 +311,7 @@ void Lowering::LowerBlockStore(GenTreeBlk* blkNode)
{
// No write barriers are needed on the stack.
// If the layout contains a byref, then we know it must live on the stack.
if (dstAddr->OperIs(GT_LCL_ADDR) || layout->IsStackOnly(comp))
if (blkNode->IsAddressNotOnHeap(comp))
{
// If the size is small enough to unroll then we need to mark the block as non-interruptible
// to actually allow unrolling. The generated code does not report GC references loaded in the
Expand Down
4 changes: 2 additions & 2 deletions src/coreclr/jit/lowerxarch.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -450,7 +450,7 @@ void Lowering::LowerBlockStore(GenTreeBlk* blkNode)
{
// No write barriers are needed on the stack.
// If the layout is byref-like, then we know it must live on the stack.
if (dstAddr->OperIs(GT_LCL_ADDR) || layout->IsStackOnly(comp))
if (blkNode->IsAddressNotOnHeap(comp))
{
// If the size is small enough to unroll then we need to mark the block as non-interruptible
// to actually allow unrolling. The generated code does not report GC references loaded in the
Expand All @@ -477,7 +477,7 @@ void Lowering::LowerBlockStore(GenTreeBlk* blkNode)
// the entire operation takes 20 cycles and encodes in 5 bytes (loading RCX and REP MOVSD/Q).
unsigned nonGCSlots = 0;

if (dstAddr->OperIs(GT_LCL_ADDR) || layout->IsStackOnly(comp))
if (blkNode->IsAddressNotOnHeap(comp))
{
// If the destination is on the stack then no write barriers are needed.
nonGCSlots = layout->GetSlotCount();
Expand Down
9 changes: 7 additions & 2 deletions src/coreclr/jit/morphblock.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1358,8 +1358,13 @@ GenTree* MorphCopyBlockHelper::CopyFieldByField()
}
else
{
GenTree* fldAddr = grabAddr(srcFieldOffset);
dstFldStore = m_comp->gtNewStoreIndNode(srcType, fldAddr, srcFld);
GenTree* fldAddr = grabAddr(srcFieldOffset);
GenTreeFlags indirFlags = GTF_EMPTY;
if (m_store->OperIs(GT_STORE_BLK, GT_STOREIND))
{
indirFlags = m_store->gtFlags & (GTF_IND_TGT_NOT_HEAP | GTF_IND_TGT_HEAP);
}
dstFldStore = m_comp->gtNewStoreIndNode(srcType, fldAddr, srcFld, indirFlags);

@EgorBo EgorBo Jan 20, 2025

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Here we were losing GTF_IND_TGT* flags when we promote GT_STORE_BLK to field-wise GT_STOREINDs

}
}
}
Expand Down