Skip to content

Commit

Permalink
Add more iterators to JIT (#52515)
Browse files Browse the repository at this point in the history 
Add more iterators compatible with range-based `for` syntax for various data structures. These iterators all assume (and some check) that the underlying data structures determining the iteration are not changed during the iteration. For example, don't use these to iterate over the predecessor edges if you are changing the order or contents of the predecessor edge list.

- BasicBlock: iterate over all blocks in the function, a subset starting not at the first block, or a specified range of blocks. Removed uses of the `foreach_block` macro. E.g.:
```
for (BasicBlock* const block : Blocks()) // all blocks in function
for (BasicBlock* const block : BasicBlockSimpleList(fgFirstBB->bbNext)) // all blocks starting at fgFirstBB->bbNext
for (BasicBlock* const testBlock : BasicBlockRangeList(firstNonLoopBlock, lastNonLoopBlock)) // all blocks in range (inclusive)
```

- block predecessors: iterate over all predecessor edges, or all predecessor blocks, e.g.:
```
for (flowList* const edge : block->PredEdges())
for (BasicBlock* const predBlock : block->PredBlocks())
```

- block successors: iterate over all block successors using the `NumSucc()/GetSucc()`, or `NumSucc(Compiler*)/GetSucc(Compiler*)` pairs, e.g.:
```
for (BasicBlock* const succ : Succs())
for (BasicBlock* const succ : Succs(compiler))
```
Note that there already exists the "AllSuccessorsIter" which iterates over block successors including possible EH successors, e.g.:
```
for (BasicBlock* succ : block->GetAllSuccs(m_pCompiler))
```

- switch targets, (namely, the successors of `BBJ_SWITCH` blocks), e.g.:
```
for (BasicBlock* const bTarget : block->SwitchTargets())
```

- loops blocks: iterate over all the blocks in a loop, e.g.:
```
for (BasicBlock* const blk : optLoopTable[loopInd].LoopBlocks())
```

- Statements: added an iterator shortcut for the non-phi statements, e.g.:
```
for (Statement* const stmt : block->NonPhiStatements())
```
Note that there already exists an iterator over all statements, e.g.:
```
for (Statement* const stmt : block->Statements())
```

- EH clauses, e.g.:
```
for (EHblkDsc* const HBtab : EHClauses(this))
```

- GenTree in linear order (but not LIR, which already has an iterator), namely, using the `gtNext` links, e.g.:
```
for (GenTree* const call : stmt->TreeList())
```

This is a no-diff change.
  • Loading branch information
@BruceForstall
BruceForstall committed Jun 7, 2021
1 parent d173cca commit 5788ea0
Show file tree
Hide file tree
Showing 45 changed files with 1,269 additions and 928 deletions.
20 changes: 10 additions & 10 deletions src/coreclr/jit/assertionprop.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -4818,15 +4818,15 @@ ASSERT_TP* Compiler::optComputeAssertionGen()
{
ASSERT_TP* jumpDestGen = fgAllocateTypeForEachBlk<ASSERT_TP>();

for (BasicBlock* block = fgFirstBB; block != nullptr; block = block->bbNext)
for (BasicBlock* const block : Blocks())
{
ASSERT_TP valueGen = BitVecOps::MakeEmpty(apTraits);
GenTree* jtrue = nullptr;

// Walk the statement trees in this basic block.
for (Statement* stmt : block->Statements())
for (Statement* const stmt : block->Statements())
{
for (GenTree* tree = stmt->GetTreeList(); tree != nullptr; tree = tree->gtNext)
for (GenTree* const tree : stmt->TreeList())
{
if (tree->gtOper == GT_JTRUE)
{
Expand Down Expand Up @@ -4930,7 +4930,7 @@ ASSERT_TP* Compiler::optInitAssertionDataflowFlags()

// Initially estimate the OUT sets to everything except killed expressions
// Also set the IN sets to 1, so that we can perform the intersection.
for (BasicBlock* block = fgFirstBB; block; block = block->bbNext)
for (BasicBlock* const block : Blocks())
{
block->bbAssertionIn = BitVecOps::MakeCopy(apTraits, apValidFull);
block->bbAssertionGen = BitVecOps::MakeEmpty(apTraits);
Expand Down Expand Up @@ -5338,7 +5338,7 @@ void Compiler::optAssertionPropMain()
noway_assert(optAssertionCount == 0);

// First discover all value assignments and record them in the table.
for (BasicBlock* block = fgFirstBB; block; block = block->bbNext)
for (BasicBlock* const block : Blocks())
{
compCurBB = block;

Expand Down Expand Up @@ -5375,7 +5375,7 @@ void Compiler::optAssertionPropMain()
}

// Perform assertion gen for control flow based assertions.
for (GenTree* tree = stmt->GetTreeList(); tree != nullptr; tree = tree->gtNext)
for (GenTree* const tree : stmt->TreeList())
{
optAssertionGen(tree);
}
Expand All @@ -5390,7 +5390,7 @@ void Compiler::optAssertionPropMain()
// Zero out the bbAssertionIn values, as these can be referenced in RangeCheck::MergeAssertion
// and this is sharedstate with the CSE phase: bbCseIn
//
for (BasicBlock* block = fgFirstBB; block; block = block->bbNext)
for (BasicBlock* const block : Blocks())
{
block->bbAssertionIn = BitVecOps::MakeEmpty(apTraits);
}
Expand All @@ -5410,7 +5410,7 @@ void Compiler::optAssertionPropMain()
AssertionPropFlowCallback ap(this, bbJtrueAssertionOut, jumpDestGen);
flow.ForwardAnalysis(ap);

for (BasicBlock* block = fgFirstBB; block; block = block->bbNext)
for (BasicBlock* const block : Blocks())
{
// Compute any implied non-Null assertions for block->bbAssertionIn
optImpliedByTypeOfAssertions(block->bbAssertionIn);
Expand All @@ -5420,7 +5420,7 @@ void Compiler::optAssertionPropMain()
if (verbose)
{
printf("\n");
for (BasicBlock* block = fgFirstBB; block; block = block->bbNext)
for (BasicBlock* const block : Blocks())
{
printf("\n" FMT_BB, block->bbNum);
printf(" valueIn = %s", BitVecOps::ToString(apTraits, block->bbAssertionIn));
Expand All @@ -5438,7 +5438,7 @@ void Compiler::optAssertionPropMain()
ASSERT_TP assertions = BitVecOps::MakeEmpty(apTraits);

// Perform assertion propagation (and constant folding)
for (BasicBlock* block = fgFirstBB; block; block = block->bbNext)
for (BasicBlock* const block : Blocks())
{
BitVecOps::Assign(apTraits, assertions, block->bbAssertionIn);

Expand Down
82 changes: 36 additions & 46 deletions src/coreclr/jit/block.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -163,10 +163,9 @@ flowList* Compiler::BlockPredsWithEH(BasicBlock* blk)
// Find the first block of the try.
EHblkDsc* ehblk = ehGetDsc(tryIndex);
BasicBlock* tryStart = ehblk->ebdTryBeg;
for (flowList* tryStartPreds = tryStart->bbPreds; tryStartPreds != nullptr;
tryStartPreds = tryStartPreds->flNext)
for (BasicBlock* const tryStartPredBlock : tryStart->PredBlocks())
{
res = new (this, CMK_FlowList) flowList(tryStartPreds->getBlock(), res);
res = new (this, CMK_FlowList) flowList(tryStartPredBlock, res);

#if MEASURE_BLOCK_SIZE
genFlowNodeCnt += 1;
Expand All @@ -182,7 +181,7 @@ flowList* Compiler::BlockPredsWithEH(BasicBlock* blk)
// (plus adding in any filter blocks outside the try whose exceptions are handled here).
// That doesn't work, however: funclets have caused us to sometimes split the body of a try into
// more than one sequence of contiguous blocks. We need to find a better way to do this.
for (BasicBlock* bb = fgFirstBB; bb != nullptr; bb = bb->bbNext)
for (BasicBlock* const bb : Blocks())
{
if (bbInExnFlowRegions(tryIndex, bb) && !bb->isBBCallAlwaysPairTail())
{
Expand Down Expand Up @@ -217,9 +216,9 @@ flowList* Compiler::BlockPredsWithEH(BasicBlock* blk)
bool BasicBlock::checkPredListOrder()
{
unsigned lastBBNum = 0;
for (flowList* pred = bbPreds; pred != nullptr; pred = pred->flNext)
for (BasicBlock* const predBlock : PredBlocks())
{
const unsigned bbNum = pred->getBlock()->bbNum;
const unsigned bbNum = predBlock->bbNum;
if (bbNum <= lastBBNum)
{
assert(bbNum != lastBBNum);
Expand Down Expand Up @@ -261,7 +260,7 @@ void BasicBlock::reorderPredList(Compiler* compiler)
// Count number or entries.
//
int count = 0;
for (flowList* pred = bbPreds; pred != nullptr; pred = pred->flNext)
for (flowList* const pred : PredEdges())
{
count++;
}
Expand All @@ -286,7 +285,7 @@ void BasicBlock::reorderPredList(Compiler* compiler)

// Fill in the vector from the list.
//
for (flowList* pred = bbPreds; pred != nullptr; pred = pred->flNext)
for (flowList* const pred : PredEdges())
{
sortVector->push_back(pred);
}
Expand Down Expand Up @@ -516,7 +515,7 @@ void BasicBlock::dspFlags()
unsigned BasicBlock::dspPreds()
{
unsigned count = 0;
for (flowList* pred = bbPreds; pred != nullptr; pred = pred->flNext)
for (flowList* const pred : PredEdges())
{
if (count != 0)
{
Expand Down Expand Up @@ -575,20 +574,16 @@ unsigned BasicBlock::dspCheapPreds()
/*****************************************************************************
*
* Display the basic block successors.
* Returns the count of successors.
*/

unsigned BasicBlock::dspSuccs(Compiler* compiler)
void BasicBlock::dspSuccs(Compiler* compiler)
{
unsigned numSuccs = NumSucc(compiler);
unsigned count = 0;
for (unsigned i = 0; i < numSuccs; i++)
bool first = true;
for (BasicBlock* const succ : Succs(compiler))
{
printf("%s", (count == 0) ? "" : ",");
printf(FMT_BB, GetSucc(i, compiler)->bbNum);
count++;
printf("%s" FMT_BB, first ? "" : ",", succ->bbNum);
first = false;
}
return count;
}

// Display a compact representation of the bbJumpKind, that is, where this block branches.
Expand Down Expand Up @@ -773,7 +768,7 @@ bool BasicBlock::CloneBlockState(
to->bbTgtStkDepth = from->bbTgtStkDepth;
#endif // DEBUG

for (Statement* fromStmt : from->Statements())
for (Statement* const fromStmt : from->Statements())
{
auto newExpr = compiler->gtCloneExpr(fromStmt->GetRootNode(), GTF_EMPTY, varNum, varVal);
if (!newExpr)
Expand All @@ -800,7 +795,7 @@ void BasicBlock::MakeLIR(GenTree* firstNode, GenTree* lastNode)
bbFlags |= BBF_IS_LIR;
}

bool BasicBlock::IsLIR()
bool BasicBlock::IsLIR() const
{
assert(isValid());
const bool isLIR = ((bbFlags & BBF_IS_LIR) != 0);
Expand Down Expand Up @@ -845,15 +840,15 @@ Statement* BasicBlock::lastStmt() const
//------------------------------------------------------------------------
// BasicBlock::firstNode: Returns the first node in the block.
//
GenTree* BasicBlock::firstNode()
GenTree* BasicBlock::firstNode() const
{
return IsLIR() ? GetFirstLIRNode() : Compiler::fgGetFirstNode(firstStmt()->GetRootNode());
}

//------------------------------------------------------------------------
// BasicBlock::lastNode: Returns the last node in the block.
//
GenTree* BasicBlock::lastNode()
GenTree* BasicBlock::lastNode() const
{
return IsLIR() ? m_lastNode : lastStmt()->GetRootNode();
}
Expand All @@ -873,7 +868,7 @@ GenTree* BasicBlock::lastNode()
// a backedge), we never want to consider it "unique" because the prolog is an
// implicit predecessor.

BasicBlock* BasicBlock::GetUniquePred(Compiler* compiler)
BasicBlock* BasicBlock::GetUniquePred(Compiler* compiler) const
{
if ((bbPreds == nullptr) || (bbPreds->flNext != nullptr) || (this == compiler->fgFirstBB))
{
Expand All @@ -895,7 +890,7 @@ BasicBlock* BasicBlock::GetUniquePred(Compiler* compiler)
// Return Value:
// The unique successor of a block, or nullptr if there is no unique successor.

BasicBlock* BasicBlock::GetUniqueSucc()
BasicBlock* BasicBlock::GetUniqueSucc() const
{
if (bbJumpKind == BBJ_ALWAYS)
{
Expand Down Expand Up @@ -933,20 +928,16 @@ unsigned JitPtrKeyFuncs<BasicBlock>::GetHashCode(const BasicBlock* ptr)
// True if block is empty, or contains only PHI assignments,
// or contains zero or more PHI assignments followed by NOPs.
//
bool BasicBlock::isEmpty()
bool BasicBlock::isEmpty() const
{
if (!IsLIR())
{
Statement* stmt = FirstNonPhiDef();

while (stmt != nullptr)
for (Statement* const stmt : NonPhiStatements())
{
if (!stmt->GetRootNode()->OperIs(GT_NOP))
{
return false;
}

stmt = stmt->GetNextStmt();
}
}
else
Expand All @@ -969,7 +960,7 @@ bool BasicBlock::isEmpty()
// Return Value:
// True if it a valid basic block.
//
bool BasicBlock::isValid()
bool BasicBlock::isValid() const
{
const bool isLIR = ((bbFlags & BBF_IS_LIR) != 0);
if (isLIR)
Expand All @@ -984,7 +975,7 @@ bool BasicBlock::isValid()
}
}

Statement* BasicBlock::FirstNonPhiDef()
Statement* BasicBlock::FirstNonPhiDef() const
{
Statement* stmt = firstStmt();
if (stmt == nullptr)
Expand All @@ -1005,7 +996,7 @@ Statement* BasicBlock::FirstNonPhiDef()
return stmt;
}

Statement* BasicBlock::FirstNonPhiDefOrCatchArgAsg()
Statement* BasicBlock::FirstNonPhiDefOrCatchArgAsg() const
{
Statement* stmt = FirstNonPhiDef();
if (stmt == nullptr)
Expand All @@ -1026,11 +1017,10 @@ Statement* BasicBlock::FirstNonPhiDefOrCatchArgAsg()
* Can a BasicBlock be inserted after this without altering the flowgraph
*/

bool BasicBlock::bbFallsThrough()
bool BasicBlock::bbFallsThrough() const
{
switch (bbJumpKind)
{

case BBJ_THROW:
case BBJ_EHFINALLYRET:
case BBJ_EHFILTERRET:
Expand Down Expand Up @@ -1063,7 +1053,7 @@ bool BasicBlock::bbFallsThrough()
// Return Value:
// Count of block successors.
//
unsigned BasicBlock::NumSucc()
unsigned BasicBlock::NumSucc() const
{
switch (bbJumpKind)
{
Expand Down Expand Up @@ -1107,7 +1097,7 @@ unsigned BasicBlock::NumSucc()
// Return Value:
// Requested successor block
//
BasicBlock* BasicBlock::GetSucc(unsigned i)
BasicBlock* BasicBlock::GetSucc(unsigned i) const
{
assert(i < NumSucc()); // Index bounds check.
switch (bbJumpKind)
Expand Down Expand Up @@ -1279,7 +1269,7 @@ void BasicBlock::InitVarSets(Compiler* comp)
}

// Returns true if the basic block ends with GT_JMP
bool BasicBlock::endsWithJmpMethod(Compiler* comp)
bool BasicBlock::endsWithJmpMethod(Compiler* comp) const
{
if (comp->compJmpOpUsed && (bbJumpKind == BBJ_RETURN) && (bbFlags & BBF_HAS_JMP))
{
Expand All @@ -1299,7 +1289,7 @@ bool BasicBlock::endsWithJmpMethod(Compiler* comp)
// comp - Compiler instance
// fastTailCallsOnly - Only consider fast tail calls excluding tail calls via helper.
//
bool BasicBlock::endsWithTailCallOrJmp(Compiler* comp, bool fastTailCallsOnly /*=false*/)
bool BasicBlock::endsWithTailCallOrJmp(Compiler* comp, bool fastTailCallsOnly /*=false*/) const
{
GenTree* tailCall = nullptr;
bool tailCallsConvertibleToLoopOnly = false;
Expand All @@ -1326,7 +1316,7 @@ bool BasicBlock::endsWithTailCallOrJmp(Compiler* comp, bool fastTailCallsOnly /*
bool BasicBlock::endsWithTailCall(Compiler* comp,
bool fastTailCallsOnly,
bool tailCallsConvertibleToLoopOnly,
GenTree** tailCall)
GenTree** tailCall) const
{
assert(!fastTailCallsOnly || !tailCallsConvertibleToLoopOnly);
*tailCall = nullptr;
Expand Down Expand Up @@ -1393,7 +1383,7 @@ bool BasicBlock::endsWithTailCall(Compiler* comp,
// Return Value:
// true if the block ends with a tail call convertible to loop.
//
bool BasicBlock::endsWithTailCallConvertibleToLoop(Compiler* comp, GenTree** tailCall)
bool BasicBlock::endsWithTailCallConvertibleToLoop(Compiler* comp, GenTree** tailCall) const
{
bool fastTailCallsOnly = false;
bool tailCallsConvertibleToLoopOnly = true;
Expand Down Expand Up @@ -1522,7 +1512,7 @@ BasicBlock* Compiler::bbNewBasicBlock(BBjumpKinds jumpKind)

//------------------------------------------------------------------------
// isBBCallAlwaysPair: Determine if this is the first block of a BBJ_CALLFINALLY/BBJ_ALWAYS pair

//
// Return Value:
// True iff "this" is the first block of a BBJ_CALLFINALLY/BBJ_ALWAYS pair
// -- a block corresponding to an exit from the try of a try/finally.
Expand All @@ -1540,7 +1530,7 @@ BasicBlock* Compiler::bbNewBasicBlock(BBjumpKinds jumpKind)
// "retless" BBJ_CALLFINALLY blocks due to a requirement to use the BBJ_ALWAYS for
// generating code.
//
bool BasicBlock::isBBCallAlwaysPair()
bool BasicBlock::isBBCallAlwaysPair() const
{
#if defined(FEATURE_EH_FUNCLETS) && defined(TARGET_ARM)
if (this->bbJumpKind == BBJ_CALLFINALLY)
Expand Down Expand Up @@ -1576,7 +1566,7 @@ bool BasicBlock::isBBCallAlwaysPair()
// Notes:
// See notes on isBBCallAlwaysPair(), above.
//
bool BasicBlock::isBBCallAlwaysPairTail()
bool BasicBlock::isBBCallAlwaysPairTail() const
{
return (bbPrev != nullptr) && bbPrev->isBBCallAlwaysPair();
}
Expand All @@ -1597,7 +1587,7 @@ bool BasicBlock::isBBCallAlwaysPairTail()
// this block might be entered via flow that is not represented by an edge
// in the flowgraph.
//
bool BasicBlock::hasEHBoundaryIn()
bool BasicBlock::hasEHBoundaryIn() const
{
bool returnVal = (bbCatchTyp != BBCT_NONE);
if (!returnVal)
Expand All @@ -1621,7 +1611,7 @@ bool BasicBlock::hasEHBoundaryIn()
// live in registers if any successor is a normal flow edge. That's because the
// EH write-thru semantics ensure that we always have an up-to-date value on the stack.
//
bool BasicBlock::hasEHBoundaryOut()
bool BasicBlock::hasEHBoundaryOut() const
{
bool returnVal = false;
if (bbJumpKind == BBJ_EHFILTERRET)
Expand Down
Loading

0 comments on commit 5788ea0

Please sign in to comment.