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

use GenTreeFlags oftener for better debugging. #61272

Merged
merged 1 commit into from
Nov 6, 2021
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
8 changes: 6 additions & 2 deletions src/coreclr/jit/compiler.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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)
{
Expand Down Expand Up @@ -9361,6 +9361,10 @@ void cTreeFlags(Compiler* comp, GenTree* tree)

chars += printf("[ICON_FIELD_OFF]");
break;

default:
assert(!"a forgotten handle flag");
break;
}
}
break;
Expand Down Expand Up @@ -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);
Expand Down
12 changes: 6 additions & 6 deletions src/coreclr/jit/compiler.h
Original file line number Diff line number Diff line change
Expand Up @@ -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);

Expand Down
6 changes: 5 additions & 1 deletion src/coreclr/jit/fgdiagnostic.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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?
Expand Down Expand Up @@ -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;
}
}
Expand All @@ -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;
}
}
Expand Down
18 changes: 9 additions & 9 deletions src/coreclr/jit/gentree.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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)
{
Expand Down Expand Up @@ -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)
Expand Down Expand Up @@ -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<SideEffectExtractor>
{
public:
const unsigned m_flags;
const GenTreeFlags m_flags;
ArrayStack<GenTree*> m_sideEffects;

enum
Expand All @@ -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))
{
}
Expand Down
10 changes: 5 additions & 5 deletions src/coreclr/jit/importer.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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;
}
Expand Down Expand Up @@ -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++)
{
Expand Down Expand Up @@ -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;
Expand Down
2 changes: 1 addition & 1 deletion src/coreclr/jit/morph.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -14538,7 +14538,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.
Expand Down