Skip to content

Commit

Permalink
Review feedback
Browse files Browse the repository at this point in the history 
Added FMT_LP formatting string.

Cached often-used `optLoopTable[loopInd]` expression.

Added `const` to many loop query member functions.

Added static `GenTree::OperIs(compareOper, oper, oper, oper...)`
functions for simplifying oper check expressions where the
compareOper isn't from a GenTree node `OperGet()`. (This doesn't
need to be part of GenTree, but it doesn't hurt, either.)

Added a few more comments.
  • Loading branch information
@BruceForstall
BruceForstall committed Mar 23, 2021
1 parent d77badb commit 3653f16
Show file tree
Hide file tree
Showing 8 changed files with 186 additions and 164 deletions.
3 changes: 3 additions & 0 deletions src/coreclr/jit/block.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,9 @@ typedef BitVec_ValRet_T ASSERT_VALRET_TP;
// This define is used with string concatenation to put this in printf format strings (Note that %u means unsigned int)
#define FMT_BB "BB%02u"

// Use this format for loop table indices.
#define FMT_LP "L%02u"

// And this format for profile weights
#define FMT_WT "%.7g"

Expand Down
69 changes: 37 additions & 32 deletions src/coreclr/jit/compiler.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -6310,83 +6310,88 @@ class Compiler

/* The following values are set only for iterator loops, i.e. has the flag LPFLG_ITER set */

GenTree* lpIterTree; // The "i = i <op> const" tree
unsigned lpIterVar(); // iterator variable #
int lpIterConst(); // the constant with which the iterator is incremented
genTreeOps lpIterOper(); // the type of the operation on the iterator (ASG_ADD, ASG_SUB, etc.)
void VERIFY_lpIterTree();
GenTree* lpIterTree; // The "i = i <op> const" tree
unsigned lpIterVar() const; // iterator variable #
int lpIterConst() const; // the constant with which the iterator is incremented
genTreeOps lpIterOper() const; // the type of the operation on the iterator (ASG_ADD, ASG_SUB, etc.)
void VERIFY_lpIterTree() const;

var_types lpIterOperType(); // For overflow instructions
var_types lpIterOperType() const; // For overflow instructions

union {
int lpConstInit; // initial constant value of iterator : Valid if LPFLG_CONST_INIT
unsigned lpVarInit; // initial local var number to which we initialize the iterator : Valid if
// LPFLG_VAR_INIT
};

/* The following is for LPFLG_ITER loops only (i.e. the loop condition is "i RELOP const or var" */
// The following is for LPFLG_ITER loops only (i.e. the loop condition is "i RELOP const or var"

GenTree* lpTestTree; // pointer to the node containing the loop test
genTreeOps lpTestOper(); // the type of the comparison between the iterator and the limit (GT_LE, GT_GE, etc.)
void VERIFY_lpTestTree();
GenTree* lpTestTree; // pointer to the node containing the loop test
genTreeOps lpTestOper() const; // the type of the comparison between the iterator and the limit (GT_LE, GT_GE,
// etc.)
void VERIFY_lpTestTree() const;

bool lpIsReversed(); // true if the iterator node is the second operand in the loop condition
GenTree* lpIterator(); // the iterator node in the loop test
GenTree* lpLimit(); // the limit node in the loop test
bool lpIsReversed() const; // true if the iterator node is the second operand in the loop condition
GenTree* lpIterator() const; // the iterator node in the loop test
GenTree* lpLimit() const; // the limit node in the loop test

int lpConstLimit(); // limit constant value of iterator - loop condition is "i RELOP const" : Valid if
// LPFLG_CONST_LIMIT
unsigned lpVarLimit(); // the lclVar # in the loop condition ( "i RELOP lclVar" ) : Valid if
// LPFLG_VAR_LIMIT
bool lpArrLenLimit(Compiler* comp, ArrIndex* index); // The array length in the loop condition ( "i RELOP
// arr.len" or "i RELOP arr[i][j].len" ) : Valid if
// LPFLG_ARRLEN_LIMIT
// Limit constant value of iterator - loop condition is "i RELOP const"
// : Valid if LPFLG_CONST_LIMIT
int lpConstLimit() const;

// The lclVar # in the loop condition ( "i RELOP lclVar" )
// : Valid if LPFLG_VAR_LIMIT
unsigned lpVarLimit() const;

// The array length in the loop condition ( "i RELOP arr.len" or "i RELOP arr[i][j].len" )
// : Valid if LPFLG_ARRLEN_LIMIT
bool lpArrLenLimit(Compiler* comp, ArrIndex* index) const;

// Returns "true" iff "*this" contains the blk.
bool lpContains(BasicBlock* blk)
bool lpContains(BasicBlock* blk) const
{
return lpFirst->bbNum <= blk->bbNum && blk->bbNum <= lpBottom->bbNum;
}
// Returns "true" iff "*this" (properly) contains the range [first, bottom] (allowing firsts
// to be equal, but requiring bottoms to be different.)
bool lpContains(BasicBlock* first, BasicBlock* bottom)
bool lpContains(BasicBlock* first, BasicBlock* bottom) const
{
return lpFirst->bbNum <= first->bbNum && bottom->bbNum < lpBottom->bbNum;
}

// Returns "true" iff "*this" (properly) contains "lp2" (allowing firsts to be equal, but requiring
// bottoms to be different.)
bool lpContains(const LoopDsc& lp2)
bool lpContains(const LoopDsc& lp2) const
{
return lpContains(lp2.lpFirst, lp2.lpBottom);
}

// Returns "true" iff "*this" is (properly) contained by the range [first, bottom]
// (allowing firsts to be equal, but requiring bottoms to be different.)
bool lpContainedBy(BasicBlock* first, BasicBlock* bottom)
bool lpContainedBy(BasicBlock* first, BasicBlock* bottom) const
{
return first->bbNum <= lpFirst->bbNum && lpBottom->bbNum < bottom->bbNum;
}

// Returns "true" iff "*this" is (properly) contained by "lp2"
// (allowing firsts to be equal, but requiring bottoms to be different.)
bool lpContainedBy(const LoopDsc& lp2)
bool lpContainedBy(const LoopDsc& lp2) const
{
return lpContains(lp2.lpFirst, lp2.lpBottom);
}

// Returns "true" iff "*this" is disjoint from the range [top, bottom].
bool lpDisjoint(BasicBlock* first, BasicBlock* bottom)
bool lpDisjoint(BasicBlock* first, BasicBlock* bottom) const
{
return bottom->bbNum < lpFirst->bbNum || lpBottom->bbNum < first->bbNum;
}
// Returns "true" iff "*this" is disjoint from "lp2".
bool lpDisjoint(const LoopDsc& lp2)
bool lpDisjoint(const LoopDsc& lp2) const
{
return lpDisjoint(lp2.lpFirst, lp2.lpBottom);
}
// Returns "true" iff the loop is well-formed (see code for defn).
bool lpWellFormed()
bool lpWellFormed() const
{
return lpFirst->bbNum <= lpTop->bbNum && lpTop->bbNum <= lpEntry->bbNum &&
lpEntry->bbNum <= lpBottom->bbNum &&
Expand Down Expand Up @@ -6426,9 +6431,9 @@ class Compiler
BasicBlock* lpBottom,
unsigned char lpExitCnt,
BasicBlock* lpExit,
unsigned parentLoop = BasicBlock::NOT_IN_LOOP);
void optPrintLoopInfo(unsigned lnum);
void optPrintLoopRecording(unsigned lnum);
unsigned parentLoop = BasicBlock::NOT_IN_LOOP) const;
void optPrintLoopInfo(unsigned lnum) const;
void optPrintLoopRecording(unsigned lnum) const;

void optCheckPreds();
#endif
Expand Down Expand Up @@ -6492,7 +6497,7 @@ class Compiler
void optCopyBlkDest(BasicBlock* from, BasicBlock* to);

// Returns true if 'block' is an entry block for any loop in 'optLoopTable'
bool optIsLoopEntry(BasicBlock* block);
bool optIsLoopEntry(BasicBlock* block) const;

// The depth of the loop described by "lnum" (an index into the loop table.) (0 == top level)
unsigned optLoopDepth(unsigned lnum)
Expand Down
30 changes: 15 additions & 15 deletions src/coreclr/jit/compiler.hpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -3388,7 +3388,7 @@ inline void Compiler::LoopDsc::AddModifiedElemType(Compiler* comp, CORINFO_CLASS
lpArrayElemTypesModified->Set(structHnd, true, ClassHandleSet::Overwrite);
}

inline void Compiler::LoopDsc::VERIFY_lpIterTree()
inline void Compiler::LoopDsc::VERIFY_lpIterTree() const
{
#ifdef DEBUG
assert(lpFlags & LPFLG_ITER);
Expand All @@ -3397,8 +3397,8 @@ inline void Compiler::LoopDsc::VERIFY_lpIterTree()

assert(lpIterTree->OperIs(GT_ASG));

GenTree* lhs = lpIterTree->AsOp()->gtOp1;
GenTree* rhs = lpIterTree->AsOp()->gtOp2;
const GenTree* lhs = lpIterTree->AsOp()->gtOp1;
const GenTree* rhs = lpIterTree->AsOp()->gtOp2;
assert(lhs->OperGet() == GT_LCL_VAR);

switch (rhs->gtOper)
Expand All @@ -3420,15 +3420,15 @@ inline void Compiler::LoopDsc::VERIFY_lpIterTree()

//-----------------------------------------------------------------------------

inline unsigned Compiler::LoopDsc::lpIterVar()
inline unsigned Compiler::LoopDsc::lpIterVar() const
{
VERIFY_lpIterTree();
return lpIterTree->AsOp()->gtOp1->AsLclVarCommon()->GetLclNum();
}

//-----------------------------------------------------------------------------

inline int Compiler::LoopDsc::lpIterConst()
inline int Compiler::LoopDsc::lpIterConst() const
{
VERIFY_lpIterTree();
GenTree* rhs = lpIterTree->AsOp()->gtOp2;
Expand All @@ -3437,14 +3437,14 @@ inline int Compiler::LoopDsc::lpIterConst()

//-----------------------------------------------------------------------------

inline genTreeOps Compiler::LoopDsc::lpIterOper()
inline genTreeOps Compiler::LoopDsc::lpIterOper() const
{
VERIFY_lpIterTree();
GenTree* rhs = lpIterTree->AsOp()->gtOp2;
return rhs->OperGet();
}

inline var_types Compiler::LoopDsc::lpIterOperType()
inline var_types Compiler::LoopDsc::lpIterOperType() const
{
VERIFY_lpIterTree();

Expand All @@ -3459,7 +3459,7 @@ inline var_types Compiler::LoopDsc::lpIterOperType()
return type;
}

inline void Compiler::LoopDsc::VERIFY_lpTestTree()
inline void Compiler::LoopDsc::VERIFY_lpTestTree() const
{
#ifdef DEBUG
assert(lpFlags & LPFLG_ITER);
Expand Down Expand Up @@ -3505,7 +3505,7 @@ inline void Compiler::LoopDsc::VERIFY_lpTestTree()

//-----------------------------------------------------------------------------

inline bool Compiler::LoopDsc::lpIsReversed()
inline bool Compiler::LoopDsc::lpIsReversed() const
{
VERIFY_lpTestTree();
return ((lpTestTree->AsOp()->gtOp2->gtOper == GT_LCL_VAR) &&
Expand All @@ -3514,7 +3514,7 @@ inline bool Compiler::LoopDsc::lpIsReversed()

//-----------------------------------------------------------------------------

inline genTreeOps Compiler::LoopDsc::lpTestOper()
inline genTreeOps Compiler::LoopDsc::lpTestOper() const
{
VERIFY_lpTestTree();
genTreeOps op = lpTestTree->OperGet();
Expand All @@ -3523,7 +3523,7 @@ inline genTreeOps Compiler::LoopDsc::lpTestOper()

//-----------------------------------------------------------------------------

inline GenTree* Compiler::LoopDsc::lpIterator()
inline GenTree* Compiler::LoopDsc::lpIterator() const
{
VERIFY_lpTestTree();

Expand All @@ -3532,7 +3532,7 @@ inline GenTree* Compiler::LoopDsc::lpIterator()

//-----------------------------------------------------------------------------

inline GenTree* Compiler::LoopDsc::lpLimit()
inline GenTree* Compiler::LoopDsc::lpLimit() const
{
VERIFY_lpTestTree();

Expand All @@ -3541,7 +3541,7 @@ inline GenTree* Compiler::LoopDsc::lpLimit()

//-----------------------------------------------------------------------------

inline int Compiler::LoopDsc::lpConstLimit()
inline int Compiler::LoopDsc::lpConstLimit() const
{
VERIFY_lpTestTree();
assert(lpFlags & LPFLG_CONST_LIMIT);
Expand All @@ -3553,7 +3553,7 @@ inline int Compiler::LoopDsc::lpConstLimit()

//-----------------------------------------------------------------------------

inline unsigned Compiler::LoopDsc::lpVarLimit()
inline unsigned Compiler::LoopDsc::lpVarLimit() const
{
VERIFY_lpTestTree();
assert(lpFlags & LPFLG_VAR_LIMIT);
Expand All @@ -3565,7 +3565,7 @@ inline unsigned Compiler::LoopDsc::lpVarLimit()

//-----------------------------------------------------------------------------

inline bool Compiler::LoopDsc::lpArrLenLimit(Compiler* comp, ArrIndex* index)
inline bool Compiler::LoopDsc::lpArrLenLimit(Compiler* comp, ArrIndex* index) const
{
VERIFY_lpTestTree();
assert(lpFlags & LPFLG_ARRLEN_LIMIT);
Expand Down
11 changes: 11 additions & 0 deletions src/coreclr/jit/gentree.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1087,6 +1087,17 @@ struct GenTree
return TypeIs(type) || TypeIs(rest...);
}

static bool OperIs(genTreeOps operCompare, genTreeOps oper)
{
return operCompare == oper;
}

template <typename... T>
static bool OperIs(genTreeOps operCompare, genTreeOps oper, T... rest)
{
return OperIs(operCompare, oper) || OperIs(operCompare, rest...);
}

bool OperIs(genTreeOps oper) const
{
return OperGet() == oper;
Expand Down
4 changes: 2 additions & 2 deletions src/coreclr/jit/loopcloning.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -241,7 +241,7 @@ JitExpandArrayStack<LcOptInfo*>* LoopCloneContext::GetLoopOptInfo(unsigned loopN
//
void LoopCloneContext::CancelLoopOptInfo(unsigned loopNum)
{
JITDUMP("Cancelling loop cloning for loop L%02u\n", loopNum);
JITDUMP("Cancelling loop cloning for loop " FMT_LP "\n", loopNum);
optInfo[loopNum] = nullptr;
if (conditions[loopNum] != nullptr)
{
Expand Down Expand Up @@ -454,7 +454,7 @@ void LoopCloneContext::EvaluateConditions(unsigned loopNum, bool* pAllTrue, bool

JitExpandArrayStack<LC_Condition>& conds = *conditions[loopNum];

JITDUMP("Evaluating %d loop cloning conditions for loop L%02u\n", conds.Size(), loopNum);
JITDUMP("Evaluating %d loop cloning conditions for loop " FMT_LP "\n", conds.Size(), loopNum);

assert(conds.Size() > 0);
for (unsigned i = 0; i < conds.Size(); ++i)
Expand Down
4 changes: 4 additions & 0 deletions src/coreclr/jit/loopcloning.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -176,6 +176,10 @@ exception occurs.
9. Constant initializations and constant limits must be non-negative (REVIEW: why? The
implementation does use `unsigned` to represent them.)
10. The cloned loop (the slow path) is not added to the loop table, meaning certain
downstream optimization passes do not see them. See
https://github.com/dotnet/runtime/issues/43713.
Assumptions
1. The assumption is that the optimization candidates collected during the
Expand Down
2 changes: 1 addition & 1 deletion src/coreclr/jit/morph.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -16643,7 +16643,7 @@ bool Compiler::fgFoldConditional(BasicBlock* block)
#ifdef DEBUG
if (verbose)
{
printf("Removing loop L%02u (from " FMT_BB " to " FMT_BB ")\n\n", loopNum,
printf("Removing loop " FMT_LP " (from " FMT_BB " to " FMT_BB ")\n\n", loopNum,
optLoopTable[loopNum].lpFirst->bbNum, optLoopTable[loopNum].lpBottom->bbNum);
}
#endif
Expand Down
Loading

0 comments on commit 3653f16

Please sign in to comment.