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.

Sign up for GitHub

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

Jump to bottom

JIT: Clean up BB successor iteration #86839

Merged
merged 2 commits into from
Jun 6, 2023
Merged

JIT: Clean up BB successor iteration #86839

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
55bf122
JIT: Clean up BB successor iteration
jakobbotsch May 27, 2023
1f49b3b
Function header
jakobbotsch May 27, 2023
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
98 changes: 24 additions & 74 deletions src/coreclr/jit/block.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -68,91 +68,41 @@ unsigned SsaStressHashHelper()
}
#endif

EHSuccessorIterPosition::EHSuccessorIterPosition(Compiler* comp, BasicBlock* block)
: m_remainingRegSuccs(block->NumSucc(comp))
, m_numRegSuccs(m_remainingRegSuccs)
, m_curRegSucc(nullptr)
, m_curTry(comp->ehGetBlockExnFlowDsc(block))
//------------------------------------------------------------------------
// AllSuccessorEnumerator: Construct an instance of the enumerator.
//
// Arguments:
// comp - Compiler instance
// block - The block whose successors are to be iterated
//
AllSuccessorEnumerator::AllSuccessorEnumerator(Compiler* comp, BasicBlock* block) : m_block(block)
{
// If "block" is a "leave helper" block (the empty BBJ_ALWAYS block that pairs with a
// preceding BBJ_CALLFINALLY block to implement a "leave" IL instruction), then no exceptions
// can occur within it, so clear m_curTry if it's non-null.
if (m_curTry != nullptr)
{
if (block->isBBCallAlwaysPairTail())
m_numSuccs = 0;
block->VisitAllSuccs(comp, [this](BasicBlock* succ) {
if (m_numSuccs < ArrLen(m_successors))
{
m_curTry = nullptr;
m_successors[m_numSuccs] = succ;
}
}

if (m_curTry == nullptr && m_remainingRegSuccs > 0)
{
// Examine the successors to see if any are the start of try blocks.
FindNextRegSuccTry(comp, block);
}
}

void EHSuccessorIterPosition::FindNextRegSuccTry(Compiler* comp, BasicBlock* block)
{
assert(m_curTry == nullptr);
m_numSuccs++;
return BasicBlockVisit::Continue;
});

// Must now consider the next regular successor, if any.
while (m_remainingRegSuccs > 0)
if (m_numSuccs > ArrLen(m_successors))
{
m_curRegSucc = block->GetSucc(m_numRegSuccs - m_remainingRegSuccs, comp);
m_remainingRegSuccs--;
if (comp->bbIsTryBeg(m_curRegSucc))
{
assert(m_curRegSucc->hasTryIndex()); // Since it is a try begin.
unsigned newTryIndex = m_curRegSucc->getTryIndex();
m_pSuccessors = new (comp, CMK_BasicBlock) BasicBlock*[m_numSuccs];

// If the try region started by "m_curRegSucc" (represented by newTryIndex) contains m_block,
// we've already yielded its handler, as one of the EH handler successors of m_block itself.
if (comp->bbInExnFlowRegions(newTryIndex, block))
{
continue;
}
unsigned numSuccs = 0;
block->VisitAllSuccs(comp, [this, &numSuccs](BasicBlock* succ) {
assert(numSuccs < m_numSuccs);
m_pSuccessors[numSuccs++] = succ;
return BasicBlockVisit::Continue;
});

// Otherwise, consider this try.
m_curTry = comp->ehGetDsc(newTryIndex);
break;
}
assert(numSuccs == m_numSuccs);
}
}

void EHSuccessorIterPosition::Advance(Compiler* comp, BasicBlock* block)
{
assert(m_curTry != nullptr);
if (m_curTry->ebdEnclosingTryIndex != EHblkDsc::NO_ENCLOSING_INDEX)
{
m_curTry = comp->ehGetDsc(m_curTry->ebdEnclosingTryIndex);

// If we've gone over into considering try's containing successors,
// then the enclosing try must have the successor as its first block.
if (m_curRegSucc == nullptr || m_curTry->ebdTryBeg == m_curRegSucc)
{
return;
}

// Otherwise, give up, try the next regular successor.
m_curTry = nullptr;
}
else
{
m_curTry = nullptr;
}

// We've exhausted all try blocks.
// See if there are any remaining regular successors that start try blocks.
FindNextRegSuccTry(comp, block);
}

BasicBlock* EHSuccessorIterPosition::Current(Compiler* comp, BasicBlock* block)
{
assert(m_curTry != nullptr);
return m_curTry->ExFlowBlock();
}

FlowEdge* Compiler::BlockPredsWithEH(BasicBlock* blk)
{
BlockToFlowEdgeMap* ehPreds = GetBlockToEHPreds();
Expand Down
224 changes: 19 additions & 205 deletions src/coreclr/jit/block.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -199,133 +199,6 @@ struct allMemoryKinds
}
};

// This encapsulates the "exception handling" successors of a block. That is,
// if a basic block BB1 occurs in a try block, we consider the first basic block
// BB2 of the corresponding handler to be an "EH successor" of BB1. Because we
// make the conservative assumption that control flow can jump from a try block
// to its handler at any time, the immediate (regular control flow)
// predecessor(s) of the first block of a try block are also considered to
// have the first block of the handler as an EH successor. This makes variables that
// are "live-in" to the handler become "live-out" for these try-predecessor block,
// so that they become live-in to the try -- which we require.
//
// This class maintains the minimum amount of state necessary to implement
// successor iteration. The basic block whose successors are enumerated and
// the compiler need to be provided by Advance/Current's callers. In addition
// to iterators, this allows the use of other approaches that are more space
// efficient.
class EHSuccessorIterPosition
{
// The number of "regular" (i.e., non-exceptional) successors that remain to
// be considered. If BB1 has successor BB2, and BB2 is the first block of a
// try block, then we consider the catch block of BB2's try to be an EH
// successor of BB1. This captures the iteration over the successors of BB1
// for this purpose. (In reverse order; we're done when this field is 0).
unsigned m_remainingRegSuccs;
unsigned m_numRegSuccs;

// The current "regular" successor of "m_block" that we're considering.
BasicBlock* m_curRegSucc;

// The current try block. If non-null, then the current successor "m_curRegSucc"
// is the first block of the handler of this block. While this try block has
// enclosing try's that also start with "m_curRegSucc", the corresponding handlers will be
// further EH successors.
EHblkDsc* m_curTry;

// Requires that "m_curTry" is NULL. Determines whether there is, as
// discussed just above, a regular successor that's the first block of a
// try; if so, sets "m_curTry" to that try block. (As noted above, selecting
// the try containing the current regular successor as the "current try" may cause
// multiple first-blocks of catches to be yielded as EH successors: trys enclosing
// the current try are also included if they also start with the current EH successor.)
void FindNextRegSuccTry(Compiler* comp, BasicBlock* block);

public:
// Constructs a position that "points" to the first EH successor of `block`.
EHSuccessorIterPosition(Compiler* comp, BasicBlock* block);

// Constructs a position that "points" past the last EH successor of `block` ("end" position).
EHSuccessorIterPosition() : m_remainingRegSuccs(0), m_numRegSuccs(0), m_curTry(nullptr)
{
}

// Go on to the next EH successor.
void Advance(Compiler* comp, BasicBlock* block);

// Returns the current EH successor.
// Requires that "*this" is not equal to the "end" position.
BasicBlock* Current(Compiler* comp, BasicBlock* block);

// Returns "true" iff "*this" is equal to "ehsi".
bool operator==(const EHSuccessorIterPosition& ehsi)
{
return m_curTry == ehsi.m_curTry && m_remainingRegSuccs == ehsi.m_remainingRegSuccs;
}

bool operator!=(const EHSuccessorIterPosition& ehsi)
{
return !((*this) == ehsi);
}
};

// Yields both normal and EH successors (in that order) in one iteration.
//
// This class maintains the minimum amount of state necessary to implement
// successor iteration. The basic block whose successors are enumerated and
// the compiler need to be provided by Advance/Current's callers. In addition
// to iterators, this allows the use of other approaches that are more space
// efficient.
class AllSuccessorIterPosition
{
// Normal successor position
unsigned m_numNormSuccs;
unsigned m_remainingNormSucc;
// EH successor position
EHSuccessorIterPosition m_ehIter;

// True iff m_blk is a BBJ_CALLFINALLY block, and the current try block of m_ehIter,
// the first block of whose handler would be next yielded, is the jump target of m_blk.
inline bool CurTryIsBlkCallFinallyTarget(Compiler* comp, BasicBlock* block);

public:
// Constructs a position that "points" to the first successor of `block`.
inline AllSuccessorIterPosition(Compiler* comp, BasicBlock* block);

// Constructs a position that "points" past the last successor of `block` ("end" position).
AllSuccessorIterPosition() : m_remainingNormSucc(0), m_ehIter()
{
}

// Go on to the next successor.
inline void Advance(Compiler* comp, BasicBlock* block);

// Returns the current successor.
// Requires that "*this" is not equal to the "end" position.
inline BasicBlock* Current(Compiler* comp, BasicBlock* block);

bool IsCurrentEH()
{
return m_remainingNormSucc == 0;
}

bool HasCurrent()
{
return *this != AllSuccessorIterPosition();
}

// Returns "true" iff "*this" is equal to "asi".
bool operator==(const AllSuccessorIterPosition& asi)
{
return (m_remainingNormSucc == asi.m_remainingNormSucc) && (m_ehIter == asi.m_ehIter);
}

bool operator!=(const AllSuccessorIterPosition& asi)
{
return !((*this) == asi);
}
};

// PredEdgeList: adapter class for forward iteration of the predecessor edge linked list using range-based `for`,
// normally used via BasicBlock::PredEdges(), e.g.:
// for (FlowEdge* const edge : block->PredEdges()) ...
Expand Down Expand Up @@ -1347,16 +1220,6 @@ struct BasicBlock : private LIR::Range
}
};

Successors<EHSuccessorIterPosition> GetEHSuccs(Compiler* comp)
{
return Successors<EHSuccessorIterPosition>(comp, this);
}

Successors<AllSuccessorIterPosition> GetAllSuccs(Compiler* comp)
{
return Successors<AllSuccessorIterPosition>(comp, this);
}

template <typename TFunc>
BasicBlockVisit VisitAllSuccs(Compiler* comp, TFunc func);

Expand Down Expand Up @@ -1988,96 +1851,47 @@ inline PredBlockList::iterator& PredBlockList::iterator::operator++()

void* emitCodeGetCookie(BasicBlock* block);

AllSuccessorIterPosition::AllSuccessorIterPosition(Compiler* comp, BasicBlock* block)
: m_numNormSuccs(block->NumSucc(comp)), m_remainingNormSucc(m_numNormSuccs), m_ehIter(comp, block)
{
if (CurTryIsBlkCallFinallyTarget(comp, block))
{
m_ehIter.Advance(comp, block);
}
}

bool AllSuccessorIterPosition::CurTryIsBlkCallFinallyTarget(Compiler* comp, BasicBlock* block)
{
return (block->bbJumpKind == BBJ_CALLFINALLY) && (m_ehIter != EHSuccessorIterPosition()) &&
(block->bbJumpDest == m_ehIter.Current(comp, block));
}

void AllSuccessorIterPosition::Advance(Compiler* comp, BasicBlock* block)
{
if (m_remainingNormSucc > 0)
{
m_remainingNormSucc--;
}
else
{
m_ehIter.Advance(comp, block);

// If the original block whose successors we're iterating over
// is a BBJ_CALLFINALLY, that finally clause's first block
// will be yielded as a normal successor. Don't also yield as
// an exceptional successor.
if (CurTryIsBlkCallFinallyTarget(comp, block))
{
m_ehIter.Advance(comp, block);
}
}
}

// Requires that "this" is not equal to the standard "end" iterator. Returns the
// current successor.
BasicBlock* AllSuccessorIterPosition::Current(Compiler* comp, BasicBlock* block)
{
if (m_remainingNormSucc > 0)
{
return block->GetSucc(m_numNormSuccs - m_remainingNormSucc, comp);
}
else
{
return m_ehIter.Current(comp, block);
}
}

typedef BasicBlock::Successors<EHSuccessorIterPosition>::iterator EHSuccessorIter;
typedef BasicBlock::Successors<AllSuccessorIterPosition>::iterator AllSuccessorIter;

// An enumerator of a block's all successors. In some cases (e.g. SsaBuilder::TopologicalSort)
// using iterators is not exactly efficient, at least because they contain an unnecessary
// member - a pointer to the Compiler object.
class AllSuccessorEnumerator
{
BasicBlock* m_block;
AllSuccessorIterPosition m_pos;
BasicBlock* m_block;
union {
// We store up to 4 successors inline in the enumerator. For ASP.NET
// and libraries.pmi this is enough in 99.7% of cases.
BasicBlock* m_successors[4];
BasicBlock** m_pSuccessors;
Comment on lines +1861 to +1864
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Some histograms for count of all-successors:
asp.net:

     <=          0 ===>  113751 count ( 15% of total)
      1 ..       1 ===>  232465 count ( 48% of total)
      2 ..       2 ===>  342117 count ( 96% of total)
      3 ..       3 ===>   23292 count ( 99% of total)
      4 ..       4 ===>    3215 count ( 99% of total)
      5 ..       5 ===>     700 count ( 99% of total)
      6 ..       6 ===>     392 count ( 99% of total)
      7 ..       7 ===>     165 count ( 99% of total)
      8 ..       8 ===>      99 count ( 99% of total)
      9 ..       9 ===>      76 count ( 99% of total)
     10 ..      10 ===>     152 count (100% of total)
      >         10 ===>     299 count (100% of total)

libraries.pmi:

     <=          0 ===>  291559 count ( 26% of total)
      1 ..       1 ===>  321737 count ( 56% of total)
      2 ..       2 ===>  438938 count ( 96% of total)
      3 ..       3 ===>   27017 count ( 99% of total)
      4 ..       4 ===>    4750 count ( 99% of total)
      5 ..       5 ===>    1642 count ( 99% of total)
      6 ..       6 ===>     522 count ( 99% of total)
      7 ..       7 ===>     261 count ( 99% of total)
      8 ..       8 ===>     161 count ( 99% of total)
      9 ..       9 ===>     130 count ( 99% of total)
     10 ..      10 ===>      79 count (100% of total)
      >         10 ===>     398 count (100% of total)

};

unsigned m_numSuccs;
unsigned m_curSucc = UINT_MAX;

public:
// Constructs an enumerator of all `block`'s successors.
AllSuccessorEnumerator(Compiler* comp, BasicBlock* block) : m_block(block), m_pos(comp, block)
{
}
AllSuccessorEnumerator(Compiler* comp, BasicBlock* block);

// Gets the block whose successors are enumerated.
BasicBlock* Block()
{
return m_block;
}

// Returns true if the next successor is an EH successor.
bool IsNextEHSuccessor()
{
return m_pos.IsCurrentEH();
}

// Returns the next available successor or `nullptr` if there are no more successors.
BasicBlock* NextSuccessor(Compiler* comp)
{
if (!m_pos.HasCurrent())
m_curSucc++;
if (m_curSucc >= m_numSuccs)
{
return nullptr;
}

BasicBlock* succ = m_pos.Current(comp, m_block);
m_pos.Advance(comp, m_block);
return succ;
if (m_numSuccs <= ArrLen(m_successors))
{
return m_successors[m_curSucc];
}

return m_pSuccessors[m_curSucc];
}
};

Expand Down
Loading