Skip to content

Commit

Permalink
JIT: revise how we track if nodes have been morphed (#110787)
Browse files Browse the repository at this point in the history 
Allow remorphing. Require that all nodes be morphed. Ensure that no node
is morphed more than a handful of times.

Fixes #6218, #56962, #107542
  • Loading branch information
@AndyAyersMS
AndyAyersMS authored Dec 18, 2024
1 parent 3e5b44a commit 4822cc0
Show file tree
Hide file tree
Showing 7 changed files with 293 additions and 222 deletions.
6 changes: 5 additions & 1 deletion src/coreclr/jit/compiler.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -5438,6 +5438,10 @@ class Compiler
void fgMorphBlock(BasicBlock* block, MorphUnreachableInfo* unreachableInfo = nullptr);
void fgMorphStmts(BasicBlock* block);

#ifdef DEBUG
void fgPostGlobalMorphChecks();
#endif

void fgMergeBlockReturn(BasicBlock* block);

bool fgMorphBlockStmt(BasicBlock* block, Statement* stmt DEBUGARG(const char* msg), bool invalidateDFSTreeOnFGChange = true);
Expand Down Expand Up @@ -6785,7 +6789,7 @@ class Compiler
void fgKillDependentAssertionsSingle(unsigned lclNum DEBUGARG(GenTree* tree));
void fgKillDependentAssertions(unsigned lclNum DEBUGARG(GenTree* tree));
void fgMorphTreeDone(GenTree* tree);
void fgMorphTreeDone(GenTree* tree, bool optAssertionPropDone, bool isMorphedTree DEBUGARG(int morphNum = 0));
void fgMorphTreeDone(GenTree* tree, bool optAssertionPropDone DEBUGARG(int morphNum = 0));

Statement* fgMorphStmt;
unsigned fgBigOffsetMorphingTemps[TYP_COUNT];
Expand Down
1 change: 1 addition & 0 deletions src/coreclr/jit/compiler.hpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1354,6 +1354,7 @@ inline GenTree::GenTree(genTreeOps oper, var_types type DEBUGARG(bool largeNode)
gtLIRFlags = 0;
#ifdef DEBUG
gtDebugFlags = GTF_DEBUG_NONE;
gtMorphCount = 0;
#endif // DEBUG
gtCSEnum = NO_CSE;
ClearAssertion();
Expand Down
151 changes: 88 additions & 63 deletions src/coreclr/jit/gentree.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -4903,7 +4903,6 @@ static void SetIndirectStoreEvalOrder(Compiler* comp, GenTreeIndir* store, bool*
* 1. GetCostEx() to the execution complexity estimate
* 2. GetCostSz() to the code size estimate
* 3. Sometimes sets GTF_ADDRMODE_NO_CSE on nodes in the tree.
* 4. DEBUG-only: clears GTF_DEBUG_NODE_MORPHED.
*/

#ifdef _PREFAST_
Expand Down Expand Up @@ -14388,10 +14387,7 @@ GenTree* Compiler::gtFoldExprSpecial(GenTree* tree)
// Helper function that creates a new IntCon node and morphs it, if required
auto NewMorphedIntConNode = [&](int value) -> GenTreeIntCon* {
GenTreeIntCon* icon = gtNewIconNode(value);
if (fgGlobalMorph)
{
INDEBUG(icon->gtDebugFlags |= GTF_DEBUG_NODE_MORPHED);
}
icon->SetMorphed(this);
return icon;
};

Expand All @@ -14402,18 +14398,14 @@ GenTree* Compiler::gtFoldExprSpecial(GenTree* tree)
assert(varTypeIsUnsigned(castToType));

GenTreeCast* cast = gtNewCastNode(TYP_INT, op1, false, castToType);
if (fgGlobalMorph)
{
fgMorphTreeDone(cast);
}
cast->SetMorphed(this);
fgMorphTreeDone(cast);

if (type == TYP_LONG)
{
cast = gtNewCastNode(TYP_LONG, cast, true, TYP_LONG);
if (fgGlobalMorph)
{
fgMorphTreeDone(cast);
}
cast->SetMorphed(this);
fgMorphTreeDone(cast);
}

return cast;
Expand Down Expand Up @@ -14680,14 +14672,7 @@ GenTree* Compiler::gtFoldExprSpecial(GenTree* tree)
DISPTREE(tree);
JITDUMP("Transformed into:\n");
DISPTREE(op);

if (fgGlobalMorph)
{
// We can sometimes produce a comma over the constant if the original op
// had a side effect, so just ensure we set the flag (which will be already
// set for the operands otherwise).
INDEBUG(op->gtDebugFlags |= GTF_DEBUG_NODE_MORPHED);
}
op->SetMorphed(this);
return op;
}

Expand Down Expand Up @@ -14742,10 +14727,9 @@ GenTree* Compiler::gtFoldExprSpecialFloating(GenTree* tree)
// Helper function that creates a new IntCon node and morphs it, if required
auto NewMorphedIntConNode = [&](int value) -> GenTreeIntCon* {
GenTreeIntCon* icon = gtNewIconNode(value);
if (fgGlobalMorph)
{
INDEBUG(icon->gtDebugFlags |= GTF_DEBUG_NODE_MORPHED);
}

icon->SetMorphed(this);

return icon;
};

Expand Down Expand Up @@ -14907,14 +14891,8 @@ GenTree* Compiler::gtFoldExprSpecialFloating(GenTree* tree)
DISPTREE(tree);
JITDUMP("Transformed into:\n");
DISPTREE(op);
op->SetMorphed(this);

if (fgGlobalMorph)
{
// We can sometimes produce a comma over the constant if the original op
// had a side effect, so just ensure we set the flag (which will be already
// set for the operands otherwise).
INDEBUG(op->gtDebugFlags |= GTF_DEBUG_NODE_MORPHED);
}
return op;
}

Expand Down Expand Up @@ -15435,6 +15413,11 @@ GenTree* Compiler::gtOptimizeEnumHasFlag(GenTree* thisOp, GenTree* flagOp)
Statement* thisStoreStmt = thisOp->AsBox()->gtCopyStmtWhenInlinedBoxValue;
thisStoreStmt->SetRootNode(thisStore);
thisValOpt = gtNewLclvNode(thisTmp, type);

// If this is invoked during global morph we are adding code to a remote tree
// Despite this being a store, we can't meaningfully add assertions
//
thisStore->SetMorphed(this);
}

if (flagVal->IsIntegralConst())
Expand All @@ -15452,6 +15435,11 @@ GenTree* Compiler::gtOptimizeEnumHasFlag(GenTree* thisOp, GenTree* flagOp)
flagStoreStmt->SetRootNode(flagStore);
flagValOpt = gtNewLclvNode(flagTmp, type);
flagValOptCopy = gtNewLclvNode(flagTmp, type);

// If this is invoked during global morph we are adding code to a remote tree
// Despite this being a store, we can't meaningfully add assertions
//
flagStore->SetMorphed(this);
}

// Turn the call into (thisValTmp & flagTmp) == flagTmp.
Expand Down Expand Up @@ -17436,16 +17424,7 @@ void Compiler::gtExtractSideEffList(GenTree* expr,
}

GenTree* comma = m_compiler->gtNewOperNode(GT_COMMA, TYP_VOID, m_result, node);

#ifdef DEBUG
if (m_compiler->fgGlobalMorph)
{
// Either both should be morphed or neither should be.
assert((m_result->gtDebugFlags & GTF_DEBUG_NODE_MORPHED) ==
(node->gtDebugFlags & GTF_DEBUG_NODE_MORPHED));
comma->gtDebugFlags |= node->gtDebugFlags & GTF_DEBUG_NODE_MORPHED;
}
#endif
comma->SetMorphed(m_compiler);

// Both should have valuenumbers defined for both or for neither
// one (unless we are remorphing, in which case a prior transform
Expand Down Expand Up @@ -30762,12 +30741,7 @@ GenTree* Compiler::gtFoldExprHWIntrinsic(GenTreeHWIntrinsic* tree)
}

vecCon->gtSimdVal = simdVal;

if (fgGlobalMorph)
{
INDEBUG(vecCon->gtDebugFlags |= GTF_DEBUG_NODE_MORPHED);
}

vecCon->SetMorphed(this);
fgUpdateConstTreeValueNumber(vecCon);
return vecCon;
}
Expand Down Expand Up @@ -30829,11 +30803,7 @@ GenTree* Compiler::gtFoldExprHWIntrinsic(GenTreeHWIntrinsic* tree)
#endif // !TARGET_XARCH && !TARGET_ARM64

DEBUG_DESTROY_NODE(op, tree);

if (fgGlobalMorph)
{
INDEBUG(vectorNode->gtDebugFlags |= GTF_DEBUG_NODE_MORPHED);
}
vectorNode->SetMorphed(this);
return vectorNode;
}
}
Expand Down Expand Up @@ -32018,17 +31988,10 @@ GenTree* Compiler::gtFoldExprHWIntrinsic(GenTreeHWIntrinsic* tree)

if (resultNode != tree)
{
if (fgGlobalMorph)
resultNode->SetMorphed(this);
if (resultNode->OperIs(GT_COMMA))
{
// We can sometimes produce a comma over the constant if the original op
// had a side effect or even a new constant node, so just ensure we set
// the flag (which will be already set for the operands otherwise).
INDEBUG(resultNode->gtDebugFlags |= GTF_DEBUG_NODE_MORPHED);

if (resultNode->OperIs(GT_COMMA))
{
INDEBUG(resultNode->AsOp()->gtGetOp2()->gtDebugFlags |= GTF_DEBUG_NODE_MORPHED);
}
resultNode->AsOp()->gtGetOp2()->SetMorphed(this);
}

if (resultNode->OperIsConst())
Expand Down Expand Up @@ -32155,3 +32118,65 @@ bool Compiler::gtCanSkipCovariantStoreCheck(GenTree* value, GenTree* array)

return false;
}

#if defined(DEBUG)
//------------------------------------------------------------------------
// SetMorphed: mark a node as having been morphed
//
// Arguments:
// compiler - compiler instance
// doChildren - recursive mark child nodes
//
// Notes:
// Does nothing outside of global morph.
//
// Useful for morph post-order expansions / optimizations.
//
// Use care when invoking this on an assignment (or when doChildren is true,
// on trees containing assignments) as those usually will also require
// local assertion updates.
//
void GenTree::SetMorphed(Compiler* compiler, bool doChildren /* = false */)
{
if (!compiler->fgGlobalMorph)
{
return;
}

struct Visitor : GenTreeVisitor<Visitor>
{
enum
{
DoPostOrder = true,
};

Visitor(Compiler* comp)
: GenTreeVisitor(comp)
{
}

fgWalkResult PostOrderVisit(GenTree** use, GenTree* user)
{
GenTree* const node = *use;
if (!node->WasMorphed())
{
node->gtDebugFlags |= GTF_DEBUG_NODE_MORPHED;
node->gtMorphCount++;
}
return Compiler::WALK_CONTINUE;
}
};

if (doChildren)
{
Visitor v(compiler);
GenTree* node = this;
v.WalkTree(&node, nullptr);
}
else if (!WasMorphed())
{
gtDebugFlags |= GTF_DEBUG_NODE_MORPHED;
gtMorphCount++;
}
}
#endif
43 changes: 31 additions & 12 deletions src/coreclr/jit/gentree.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -602,21 +602,21 @@ inline GenTreeFlags& operator ^=(GenTreeFlags& a, GenTreeFlags b)
//------------------------------------------------------------------------
// GenTreeDebugFlags: a bitmask of debug-only flags for GenTree stored in gtDebugFlags
//
enum GenTreeDebugFlags : unsigned int
enum GenTreeDebugFlags : unsigned short
{
GTF_DEBUG_NONE = 0x00000000, // No debug flags.
GTF_DEBUG_NONE = 0x0000, // No debug flags.

GTF_DEBUG_NODE_MORPHED = 0x00000001, // the node has been morphed (in the global morphing phase)
GTF_DEBUG_NODE_SMALL = 0x00000002,
GTF_DEBUG_NODE_LARGE = 0x00000004,
GTF_DEBUG_NODE_CG_PRODUCED = 0x00000008, // genProduceReg has been called on this node
GTF_DEBUG_NODE_CG_CONSUMED = 0x00000010, // genConsumeReg has been called on this node
GTF_DEBUG_NODE_LSRA_ADDED = 0x00000020, // This node was added by LSRA
GTF_DEBUG_NODE_MORPHED = 0x0001, // the node has been morphed (in the global morphing phase)
GTF_DEBUG_NODE_SMALL = 0x0002,
GTF_DEBUG_NODE_LARGE = 0x0004,
GTF_DEBUG_NODE_CG_PRODUCED = 0x0008, // genProduceReg has been called on this node
GTF_DEBUG_NODE_CG_CONSUMED = 0x0010, // genConsumeReg has been called on this node
GTF_DEBUG_NODE_LSRA_ADDED = 0x0020, // This node was added by LSRA

GTF_DEBUG_NODE_MASK = 0x0000003F, // These flags are all node (rather than operation) properties.
GTF_DEBUG_NODE_MASK = 0x003F, // These flags are all node (rather than operation) properties.

GTF_DEBUG_VAR_CSE_REF = 0x00800000, // GT_LCL_VAR -- This is a CSE LCL_VAR node
GTF_DEBUG_CAST_DONT_FOLD = 0x00400000, // GT_CAST -- Try to prevent this cast from being folded
GTF_DEBUG_VAR_CSE_REF = 0x8000, // GT_LCL_VAR -- This is a CSE LCL_VAR node
GTF_DEBUG_CAST_DONT_FOLD = 0x4000, // GT_CAST -- Try to prevent this cast from being folded
};

inline constexpr GenTreeDebugFlags operator ~(GenTreeDebugFlags a)
Expand Down Expand Up @@ -972,7 +972,26 @@ struct GenTree

#if defined(DEBUG)
GenTreeDebugFlags gtDebugFlags;
#endif // defined(DEBUG)
unsigned short gtMorphCount;
void SetMorphed(Compiler* compiler, bool doChilren = false);

bool WasMorphed() const
{
return (gtDebugFlags & GTF_DEBUG_NODE_MORPHED) != 0;
}

void ClearMorphed()
{
gtDebugFlags &= ~GTF_DEBUG_NODE_MORPHED;
}
#else
void SetMorphed(Compiler* compiler, bool doChildren = false)
{
}
void ClearMorphed()
{
}
#endif

ValueNumPair gtVNPair;

Expand Down
Loading

0 comments on commit 4822cc0

Please sign in to comment.