From 390185cb244d3fb661a1f4b6197a7a852924ff45 Mon Sep 17 00:00:00 2001 From: Jeremy Koritzinsky Date: Mon, 5 Aug 2024 19:45:52 -0700 Subject: [PATCH] Call copy helper in lowering and heavily reduce changes in higher layers --- src/coreclr/jit/gentree.cpp | 32 ++-------- src/coreclr/jit/gschecks.cpp | 46 ++++----------- src/coreclr/jit/lower.cpp | 110 +++++++++++++++++++++++++++++++++-- src/coreclr/jit/lower.h | 2 + 4 files changed, 125 insertions(+), 65 deletions(-) diff --git a/src/coreclr/jit/gentree.cpp b/src/coreclr/jit/gentree.cpp index 296b3670f5100..3e7898054fa91 100644 --- a/src/coreclr/jit/gentree.cpp +++ b/src/coreclr/jit/gentree.cpp @@ -16678,36 +16678,14 @@ GenTree* Compiler::gtNewTempStore( compFloatingPointUsed = true; } - GenTree* store; - if (varDsc->lvRequiresSpecialCopy) - { - JITDUMP("Var V%02u requires special copy\n", tmp); - CORINFO_METHOD_HANDLE copyHelper = - info.compCompHnd->GetSpecialCopyHelper(varDsc->GetLayout()->GetClassHandle()); - GenTreeCall* call = gtNewCallNode(CT_USER_FUNC, copyHelper, TYP_VOID); - - GenTree* src; + GenTree* store = gtNewStoreLclVarNode(tmp, val); - assert(val->OperIs(GT_BLK)); - src = val->AsBlk()->Addr(); + // TODO-ASG: delete this zero-diff quirk. Requires some forward substitution work. + store->gtType = dstTyp; - GenTree* dst = gtNewLclVarAddrNode(tmp); - - call->gtArgs.PushBack(this, NewCallArg::Primitive(dst)); - call->gtArgs.PushBack(this, NewCallArg::Primitive(src)); - store = call; - } - else + if (varTypeIsStruct(varDsc) && !val->IsInitVal()) { - store = gtNewStoreLclVarNode(tmp, val); - - // TODO-ASG: delete this zero-diff quirk. Requires some forward substitution work. - store->gtType = dstTyp; - - if (varTypeIsStruct(varDsc) && !val->IsInitVal()) - { - store = impStoreStruct(store, curLevel, pAfterStmt, di, block); - } + store = impStoreStruct(store, curLevel, pAfterStmt, di, block); } return store; diff --git a/src/coreclr/jit/gschecks.cpp b/src/coreclr/jit/gschecks.cpp index 72b68b5915b97..f40b710325e79 100644 --- a/src/coreclr/jit/gschecks.cpp +++ b/src/coreclr/jit/gschecks.cpp @@ -516,46 +516,24 @@ void Compiler::gsParamsToShadows() continue; } - if (varDsc->lvRequiresSpecialCopy) - { - JITDUMP("Var V%02u requires special copy, using special copy helper to copy to shadow var V%02u\n", lclNum, - shadowVarNum); - CORINFO_METHOD_HANDLE copyHelper = - info.compCompHnd->GetSpecialCopyHelper(varDsc->GetLayout()->GetClassHandle()); - GenTreeCall* call = gtNewCallNode(CT_USER_FUNC, copyHelper, TYP_VOID); + GenTree* src = gtNewLclvNode(lclNum, varDsc->TypeGet()); + src->gtFlags |= GTF_DONT_CSE; - GenTree* src = gtNewLclVarAddrNode(lclNum); - GenTree* dst = gtNewLclVarAddrNode(shadowVarNum); + GenTree* store = gtNewStoreLclVarNode(shadowVarNum, src); - call->gtArgs.PushBack(this, NewCallArg::Primitive(dst)); - call->gtArgs.PushBack(this, NewCallArg::Primitive(src)); + fgEnsureFirstBBisScratch(); + compCurBB = fgFirstBB; // Needed by some morphing - fgEnsureFirstBBisScratch(); - compCurBB = fgFirstBB; // Needed by some morphing - if (opts.IsReversePInvoke()) - { - JITDUMP( - "Inserting special copy helper call at the end of the first block after Reverse P/Invoke transition\n"); - // If we are in a reverse P/Invoke, then we need to insert - // the call at the end of the first block as we need to do the GC transition - // before we can call the helper. - (void)fgNewStmtAtEnd(fgFirstBB, fgMorphTree(call)); - } - else - { - JITDUMP("Inserting special copy helper call at the beginning of the first block\n"); - (void)fgNewStmtAtBeg(fgFirstBB, fgMorphTree(call)); - } + if (varDsc->lvRequiresSpecialCopy && opts.IsReversePInvoke()) + { + // If we are in a reverse P/Invoke, then we need to insert + // the call at the end of the first block as we need to do the GC transition + // before we do the copy. We may end up calling a managed helper. + (void)fgNewStmtAtEnd(fgFirstBB, fgMorphTree(store)); } else { - GenTree* src = gtNewLclvNode(lclNum, varDsc->TypeGet()); - src->gtFlags |= GTF_DONT_CSE; - - GenTree* store = gtNewStoreLclVarNode(shadowVarNum, src); - - fgEnsureFirstBBisScratch(); - compCurBB = fgFirstBB; // Needed by some morphing + (void)fgNewStmtAtBeg(fgFirstBB, fgMorphTree(store)); } } diff --git a/src/coreclr/jit/lower.cpp b/src/coreclr/jit/lower.cpp index 88f1b1297bfdb..3817c1b4a8e88 100644 --- a/src/coreclr/jit/lower.cpp +++ b/src/coreclr/jit/lower.cpp @@ -4991,7 +4991,14 @@ GenTree* Lowering::LowerStoreLocCommon(GenTreeLclVarCommon* lclStore) } #endif // TARGET_XARCH } - convertToStoreObj = false; + if (varDsc->lvRequiresSpecialCopy) + { + convertToStoreObj = true; + } + else + { + convertToStoreObj = false; + } #else // TARGET_ARM64 // This optimization on arm64 allows more SIMD16 vars to be enregistered but it could cause // regressions when there are many calls and before/after each one we have to store/save the upper @@ -8670,6 +8677,92 @@ void Lowering::LowerBlockStoreAsHelperCall(GenTreeBlk* blkNode) #endif } +//------------------------------------------------------------------------ +// LowerBlockStoreWithSpecialCopyHelper: Lower a block store node using a special copy helper +// +// Arguments: +// blkNode - The block store node to lower +// +void Lowering::LowerBlockStoreWithSpecialCopyHelper(GenTreeBlk* blkNode) +{ + LIR::Use use; + assert(!BlockRange().TryGetUse(blkNode, &use)); + + const bool isVolatile = blkNode->IsVolatile(); + + GenTree* dest = blkNode->Addr(); + GenTree* data = blkNode->Data(); + + if (data->OperIs(GT_IND)) + { + // Drop GT_IND nodes + BlockRange().Remove(data); + data = data->AsIndir()->Addr(); + } + else + { + assert(data->OperIs(GT_LCL_VAR, GT_LCL_FLD)); + + // Convert local to LCL_ADDR + unsigned lclOffset = data->AsLclVarCommon()->GetLclOffs(); + + data->ChangeOper(GT_LCL_ADDR); + data->ChangeType(TYP_I_IMPL); + data->AsLclFld()->SetLclOffs(lclOffset); + data->ClearContained(); + } + + // A hacky way to safely call fgMorphTree in Lower + GenTree* destPlaceholder = comp->gtNewZeroConNode(dest->TypeGet()); + GenTree* dataPlaceholder = comp->gtNewZeroConNode(genActualType(data)); + + GenTreeCall* call = comp->gtNewCallNode(CT_USER_FUNC, comp->info.compCompHnd->GetSpecialCopyHelper(blkNode->GetLayout()->GetClassHandle()), TYP_VOID); + call->gtArgs.PushBack(comp, NewCallArg::Primitive(destPlaceholder)); + call->gtArgs.PushBack(comp, NewCallArg::Primitive(dataPlaceholder)); + + comp->fgMorphArgs(call); + + LIR::Range range = LIR::SeqTree(comp, call); + GenTree* rangeStart = range.FirstNode(); + GenTree* rangeEnd = range.LastNode(); + + BlockRange().InsertBefore(blkNode, std::move(range)); + blkNode->gtBashToNOP(); + + LIR::Use destUse; + BlockRange().TryGetUse(destPlaceholder, &destUse); + destUse.ReplaceWith(dest); + destPlaceholder->SetUnusedValue(); + + LIR::Use dataUse; + BlockRange().TryGetUse(dataPlaceholder, &dataUse); + dataUse.ReplaceWith(data); + dataPlaceholder->SetUnusedValue(); + + LowerRange(rangeStart, rangeEnd); + + // Finally move all GT_PUTARG_* nodes + // Re-use the existing logic for CFG call args here + MoveCFGCallArgs(call); + + BlockRange().Remove(destPlaceholder); + BlockRange().Remove(dataPlaceholder); + +// Wrap with memory barriers on weak memory models +// if the block store was volatile +#ifndef TARGET_XARCH + if (isVolatile) + { + GenTree* firstBarrier = comp->gtNewMemoryBarrier(); + GenTree* secondBarrier = comp->gtNewMemoryBarrier(/*loadOnly*/ true); + BlockRange().InsertBefore(call, firstBarrier); + BlockRange().InsertAfter(call, secondBarrier); + LowerNode(firstBarrier); + LowerNode(secondBarrier); + } +#endif +} + //------------------------------------------------------------------------ // GetLoadStoreCoalescingData: given a STOREIND/IND node, get the data needed to perform // store/load coalescing including pointer to the previous node. @@ -10023,11 +10116,20 @@ void Lowering::LowerBlockStoreCommon(GenTreeBlk* blkNode) assert(blkNode->Data()->IsIntegralConst(0)); } + GenTree* src = blkNode->Data(); + // Lose the type information stored in the source - we no longer need it. - if (blkNode->Data()->OperIs(GT_BLK)) + if (src->OperIs(GT_BLK)) { - blkNode->Data()->SetOper(GT_IND); - LowerIndir(blkNode->Data()->AsIndir()); + src->SetOper(GT_IND); + LowerIndir(src->AsIndir()); + } + + if ((src->OperIsIndir() && src->AsIndir()->Addr()->OperIsLocalRead() && comp->lvaTable[src->AsIndir()->Addr()->AsLclVarCommon()->GetLclNum()].lvRequiresSpecialCopy) + || (src->OperIsLocalRead() && comp->lvaTable[src->AsLclVarCommon()->GetLclNum()].lvRequiresSpecialCopy)) + { + LowerBlockStoreWithSpecialCopyHelper(blkNode); + return; } if (TryTransformStoreObjAsStoreInd(blkNode)) diff --git a/src/coreclr/jit/lower.h b/src/coreclr/jit/lower.h index 5a7214822d53b..62b50fab0a474 100644 --- a/src/coreclr/jit/lower.h +++ b/src/coreclr/jit/lower.h @@ -360,6 +360,7 @@ class Lowering final : public Phase void MarkTree(GenTree* root); void UnmarkTree(GenTree* root); GenTree* LowerStoreIndir(GenTreeStoreInd* node); + GenTree* LowerStoreIndirWithSpecialCopyHelper(GenTreeStoreInd* node); void LowerStoreIndirCoalescing(GenTreeIndir* node); GenTree* LowerAdd(GenTreeOp* node); GenTree* LowerMul(GenTreeOp* mul); @@ -371,6 +372,7 @@ class Lowering final : public Phase void LowerBlockStore(GenTreeBlk* blkNode); void LowerBlockStoreCommon(GenTreeBlk* blkNode); void LowerBlockStoreAsHelperCall(GenTreeBlk* blkNode); + void LowerBlockStoreWithSpecialCopyHelper(GenTreeBlk* blkNode); bool TryLowerBlockStoreAsGcBulkCopyCall(GenTreeBlk* blkNode); void LowerLclHeap(GenTree* node); void ContainBlockStoreAddress(GenTreeBlk* blkNode, unsigned size, GenTree* addr, GenTree* addrParent);