mirror of https://github.com/dotnet/runtime
JIT: Update GTF_GLOB_REF after last-use copy omission (#81430)
Morph is responsible for marking accesses of address-exposed locals with GTF_GLOB_REF, however the last-use copy elision can newly address expose a preexisting local. This means previous uses that morph already saw have not had GTF_GLOB_REF added. This updates the JIT to at least add GTF_GLOB_REF on locals within the same statement as the call. This is not a complete fix as nothing prevents downstream phases from reordering the call with uses from earlier statements, but this at least fixes the one issue we know of right now (which is itself stress-only). A more complete fix will likely entail moving the identifying of last-use candidates earlier and address exposing them at that point. However, that first requires ABI determination to be moved earlier, so this depends on #76926, which I expect to get to within the near future. Fix #81049
This commit is contained in:
parent
eadf678c8e
commit
58176f5c52
|
@ -4534,6 +4534,7 @@ public:
|
|||
|
||||
bool fgRemoveRestOfBlock; // true if we know that we will throw
|
||||
bool fgStmtRemoved; // true if we remove statements -> need new DFA
|
||||
bool fgRemarkGlobalUses; // true if morph should remark global uses after processing a statement
|
||||
|
||||
enum FlowGraphOrder
|
||||
{
|
||||
|
@ -5820,6 +5821,7 @@ private:
|
|||
GenTreeCall* fgMorphArgs(GenTreeCall* call);
|
||||
|
||||
void fgMakeOutgoingStructArgCopy(GenTreeCall* call, CallArg* arg);
|
||||
void fgMarkGlobalUses(Statement* stmt);
|
||||
|
||||
GenTree* fgMorphLocal(GenTreeLclVarCommon* lclNode);
|
||||
#ifdef TARGET_X86
|
||||
|
|
|
@ -3981,7 +3981,7 @@ void Compiler::fgMakeOutgoingStructArgCopy(GenTreeCall* call, CallArg* arg)
|
|||
//
|
||||
bool omitCopy = call->IsTailCall();
|
||||
|
||||
if (!omitCopy && fgDidEarlyLiveness)
|
||||
if (!omitCopy && fgGlobalMorph)
|
||||
{
|
||||
omitCopy = !varDsc->lvPromoted && ((lcl->gtFlags & GTF_VAR_DEATH) != 0);
|
||||
}
|
||||
|
@ -4016,6 +4016,17 @@ void Compiler::fgMakeOutgoingStructArgCopy(GenTreeCall* call, CallArg* arg)
|
|||
|
||||
// Copy prop could allow creating another later use of lcl if there are live assertions about it.
|
||||
fgKillDependentAssertions(varNum DEBUGARG(lcl));
|
||||
|
||||
// We may have seen previous uses of this local already,
|
||||
// and those uses should now be marked as GTF_GLOB_REF
|
||||
// since otherwise they could be reordered with the call.
|
||||
// The only known issues are under stress mode and happen
|
||||
// in the same statement, so we only handle this case currently.
|
||||
// TODO: A more complete fix will likely entail identifying
|
||||
// these candidates before morph and address exposing them
|
||||
// at that point, which first requires ABI determination to
|
||||
// be moved earlier.
|
||||
fgRemarkGlobalUses = true;
|
||||
}
|
||||
|
||||
JITDUMP("did not need to make outgoing copy for last use of V%02d\n", varNum);
|
||||
|
@ -4096,6 +4107,51 @@ void Compiler::fgMakeOutgoingStructArgCopy(GenTreeCall* call, CallArg* arg)
|
|||
arg->SetEarlyNode(argNode);
|
||||
}
|
||||
|
||||
//------------------------------------------------------------------------
|
||||
// fgMarkNewlyGlobalUses: Given a local that is newly address exposed, add
|
||||
// GTF_GLOB_REF whever necessary in the specified statement.
|
||||
//
|
||||
// Arguments:
|
||||
// stmt - The statement
|
||||
// lclNum - The local that is newly address exposed
|
||||
//
|
||||
// Notes:
|
||||
// See comment in fgMakeOutgoingStructArgCopy.
|
||||
//
|
||||
void Compiler::fgMarkGlobalUses(Statement* stmt)
|
||||
{
|
||||
struct Visitor : GenTreeVisitor<Visitor>
|
||||
{
|
||||
enum
|
||||
{
|
||||
DoPostOrder = true,
|
||||
};
|
||||
|
||||
Visitor(Compiler* comp) : GenTreeVisitor(comp)
|
||||
{
|
||||
}
|
||||
|
||||
fgWalkResult PostOrderVisit(GenTree** use, GenTree* user)
|
||||
{
|
||||
GenTree* node = *use;
|
||||
if (node->OperIsLocal() && m_compiler->lvaGetDesc(node->AsLclVarCommon()->GetLclNum())->IsAddressExposed())
|
||||
{
|
||||
node->gtFlags |= GTF_GLOB_REF;
|
||||
}
|
||||
|
||||
if (user != nullptr)
|
||||
{
|
||||
user->gtFlags |= node->gtFlags & GTF_GLOB_REF;
|
||||
}
|
||||
|
||||
return WALK_CONTINUE;
|
||||
}
|
||||
};
|
||||
|
||||
Visitor visitor(this);
|
||||
visitor.WalkTree(stmt->GetRootNodePointer(), nullptr);
|
||||
}
|
||||
|
||||
/*****************************************************************************
|
||||
*
|
||||
* A little helper used to rearrange nested commutative operations. The
|
||||
|
@ -13349,6 +13405,7 @@ bool Compiler::fgMorphBlockStmt(BasicBlock* block, Statement* stmt DEBUGARG(cons
|
|||
void Compiler::fgMorphStmts(BasicBlock* block)
|
||||
{
|
||||
fgRemoveRestOfBlock = false;
|
||||
fgRemarkGlobalUses = false;
|
||||
|
||||
for (Statement* const stmt : block->Statements())
|
||||
{
|
||||
|
@ -13463,6 +13520,12 @@ void Compiler::fgMorphStmts(BasicBlock* block)
|
|||
|
||||
stmt->SetRootNode(morphedTree);
|
||||
|
||||
if (fgRemarkGlobalUses)
|
||||
{
|
||||
fgMarkGlobalUses(stmt);
|
||||
fgRemarkGlobalUses = false;
|
||||
}
|
||||
|
||||
if (fgRemoveRestOfBlock)
|
||||
{
|
||||
continue;
|
||||
|
|
Loading…
Reference in New Issue