Skip to content

Commit ae9f1ca

Browse files
authored
Decouple call store operands from local ret buf optimization (#68469)
* Move GenTreeCall::GetLclRetBufArgNode -> Compiler::gtCallGetDefinedRetBufLclAddr and change it to not try to look into stores * Assert that the node we are returning actually defines a local that has lvHiddenBufferStructArg set * Assert that call morphing does not break our recognition of the defined local when optimizing * Update Compiler::DefinesLocalAddr to check for GT_LCL_FLD_ADDR
1 parent 1bc09e7 commit ae9f1ca

File tree

7 files changed

+69
-46
lines changed

7 files changed

+69
-46
lines changed

src/coreclr/jit/compiler.h

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2790,6 +2790,8 @@ class Compiler
27902790
// Check if this tree is a gc static base helper call
27912791
bool gtIsStaticGCBaseHelperCall(GenTree* tree);
27922792

2793+
GenTree* gtCallGetDefinedRetBufLclAddr(GenTreeCall* call);
2794+
27932795
//-------------------------------------------------------------------------
27942796
// Functions to display the trees
27952797

src/coreclr/jit/gentree.cpp

Lines changed: 54 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -6273,7 +6273,7 @@ bool GenTree::OperRequiresAsgFlag()
62736273
{
62746274
// If the call has return buffer argument, it produced a definition and hence
62756275
// should be marked with assignment.
6276-
return AsCall()->GetLclRetBufArgNode() != nullptr;
6276+
return AsCall()->IsOptimizingRetBufAsLocal();
62776277
}
62786278
return false;
62796279
}
@@ -16138,7 +16138,7 @@ bool GenTree::DefinesLocal(Compiler* comp, GenTreeLclVarCommon** pLclVarTree, bo
1613816138
}
1613916139
else if (OperIs(GT_CALL))
1614016140
{
16141-
GenTree* retBufArg = AsCall()->GetLclRetBufArgNode();
16141+
GenTree* retBufArg = comp->gtCallGetDefinedRetBufLclAddr(AsCall());
1614216142
if (retBufArg == nullptr)
1614316143
{
1614416144
return false;
@@ -16180,7 +16180,7 @@ bool GenTree::DefinesLocal(Compiler* comp, GenTreeLclVarCommon** pLclVarTree, bo
1618016180
// Returns true if this GenTree defines a result which is based on the address of a local.
1618116181
bool GenTree::DefinesLocalAddr(Compiler* comp, unsigned width, GenTreeLclVarCommon** pLclVarTree, bool* pIsEntire)
1618216182
{
16183-
if (OperGet() == GT_ADDR || OperGet() == GT_LCL_VAR_ADDR)
16183+
if (OperIs(GT_ADDR, GT_LCL_VAR_ADDR, GT_LCL_FLD_ADDR))
1618416184
{
1618516185
GenTree* addrArg = this;
1618616186
if (OperGet() == GT_ADDR)
@@ -17864,6 +17864,57 @@ bool Compiler::gtIsStaticGCBaseHelperCall(GenTree* tree)
1786417864
return false;
1786517865
}
1786617866

17867+
//------------------------------------------------------------------------
17868+
// gtCallGetDefinedRetBufLclAddr:
17869+
// Get the tree corresponding to the address of the retbuf taht this call defines.
17870+
//
17871+
// Parameters:
17872+
// call - The call node
17873+
//
17874+
// Returns:
17875+
// A tree representing the address of a local.
17876+
//
17877+
// Remarks:
17878+
// This function should not be used until after morph when local address
17879+
// nodes have been normalized. However, before that IsOptimizingRetBufAsLocal
17880+
// can be used to at least check if the call has a retbuf that we are
17881+
// optimizing.
17882+
//
17883+
GenTree* Compiler::gtCallGetDefinedRetBufLclAddr(GenTreeCall* call)
17884+
{
17885+
if (!call->IsOptimizingRetBufAsLocal())
17886+
{
17887+
return nullptr;
17888+
}
17889+
17890+
CallArg* retBufArg = call->gtArgs.GetRetBufferArg();
17891+
assert(retBufArg != nullptr);
17892+
17893+
GenTree* node = retBufArg->GetNode();
17894+
switch (node->OperGet())
17895+
{
17896+
// Get the value from putarg wrapper nodes
17897+
case GT_PUTARG_REG:
17898+
case GT_PUTARG_STK:
17899+
node = node->AsOp()->gtGetOp1();
17900+
break;
17901+
17902+
default:
17903+
break;
17904+
}
17905+
17906+
// This may be called very late to check validity of LIR.
17907+
node = node->gtSkipReloadOrCopy();
17908+
17909+
#ifdef DEBUG
17910+
unsigned size = typGetObjLayout(call->gtRetClsHnd)->GetSize();
17911+
GenTreeLclVarCommon* lcl;
17912+
assert(node->DefinesLocalAddr(this, size, &lcl, nullptr) && lvaGetDesc(lcl)->lvHiddenBufferStructArg);
17913+
#endif
17914+
17915+
return node;
17916+
}
17917+
1786717918
//------------------------------------------------------------------------
1786817919
// ParseArrayAddress: Rehydrate the array and index expression from ARR_ADDR.
1786917920
//

src/coreclr/jit/gentree.h

Lines changed: 5 additions & 33 deletions
Original file line numberDiff line numberDiff line change
@@ -5035,6 +5035,11 @@ struct GenTreeCall final : public GenTree
50355035
return (gtCallMoreFlags & GTF_CALL_M_EXPANDED_EARLY) != 0;
50365036
}
50375037

5038+
bool IsOptimizingRetBufAsLocal()
5039+
{
5040+
return (gtCallMoreFlags & GTF_CALL_M_RETBUFFARG_LCLOPT) != 0;
5041+
}
5042+
50385043
//-----------------------------------------------------------------------------------------
50395044
// GetIndirectionCellArgKind: Get the kind of indirection cell used by this call.
50405045
//
@@ -5181,39 +5186,6 @@ struct GenTreeCall final : public GenTree
51815186
{
51825187
}
51835188
#endif
5184-
5185-
GenTree* GetLclRetBufArgNode()
5186-
{
5187-
if (!gtArgs.HasRetBuffer() || ((gtCallMoreFlags & GTF_CALL_M_RETBUFFARG_LCLOPT) == 0))
5188-
{
5189-
return nullptr;
5190-
}
5191-
5192-
CallArg* retBufArg = gtArgs.GetRetBufferArg();
5193-
GenTree* lclRetBufArgNode = retBufArg->GetEarlyNode();
5194-
if (lclRetBufArgNode == nullptr)
5195-
{
5196-
lclRetBufArgNode = retBufArg->GetLateNode();
5197-
}
5198-
5199-
switch (lclRetBufArgNode->OperGet())
5200-
{
5201-
// Get the true value from setup args
5202-
case GT_ASG:
5203-
return lclRetBufArgNode->AsOp()->gtGetOp2();
5204-
case GT_STORE_LCL_VAR:
5205-
return lclRetBufArgNode->AsUnOp()->gtGetOp1();
5206-
5207-
// Get the value from putarg wrapper nodes
5208-
case GT_PUTARG_REG:
5209-
case GT_PUTARG_STK:
5210-
return lclRetBufArgNode->AsOp()->gtGetOp1();
5211-
5212-
// Otherwise the node should be in the Use*
5213-
default:
5214-
return lclRetBufArgNode;
5215-
}
5216-
}
52175189
};
52185190

52195191
struct GenTreeCmpXchg : public GenTree

src/coreclr/jit/lclmorph.cpp

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -723,8 +723,8 @@ class LocalAddressVisitor final : public GenTreeVisitor<LocalAddressVisitor>
723723

724724
// TODO-ADDR: For now use LCL_VAR_ADDR and LCL_FLD_ADDR only as call arguments and assignment sources.
725725
// Other usages require more changes. For example, a tree like OBJ(ADD(ADDR(LCL_VAR), 4))
726-
// could be changed to OBJ(LCL_FLD_ADDR) but then DefinesLocalAddr does not recognize
727-
// LCL_FLD_ADDR (even though it does recognize LCL_VAR_ADDR).
726+
// could be changed to OBJ(LCL_FLD_ADDR) but historically DefinesLocalAddr did not recognize
727+
// LCL_FLD_ADDR (and there may be other things now as well).
728728
if (user->OperIs(GT_CALL, GT_ASG) && !hasHiddenStructArg)
729729
{
730730
MorphLocalAddress(val);

src/coreclr/jit/morph.cpp

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1782,6 +1782,10 @@ void CallArgs::EvalArgsToTemps(Compiler* comp, GenTreeCall* call)
17821782
lateTail = &arg.LateNextRef();
17831783
}
17841784

1785+
// Make sure we did not do anything here that stops us from being able to
1786+
// find the local ret buf if we are optimizing it.
1787+
noway_assert(!call->IsOptimizingRetBufAsLocal() || (comp->gtCallGetDefinedRetBufLclAddr(call) != nullptr));
1788+
17851789
#ifdef DEBUG
17861790
if (comp->verbose)
17871791
{

src/coreclr/jit/optimizer.cpp

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -5533,7 +5533,7 @@ Compiler::fgWalkResult Compiler::optIsVarAssgCB(GenTree** pTree, fgWalkData* dat
55335533
{
55345534
desc->ivaMaskCall = optCallInterf(tree->AsCall());
55355535

5536-
dest = tree->AsCall()->GetLclRetBufArgNode();
5536+
dest = data->compiler->gtCallGetDefinedRetBufLclAddr(tree->AsCall());
55375537
if (dest == nullptr)
55385538
{
55395539
return WALK_CONTINUE;

src/coreclr/jit/sideeffects.cpp

Lines changed: 1 addition & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -141,15 +141,9 @@ AliasSet::NodeInfo::NodeInfo(Compiler* compiler, GenTree* node)
141141
if (node->IsCall())
142142
{
143143
// For calls having return buffer, update the local number that is written after this call.
144-
GenTree* retBufArgNode = node->AsCall()->GetLclRetBufArgNode();
144+
GenTree* retBufArgNode = compiler->gtCallGetDefinedRetBufLclAddr(node->AsCall());
145145
if (retBufArgNode != nullptr)
146146
{
147-
// If a copy/reload is inserted by LSRA, retrieve the returnBuffer
148-
if (retBufArgNode->IsCopyOrReload())
149-
{
150-
retBufArgNode = retBufArgNode->AsCopyOrReload()->gtGetOp1();
151-
}
152-
153147
m_flags |= ALIAS_WRITES_LCL_VAR;
154148
m_lclNum = retBufArgNode->AsLclVarCommon()->GetLclNum();
155149
m_lclOffs = retBufArgNode->AsLclVarCommon()->GetLclOffs();

0 commit comments

Comments
 (0)