From 6547b55d2ac61db14a603f9b83eec7e9a353321e Mon Sep 17 00:00:00 2001 From: Sergey Andreenko Date: Sat, 6 Nov 2021 02:28:18 -0700 Subject: [PATCH] use GenTreeFlags for better debugging (#61272) --- src/coreclr/jit/compiler.cpp | 8 ++++++-- src/coreclr/jit/compiler.h | 12 ++++++------ src/coreclr/jit/fgdiagnostic.cpp | 6 +++++- src/coreclr/jit/gentree.cpp | 18 +++++++++--------- src/coreclr/jit/importer.cpp | 10 +++++----- src/coreclr/jit/morph.cpp | 2 +- 6 files changed, 32 insertions(+), 24 deletions(-) diff --git a/src/coreclr/jit/compiler.cpp b/src/coreclr/jit/compiler.cpp index a26a003a455fb..d4f616892c927 100644 --- a/src/coreclr/jit/compiler.cpp +++ b/src/coreclr/jit/compiler.cpp @@ -9277,7 +9277,7 @@ void cTreeFlags(Compiler* comp, GenTree* tree) case GT_CNS_INT: { - unsigned handleKind = (tree->gtFlags & GTF_ICON_HDL_MASK); + GenTreeFlags handleKind = (tree->gtFlags & GTF_ICON_HDL_MASK); switch (handleKind) { @@ -9361,6 +9361,10 @@ void cTreeFlags(Compiler* comp, GenTree* tree) chars += printf("[ICON_FIELD_OFF]"); break; + + default: + assert(!"a forgotten handle flag"); + break; } } break; @@ -9517,7 +9521,7 @@ void cTreeFlags(Compiler* comp, GenTree* tree) default: { - unsigned flags = (tree->gtFlags & (~(unsigned)(GTF_COMMON_MASK | GTF_OVERFLOW))); + GenTreeFlags flags = (tree->gtFlags & (~(GTF_COMMON_MASK | GTF_OVERFLOW))); if (flags != 0) { chars += printf("[%08X]", flags); diff --git a/src/coreclr/jit/compiler.h b/src/coreclr/jit/compiler.h index befb4a6f60b7b..188c583b3dca7 100644 --- a/src/coreclr/jit/compiler.h +++ b/src/coreclr/jit/compiler.h @@ -3455,20 +3455,20 @@ class Compiler void gtSetStmtInfo(Statement* stmt); // Returns "true" iff "node" has any of the side effects in "flags". - bool gtNodeHasSideEffects(GenTree* node, unsigned flags); + bool gtNodeHasSideEffects(GenTree* node, GenTreeFlags flags); // Returns "true" iff "tree" or its (transitive) children have any of the side effects in "flags". - bool gtTreeHasSideEffects(GenTree* tree, unsigned flags); + bool gtTreeHasSideEffects(GenTree* tree, GenTreeFlags flags); // Appends 'expr' in front of 'list' // 'list' will typically start off as 'nullptr' // when 'list' is non-null a GT_COMMA node is used to insert 'expr' GenTree* gtBuildCommaList(GenTree* list, GenTree* expr); - void gtExtractSideEffList(GenTree* expr, - GenTree** pList, - unsigned flags = GTF_SIDE_EFFECT, - bool ignoreRoot = false); + void gtExtractSideEffList(GenTree* expr, + GenTree** pList, + GenTreeFlags GenTreeFlags = GTF_SIDE_EFFECT, + bool ignoreRoot = false); GenTree* gtGetThisArg(GenTreeCall* call); diff --git a/src/coreclr/jit/fgdiagnostic.cpp b/src/coreclr/jit/fgdiagnostic.cpp index c1af1a30badfe..c5b6490b10b97 100644 --- a/src/coreclr/jit/fgdiagnostic.cpp +++ b/src/coreclr/jit/fgdiagnostic.cpp @@ -2967,7 +2967,7 @@ void Compiler::fgDebugCheckFlags(GenTree* tree) { // Is this constant a handle of some kind? // - unsigned handleKind = (op1->gtFlags & GTF_ICON_HDL_MASK); + GenTreeFlags handleKind = (op1->gtFlags & GTF_ICON_HDL_MASK); if (handleKind != 0) { // Is the GTF_IND_INVARIANT flag set or unset? @@ -3130,6 +3130,8 @@ void Compiler::fgDebugCheckFlags(GenTree* tree) if ((call->gtCallThisArg->GetNode()->gtFlags & GTF_ASG) != 0) { + // TODO-Cleanup: this is a patch for a violation in our GT_ASG propogation + // see https://github.com/dotnet/runtime/issues/13758 treeFlags |= GTF_ASG; } } @@ -3142,6 +3144,8 @@ void Compiler::fgDebugCheckFlags(GenTree* tree) if ((use.GetNode()->gtFlags & GTF_ASG) != 0) { + // TODO-Cleanup: this is a patch for a violation in our GT_ASG propogation + // see https://github.com/dotnet/runtime/issues/13758 treeFlags |= GTF_ASG; } } diff --git a/src/coreclr/jit/gentree.cpp b/src/coreclr/jit/gentree.cpp index 90093f9e7bba6..81f2c5e9205ea 100644 --- a/src/coreclr/jit/gentree.cpp +++ b/src/coreclr/jit/gentree.cpp @@ -15567,7 +15567,7 @@ GenTree* Compiler::gtNewRefCOMfield(GenTree* objPtr, * assignments too. */ -bool Compiler::gtNodeHasSideEffects(GenTree* tree, unsigned flags) +bool Compiler::gtNodeHasSideEffects(GenTree* tree, GenTreeFlags flags) { if (flags & GTF_ASG) { @@ -15643,10 +15643,10 @@ bool Compiler::gtNodeHasSideEffects(GenTree* tree, unsigned flags) * Returns true if the expr tree has any side effects. */ -bool Compiler::gtTreeHasSideEffects(GenTree* tree, unsigned flags /* = GTF_SIDE_EFFECT*/) +bool Compiler::gtTreeHasSideEffects(GenTree* tree, GenTreeFlags flags /* = GTF_SIDE_EFFECT*/) { // These are the side effect flags that we care about for this tree - unsigned sideEffectFlags = tree->gtFlags & flags; + GenTreeFlags sideEffectFlags = tree->gtFlags & flags; // Does this tree have any Side-effect flags set that we care about? if (sideEffectFlags == 0) @@ -15760,15 +15760,15 @@ GenTree* Compiler::gtBuildCommaList(GenTree* list, GenTree* expr) // each comma node holds the side effect tree and op2 points to the // next comma node. The original side effect execution order is preserved. // -void Compiler::gtExtractSideEffList(GenTree* expr, - GenTree** pList, - unsigned flags /* = GTF_SIDE_EFFECT*/, - bool ignoreRoot /* = false */) +void Compiler::gtExtractSideEffList(GenTree* expr, + GenTree** pList, + GenTreeFlags flags /* = GTF_SIDE_EFFECT*/, + bool ignoreRoot /* = false */) { class SideEffectExtractor final : public GenTreeVisitor { public: - const unsigned m_flags; + const GenTreeFlags m_flags; ArrayStack m_sideEffects; enum @@ -15777,7 +15777,7 @@ void Compiler::gtExtractSideEffList(GenTree* expr, UseExecutionOrder = true }; - SideEffectExtractor(Compiler* compiler, unsigned flags) + SideEffectExtractor(Compiler* compiler, GenTreeFlags flags) : GenTreeVisitor(compiler), m_flags(flags), m_sideEffects(compiler->getAllocator(CMK_SideEffects)) { } diff --git a/src/coreclr/jit/importer.cpp b/src/coreclr/jit/importer.cpp index 9dc10836d10a4..98c5354269824 100644 --- a/src/coreclr/jit/importer.cpp +++ b/src/coreclr/jit/importer.cpp @@ -548,8 +548,8 @@ inline void Compiler::impAppendStmt(Statement* stmt, unsigned chkLevel) /* If the statement being appended has any side-effects, check the stack to see if anything needs to be spilled to preserve correct ordering. */ - GenTree* expr = stmt->GetRootNode(); - unsigned flags = expr->gtFlags & GTF_GLOB_EFFECT; + GenTree* expr = stmt->GetRootNode(); + GenTreeFlags flags = expr->gtFlags & GTF_GLOB_EFFECT; // Assignment to (unaliased) locals don't count as a side-effect as // we handle them specially using impSpillLclRefs(). Temp locals should @@ -558,7 +558,7 @@ inline void Compiler::impAppendStmt(Statement* stmt, unsigned chkLevel) if ((expr->gtOper == GT_ASG) && (expr->AsOp()->gtOp1->gtOper == GT_LCL_VAR) && ((expr->AsOp()->gtOp1->gtFlags & GTF_GLOB_REF) == 0) && !gtHasLocalsWithAddrOp(expr->AsOp()->gtOp2)) { - unsigned op2Flags = expr->AsOp()->gtOp2->gtFlags & GTF_GLOB_EFFECT; + GenTreeFlags op2Flags = expr->AsOp()->gtOp2->gtFlags & GTF_GLOB_EFFECT; assert(flags == (op2Flags | GTF_ASG)); flags = op2Flags; } @@ -2617,7 +2617,7 @@ inline void Compiler::impSpillSideEffects(bool spillGlobEffects, unsigned chkLev assert(chkLevel <= verCurrentState.esStackDepth); - unsigned spillFlags = spillGlobEffects ? GTF_GLOB_EFFECT : GTF_SIDE_EFFECT; + GenTreeFlags spillFlags = spillGlobEffects ? GTF_GLOB_EFFECT : GTF_SIDE_EFFECT; for (unsigned i = 0; i < chkLevel; i++) { @@ -20732,7 +20732,7 @@ bool Compiler::impInlineIsGuaranteedThisDerefBeforeAnySideEffects(GenTree* for (unsigned level = 0; level < verCurrentState.esStackDepth; level++) { - unsigned stackTreeFlags = verCurrentState.esStack[level].val->gtFlags; + GenTreeFlags stackTreeFlags = verCurrentState.esStack[level].val->gtFlags; if (GTF_GLOBALLY_VISIBLE_SIDE_EFFECTS(stackTreeFlags)) { return false; diff --git a/src/coreclr/jit/morph.cpp b/src/coreclr/jit/morph.cpp index 83946626f25d9..f62ab8a5e2ffc 100644 --- a/src/coreclr/jit/morph.cpp +++ b/src/coreclr/jit/morph.cpp @@ -14530,7 +14530,7 @@ GenTree* Compiler::fgRecognizeAndMorphBitwiseRotation(GenTree* tree) { noway_assert(GenTree::OperIsRotate(rotateOp)); - unsigned inputTreeEffects = tree->gtFlags & GTF_ALL_EFFECT; + GenTreeFlags inputTreeEffects = tree->gtFlags & GTF_ALL_EFFECT; // We can use the same tree only during global morph; reusing the tree in a later morph // may invalidate value numbers.