Skip to content

JIT: Clean up gtCloneExpr #97411

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.

Sign up for GitHub

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

Merged
merged 2 commits into from
Jan 25, 2024
Merged

JIT: Clean up gtCloneExpr #97411

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
ad9167b
JIT: Clean up gtCloneExpr
jakobbotsch Jan 23, 2024
432a0c9
Fix x86/arm32 assert
jakobbotsch Jan 24, 2024
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
7 changes: 2 additions & 5 deletions src/coreclr/jit/block.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -791,14 +791,11 @@ void* BasicBlock::MemoryPhiArg::operator new(size_t sz, Compiler* comp)
// compiler - Jit compiler instance
// to - New/empty block to copy statements into
// from - Block to copy statements from
// varNum - lclVar uses with lclNum `varNum` will be replaced; can be ~0 to indicate no replacement.
// varVal - If replacing uses of `varNum`, replace them with int constants with value `varVal`.
//
// Note:
// Leaves block ref count at zero, and pred edge list empty.
//
void BasicBlock::CloneBlockState(
Compiler* compiler, BasicBlock* to, const BasicBlock* from, unsigned varNum, int varVal)
void BasicBlock::CloneBlockState(Compiler* compiler, BasicBlock* to, const BasicBlock* from)
{
assert(to->bbStmtList == nullptr);
to->CopyFlags(from);
Expand All @@ -817,7 +814,7 @@ void BasicBlock::CloneBlockState(

for (Statement* const fromStmt : from->Statements())
{
GenTree* newExpr = compiler->gtCloneExpr(fromStmt->GetRootNode(), GTF_EMPTY, varNum, varVal);
GenTree* newExpr = compiler->gtCloneExpr(fromStmt->GetRootNode());
assert(newExpr != nullptr);
compiler->fgInsertStmtAtEnd(to, compiler->fgNewStmtFromTree(newExpr, fromStmt->GetDebugInfo()));
}
Expand Down
6 changes: 2 additions & 4 deletions src/coreclr/jit/block.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1647,10 +1647,8 @@ struct BasicBlock : private LIR::Range
return BBCompilerSuccList(comp, this);
}

// Clone block state and statements from `from` block to `to` block (which must be new/empty),
// optionally replacing uses of local `varNum` with IntCns `varVal`.
static void CloneBlockState(
Compiler* compiler, BasicBlock* to, const BasicBlock* from, unsigned varNum = (unsigned)-1, int varVal = 0);
// Clone block state and statements from `from` block to `to` block (which must be new/empty)
static void CloneBlockState(Compiler* compiler, BasicBlock* to, const BasicBlock* from);

// Copy the block kind and targets. The `from` block is untouched.
void CopyTarget(Compiler* compiler, const BasicBlock* from);
Expand Down
23 changes: 4 additions & 19 deletions src/coreclr/jit/compiler.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3406,21 +3406,8 @@ class Compiler

GenTree* gtClone(GenTree* tree, bool complexOK = false);

// If `tree` is a lclVar with lclNum `varNum`, return an IntCns with value `varVal`; otherwise,
// create a copy of `tree`, adding specified flags, replacing uses of lclVar `deepVarNum` with
// IntCnses with value `deepVarVal`.
GenTree* gtCloneExpr(
GenTree* tree, GenTreeFlags addFlags, unsigned varNum, int varVal, unsigned deepVarNum, int deepVarVal);

// Create a copy of `tree`, optionally adding specified flags, and optionally mapping uses of local
// `varNum` to int constants with value `varVal`.
GenTree* gtCloneExpr(GenTree* tree,
GenTreeFlags addFlags = GTF_EMPTY,
unsigned varNum = BAD_VAR_NUM,
int varVal = 0)
{
return gtCloneExpr(tree, addFlags, varNum, varVal, varNum, varVal);
}
// Create a copy of `tree`
GenTree* gtCloneExpr(GenTree* tree);

Statement* gtCloneStmt(Statement* stmt)
{
Expand All @@ -3429,10 +3416,7 @@ class Compiler
}

// Internal helper for cloning a call
GenTreeCall* gtCloneExprCallHelper(GenTreeCall* call,
GenTreeFlags addFlags = GTF_EMPTY,
unsigned deepVarNum = BAD_VAR_NUM,
int deepVarVal = 0);
GenTreeCall* gtCloneExprCallHelper(GenTreeCall* call);

// Create copy of an inline or guarded devirtualization candidate tree.
GenTreeCall* gtCloneCandidateCall(GenTreeCall* call);
Expand Down Expand Up @@ -6781,6 +6765,7 @@ class Compiler
void optCloneLoop(FlowGraphNaturalLoop* loop, LoopCloneContext* context);
PhaseStatus optUnrollLoops(); // Unrolls loops (needs to have cost info)
bool optTryUnrollLoop(FlowGraphNaturalLoop* loop, bool* changedIR);
void optReplaceScalarUsesWithConst(BasicBlock* block, unsigned lclNum, ssize_t cnsVal);
void optRemoveRedundantZeroInits();
PhaseStatus optIfConversion(); // If conversion

Expand Down
170 changes: 40 additions & 130 deletions src/coreclr/jit/gentree.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9085,29 +9085,15 @@ GenTree* Compiler::gtClone(GenTree* tree, bool complexOK)
}

//------------------------------------------------------------------------
// gtCloneExpr: Create a copy of `tree`, adding flags `addFlags`, mapping
// local `varNum` to int constant `varVal` if it appears at
// the root, and mapping uses of local `deepVarNum` to constant
// `deepVarVal` if they occur beyond the root.
// gtCloneExpr: Create a copy of `tree`
//
// Arguments:
// tree - GenTree to create a copy of
// addFlags - GTF_* flags to add to the copied tree nodes
// varNum - lclNum to replace at the root, or ~0 for no root replacement
// varVal - If replacing at root, replace local `varNum` with IntCns `varVal`
// deepVarNum - lclNum to replace uses of beyond the root, or ~0 for no replacement
// deepVarVal - If replacing beyond root, replace `deepVarNum` with IntCns `deepVarVal`
//
// Return Value:
// A copy of the given tree with the replacements and added flags specified.
//
// Notes:
// Top-level callers should generally call the overload that doesn't have
// the explicit `deepVarNum` and `deepVarVal` parameters; those are used in
// recursive invocations to avoid replacing defs.
// A copy of the given tree.

GenTree* Compiler::gtCloneExpr(
GenTree* tree, GenTreeFlags addFlags, unsigned varNum, int varVal, unsigned deepVarNum, int deepVarVal)
GenTree* Compiler::gtCloneExpr(GenTree* tree)
{
if (tree == nullptr)
{
Expand Down Expand Up @@ -9172,34 +9158,20 @@ GenTree* Compiler::gtCloneExpr(

case GT_LCL_VAR:

if (tree->AsLclVarCommon()->GetLclNum() == varNum)
{
copy = gtNewIconNode(varVal, tree->gtType);
}
else
{
// Remember that the local node has been cloned. The flag will be set on 'copy' as well.
tree->gtFlags |= GTF_VAR_MOREUSES;
copy = gtNewLclvNode(tree->AsLclVar()->GetLclNum(),
tree->gtType DEBUGARG(tree->AsLclVar()->gtLclILoffs));
copy->AsLclVarCommon()->SetSsaNum(tree->AsLclVarCommon()->GetSsaNum());
}
// Remember that the local node has been cloned. The flag will be set on 'copy' as well.
tree->gtFlags |= GTF_VAR_MOREUSES;
copy =
gtNewLclvNode(tree->AsLclVar()->GetLclNum(), tree->gtType DEBUGARG(tree->AsLclVar()->gtLclILoffs));
copy->AsLclVarCommon()->SetSsaNum(tree->AsLclVarCommon()->GetSsaNum());
goto DONE;

case GT_LCL_FLD:
if (tree->AsLclFld()->GetLclNum() == varNum)
{
IMPL_LIMITATION("replacing GT_LCL_FLD with a constant");
}
else
{
// Remember that the local node has been cloned. The flag will be set on 'copy' as well.
tree->gtFlags |= GTF_VAR_MOREUSES;
copy = new (this, GT_LCL_FLD)
GenTreeLclFld(GT_LCL_FLD, tree->TypeGet(), tree->AsLclFld()->GetLclNum(),
tree->AsLclFld()->GetLclOffs(), tree->AsLclFld()->GetLayout());
copy->AsLclFld()->SetSsaNum(tree->AsLclFld()->GetSsaNum());
}
// Remember that the local node has been cloned. The flag will be set on 'copy' as well.
tree->gtFlags |= GTF_VAR_MOREUSES;
copy =
new (this, GT_LCL_FLD) GenTreeLclFld(GT_LCL_FLD, tree->TypeGet(), tree->AsLclFld()->GetLclNum(),
tree->AsLclFld()->GetLclOffs(), tree->AsLclFld()->GetLayout());
copy->AsLclFld()->SetSsaNum(tree->AsLclFld()->GetSsaNum());
goto DONE;

case GT_RET_EXPR:
Expand Down Expand Up @@ -9453,38 +9425,14 @@ GenTree* Compiler::gtCloneExpr(
break;
}

// Some flags are conceptually part of the gtOper, and should be copied immediately.
if (tree->gtOverflowEx())
{
copy->gtFlags |= GTF_OVERFLOW;
}

if (tree->AsOp()->gtOp1)
{
copy->AsOp()->gtOp1 = gtCloneExpr(tree->AsOp()->gtOp1, addFlags, deepVarNum, deepVarVal);
copy->AsOp()->gtOp1 = gtCloneExpr(tree->AsOp()->gtOp1);
}

if (tree->gtGetOp2IfPresent())
{
copy->AsOp()->gtOp2 = gtCloneExpr(tree->AsOp()->gtOp2, addFlags, deepVarNum, deepVarVal);
}

/* Flags */
addFlags |= tree->gtFlags;

#ifdef DEBUG
/* GTF_NODE_MASK should not be propagated from 'tree' to 'copy' */
addFlags &= ~GTF_NODE_MASK;
#endif

// Effects flags propagate upwards.
if (copy->AsOp()->gtOp1 != nullptr)
{
copy->gtFlags |= (copy->AsOp()->gtOp1->gtFlags & GTF_ALL_EFFECT);
}
if (copy->gtGetOp2IfPresent() != nullptr)
{
copy->gtFlags |= (copy->gtGetOp2()->gtFlags & GTF_ALL_EFFECT);
copy->AsOp()->gtOp2 = gtCloneExpr(tree->AsOp()->gtOp2);
}

goto DONE;
Expand All @@ -9503,7 +9451,7 @@ GenTree* Compiler::gtCloneExpr(
NO_WAY("Cloning of calls with associated GT_RET_EXPR nodes is not supported");
}

copy = gtCloneExprCallHelper(tree->AsCall(), addFlags, deepVarNum, deepVarVal);
copy = gtCloneExprCallHelper(tree->AsCall());
break;

#ifdef FEATURE_HW_INTRINSICS
Expand All @@ -9519,7 +9467,7 @@ GenTree* Compiler::gtCloneExpr(
CLONE_MULTIOP_OPERANDS:
for (GenTree** use : copy->AsMultiOp()->UseEdges())
{
*use = gtCloneExpr(*use, addFlags, deepVarNum, deepVarVal);
*use = gtCloneExpr(*use);
}
break;
#endif
Expand All @@ -9530,11 +9478,10 @@ GenTree* Compiler::gtCloneExpr(
GenTree* inds[GT_ARR_MAX_RANK];
for (unsigned dim = 0; dim < arrElem->gtArrRank; dim++)
{
inds[dim] = gtCloneExpr(arrElem->gtArrInds[dim], addFlags, deepVarNum, deepVarVal);
inds[dim] = gtCloneExpr(arrElem->gtArrInds[dim]);
}
copy = new (this, GT_ARR_ELEM)
GenTreeArrElem(arrElem->TypeGet(), gtCloneExpr(arrElem->gtArrObj, addFlags, deepVarNum, deepVarVal),
arrElem->gtArrRank, arrElem->gtArrElemSize, &inds[0]);
copy = new (this, GT_ARR_ELEM) GenTreeArrElem(arrElem->TypeGet(), gtCloneExpr(arrElem->gtArrObj),
arrElem->gtArrRank, arrElem->gtArrElemSize, &inds[0]);
}
break;

Expand All @@ -9544,9 +9491,8 @@ GenTree* Compiler::gtCloneExpr(
GenTreePhi::Use** prevUse = &copy->AsPhi()->gtUses;
for (GenTreePhi::Use& use : tree->AsPhi()->Uses())
{
*prevUse = new (this, CMK_ASTNode)
GenTreePhi::Use(gtCloneExpr(use.GetNode(), addFlags, deepVarNum, deepVarVal), *prevUse);
prevUse = &((*prevUse)->NextRef());
*prevUse = new (this, CMK_ASTNode) GenTreePhi::Use(gtCloneExpr(use.GetNode()), *prevUse);
prevUse = &((*prevUse)->NextRef());
}
}
break;
Expand All @@ -9555,32 +9501,27 @@ GenTree* Compiler::gtCloneExpr(
copy = new (this, GT_FIELD_LIST) GenTreeFieldList();
for (GenTreeFieldList::Use& use : tree->AsFieldList()->Uses())
{
copy->AsFieldList()->AddField(this, gtCloneExpr(use.GetNode(), addFlags, deepVarNum, deepVarVal),
use.GetOffset(), use.GetType());
copy->AsFieldList()->AddField(this, gtCloneExpr(use.GetNode()), use.GetOffset(), use.GetType());
}
break;

case GT_CMPXCHG:
copy = new (this, GT_CMPXCHG)
GenTreeCmpXchg(tree->TypeGet(),
gtCloneExpr(tree->AsCmpXchg()->Addr(), addFlags, deepVarNum, deepVarVal),
gtCloneExpr(tree->AsCmpXchg()->Data(), addFlags, deepVarNum, deepVarVal),
gtCloneExpr(tree->AsCmpXchg()->Comparand(), addFlags, deepVarNum, deepVarVal));
GenTreeCmpXchg(tree->TypeGet(), gtCloneExpr(tree->AsCmpXchg()->Addr()),
gtCloneExpr(tree->AsCmpXchg()->Data()), gtCloneExpr(tree->AsCmpXchg()->Comparand()));
break;

case GT_STORE_DYN_BLK:
copy = new (this, oper)
GenTreeStoreDynBlk(gtCloneExpr(tree->AsStoreDynBlk()->Addr(), addFlags, deepVarNum, deepVarVal),
gtCloneExpr(tree->AsStoreDynBlk()->Data(), addFlags, deepVarNum, deepVarVal),
gtCloneExpr(tree->AsStoreDynBlk()->gtDynamicSize, addFlags, deepVarNum, deepVarVal));
copy = new (this, oper) GenTreeStoreDynBlk(gtCloneExpr(tree->AsStoreDynBlk()->Addr()),
gtCloneExpr(tree->AsStoreDynBlk()->Data()),
gtCloneExpr(tree->AsStoreDynBlk()->gtDynamicSize));
break;

case GT_SELECT:
copy = new (this, oper)
GenTreeConditional(oper, tree->TypeGet(),
gtCloneExpr(tree->AsConditional()->gtCond, addFlags, deepVarNum, deepVarVal),
gtCloneExpr(tree->AsConditional()->gtOp1, addFlags, deepVarNum, deepVarVal),
gtCloneExpr(tree->AsConditional()->gtOp2, addFlags, deepVarNum, deepVarVal));
copy =
new (this, oper) GenTreeConditional(oper, tree->TypeGet(), gtCloneExpr(tree->AsConditional()->gtCond),
gtCloneExpr(tree->AsConditional()->gtOp1),
gtCloneExpr(tree->AsConditional()->gtOp2));
break;
default:
#ifdef DEBUG
Expand All @@ -9590,33 +9531,10 @@ GenTree* Compiler::gtCloneExpr(
}

DONE:
assert(copy->gtOper == oper);

copy->gtVNPair = tree->gtVNPair; // A cloned tree gets the original's Value number pair

/* Compute the flags for the copied node. Note that we can do this only
if we didnt gtFoldExpr(copy) */

if (copy->gtOper == oper)
{
addFlags |= tree->gtFlags;

#ifdef DEBUG
/* GTF_NODE_MASK should not be propagated from 'tree' to 'copy' */
addFlags &= ~GTF_NODE_MASK;
#endif
copy->gtFlags |= addFlags;

// Update side effect flags since they may be different from the source side effect flags.
// For example, we may have replaced some locals with constants and made indirections non-throwing.
gtUpdateNodeSideEffects(copy);
if ((varNum != BAD_VAR_NUM) && copy->OperIsSsaDef())
{
fgAssignSetVarDef(copy);
}
}

/* GTF_COLON_COND should be propagated from 'tree' to 'copy' */
copy->gtFlags |= (tree->gtFlags & GTF_COLON_COND);
copy->gtFlags |= tree->gtFlags;

#if defined(DEBUG)
// Non-node debug flags should be propagated from 'tree' to 'copy'
Expand Down Expand Up @@ -9704,25 +9622,18 @@ void CallArgs::InternalCopyFrom(Compiler* comp, CallArgs* other, CopyNodeFunc co
//
// Arguments:
// tree - the call to clone
// addFlags - GTF_* flags to add to the copied tree nodes
// deepVarNum - lclNum to replace uses of beyond the root, or BAD_VAR_NUM for no replacement
// deepVarVal - If replacing beyond root, replace `deepVarNum` with IntCns `deepVarVal`
//
// Returns:
// Cloned copy of call and all subtrees.

GenTreeCall* Compiler::gtCloneExprCallHelper(GenTreeCall* tree,
GenTreeFlags addFlags,
unsigned deepVarNum,
int deepVarVal)
GenTreeCall* Compiler::gtCloneExprCallHelper(GenTreeCall* tree)
{
GenTreeCall* copy = new (this, GT_CALL) GenTreeCall(tree->TypeGet());

copy->gtCallMoreFlags = tree->gtCallMoreFlags;
INDEBUG(copy->gtCallDebugFlags = tree->gtCallDebugFlags);

copy->gtArgs.InternalCopyFrom(this, &tree->gtArgs,
[=](GenTree* node) { return gtCloneExpr(node, addFlags, deepVarNum, deepVarVal); });
copy->gtArgs.InternalCopyFrom(this, &tree->gtArgs, [=](GenTree* node) { return gtCloneExpr(node); });

// The call sig comes from the EE and doesn't change throughout the compilation process, meaning
// we only really need one physical copy of it. Therefore a shallow pointer copy will suffice.
Expand All @@ -9735,15 +9646,14 @@ GenTreeCall* Compiler::gtCloneExprCallHelper(GenTreeCall* tree,
copy->tailCallInfo = tree->tailCallInfo;

copy->gtRetClsHnd = tree->gtRetClsHnd;
copy->gtControlExpr = gtCloneExpr(tree->gtControlExpr, addFlags, deepVarNum, deepVarVal);
copy->gtControlExpr = gtCloneExpr(tree->gtControlExpr);
copy->gtStubCallStubAddr = tree->gtStubCallStubAddr;

/* Copy the union */
if (tree->gtCallType == CT_INDIRECT)
{
copy->gtCallCookie =
tree->gtCallCookie ? gtCloneExpr(tree->gtCallCookie, addFlags, deepVarNum, deepVarVal) : nullptr;
copy->gtCallAddr = tree->gtCallAddr ? gtCloneExpr(tree->gtCallAddr, addFlags, deepVarNum, deepVarVal) : nullptr;
copy->gtCallCookie = tree->gtCallCookie ? gtCloneExpr(tree->gtCallCookie) : nullptr;
copy->gtCallAddr = tree->gtCallAddr ? gtCloneExpr(tree->gtCallAddr) : nullptr;
}
else
{
Expand Down
Loading