Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

JIT: Update GTF_GLOB_REF after last-use copy omission #81430

Merged
merged 2 commits into from
Feb 1, 2023
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
2 changes: 2 additions & 0 deletions src/coreclr/jit/compiler.h
Original file line number Diff line number Diff line change
Expand Up @@ -4534,6 +4534,7 @@ class Compiler

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
{
Expand Down Expand Up @@ -5818,6 +5819,7 @@ class Compiler
GenTreeCall* fgMorphArgs(GenTreeCall* call);

void fgMakeOutgoingStructArgCopy(GenTreeCall* call, CallArg* arg);
void fgMarkGlobalUses(Statement* stmt);

GenTree* fgMorphLocal(GenTreeLclVarCommon* lclNode);
#ifdef TARGET_X86
Expand Down
65 changes: 64 additions & 1 deletion src/coreclr/jit/morph.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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);
}
Expand Down Expand Up @@ -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);
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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())
{
Expand Down Expand Up @@ -13463,6 +13520,12 @@ void Compiler::fgMorphStmts(BasicBlock* block)

stmt->SetRootNode(morphedTree);

if (fgRemarkGlobalUses)
{
fgMarkGlobalUses(stmt);
fgRemarkGlobalUses = false;
}

if (fgRemoveRestOfBlock)
{
continue;
Expand Down