Skip to content

Commit

Permalink
JIT: speed up fgComputePreds (#35352)
Browse files Browse the repository at this point in the history
Interaction of `fgComputePreds` and `fgAddRefPred` could be quadratic in the
number of preds.

Usually the number of preds is small (1 or 2) but in some cases seen from
compiled regular expressions it could be in the thousands. On one such case
a single call to fgComputePreds was taking ~20% of jit time.

Since we build the pred list in sorted order we can take advantage of this
to avoid searching the list for potential duplicates in `fgAddRefPred` when
it is called from `fgComputePreds` -- the only possible duplicate entry is
at the end of the list.

This doesn't address perf of subsequent calls to `fgAddRefPred` but likely
those happen somewhat randomly and are unlikely to be as costly.
  • Loading branch information
AndyAyersMS authored Apr 24, 2020
1 parent a8476a1 commit af36c6d
Show file tree
Hide file tree
Showing 2 changed files with 65 additions and 21 deletions.
5 changes: 4 additions & 1 deletion src/coreclr/src/jit/block.h
Original file line number Diff line number Diff line change
Expand Up @@ -725,7 +725,10 @@ struct BasicBlock : private LIR::Range
m_firstNode = tree;
}

EntryState* bbEntryState; // verifier tracked state of all entries in stack.
union {
EntryState* bbEntryState; // verifier tracked state of all entries in stack.
flowList* bbLastPred; // last pred list entry
};

#define NO_BASE_TMP UINT_MAX // base# to use when we have none
unsigned bbStkTempsIn; // base# for input stack temps
Expand Down
81 changes: 61 additions & 20 deletions src/coreclr/src/jit/flowgraph.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1106,28 +1106,59 @@ flowList* Compiler::fgAddRefPred(BasicBlock* block,

assert(!fgCheapPredsValid);

flowList* flow;

// Keep the predecessor list in lowest to highest bbNum order. This allows us to discover the loops in
// optFindNaturalLoops from innermost to outermost.
//
// If we are initializing preds, we rely on the fact that we are adding references in increasing
// order of blockPred->bbNum to avoid searching the list.
//
// TODO-Throughput: Inserting an edge for a block in sorted order requires searching every existing edge.
// Thus, inserting all the edges for a block is quadratic in the number of edges. We need to either
// not bother sorting for debuggable code, or sort in optFindNaturalLoops, or better, make the code in
// optFindNaturalLoops not depend on order. This also requires ensuring that nobody else has taken a
// dependency on this order. Note also that we don't allow duplicates in the list; we maintain a flDupCount
// count of duplication. This also necessitates walking the flow list for every edge we add.

//
flowList* flow = nullptr;
flowList** listp = &block->bbPreds;
while ((*listp != nullptr) && ((*listp)->flBlock->bbNum < blockPred->bbNum))

if (initializingPreds)
{
listp = &(*listp)->flNext;
// List is sorted order and we're adding references in
// increasing blockPred->bbNum order. The only possible
// dup list entry is the last one.
//
flowList* flowLast = block->bbLastPred;
if (flowLast != nullptr)
{
listp = &flowLast->flNext;

assert(flowLast->flBlock->bbNum <= blockPred->bbNum);

if (flowLast->flBlock == blockPred)
{
flow = flowLast;
}
}
}
else
{
// References are added randomly, so we have to search.
//
while ((*listp != nullptr) && ((*listp)->flBlock->bbNum < blockPred->bbNum))
{
listp = &(*listp)->flNext;
}

if ((*listp != nullptr) && ((*listp)->flBlock == blockPred))
{
flow = *listp;
}
}

if ((*listp != nullptr) && ((*listp)->flBlock == blockPred))
if (flow != nullptr)
{
// The predecessor block already exists in the flow list; simply add to its duplicate count.
flow = *listp;
noway_assert(flow->flDupCount > 0);
flow->flDupCount++;
}
Expand All @@ -1150,6 +1181,11 @@ flowList* Compiler::fgAddRefPred(BasicBlock* block,
flow->flBlock = blockPred;
flow->flDupCount = 1;

if (initializingPreds)
{
block->bbLastPred = flow;
}

if (fgHaveValidEdgeWeights)
{
// We are creating an edge from blockPred to block
Expand Down Expand Up @@ -2986,6 +3022,9 @@ void Compiler::fgRemoveCheapPred(BasicBlock* block, BasicBlock* blockPred)
}
}

//------------------------------------------------------------------------
// fgRemovePreds - remove all pred information from blocks
//
void Compiler::fgRemovePreds()
{
C_ASSERT(offsetof(BasicBlock, bbPreds) ==
Expand All @@ -3001,10 +3040,13 @@ void Compiler::fgRemovePreds()
fgCheapPredsValid = false;
}

/*****************************************************************************
*
* Function called to compute the bbPreds lists.
*/
//------------------------------------------------------------------------
// fgComputePreds - compute the bbPreds lists
//
// Notes:
// Resets and then fills in the list of predecessors for each basic
// block. Assumes blocks (via bbNext) are in increasing bbNum order.
//
void Compiler::fgComputePreds()
{
noway_assert(fgFirstBB);
Expand All @@ -3020,21 +3062,20 @@ void Compiler::fgComputePreds()
}
#endif // DEBUG

// reset the refs count for each basic block

for (block = fgFirstBB; block; block = block->bbNext)
// Reset everything pred related
for (BasicBlock* block = fgFirstBB; block != nullptr; block = block->bbNext)
{
block->bbRefs = 0;
block->bbPreds = nullptr;
block->bbLastPred = nullptr;
block->bbRefs = 0;
}

/* the first block is always reachable! */
// the first block is always reachable
fgFirstBB->bbRefs = 1;

/* Treat the initial block as a jump target */
// Treat the initial block as a jump target
fgFirstBB->bbFlags |= BBF_JMP_TARGET | BBF_HAS_LABEL;

fgRemovePreds();

for (block = fgFirstBB; block; block = block->bbNext)
{
switch (block->bbJumpKind)
Expand Down Expand Up @@ -6948,7 +6989,7 @@ PhaseStatus Compiler::fgImport()
if ((block->bbFlags & BBF_IMPORTED) != 0)
{
// Assume if we generate any IR for the block we generate IR for the entire block.
if (!block->isEmpty())
if (block->firstStmt() != nullptr)
{
IL_OFFSET beginOffset = block->bbCodeOffs;
IL_OFFSET endOffset = block->bbCodeOffsEnd;
Expand Down

0 comments on commit af36c6d

Please sign in to comment.