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: fix remaining phases that were rebuilding pred edges #80769

Merged
merged 2 commits into from
Jan 18, 2023
Merged

JIT: fix remaining phases that were rebuilding pred edges #80769

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
63ff3a1
JIT: fix remaining phases that were rebuilding pred edges
AndyAyersMS Jan 14, 2023
ca77fc5
also fix #80772
AndyAyersMS Jan 18, 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
26 changes: 21 additions & 5 deletions src/coreclr/jit/compiler.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1511,6 +1511,24 @@ enum API_ICorJitInfo_Names
API_COUNT
};

enum class FlowGraphUpdates
{
COMPUTE_BASICS = 0, // renumber blocks, reachability, etc
COMPUTE_DOMS = 1 << 0, // recompute dominators
COMPUTE_RETURNS = 1 << 1, // recompute return blocks
COMPUTE_LOOPS = 1 << 2, // recompute loop table
};

inline constexpr FlowGraphUpdates operator|(FlowGraphUpdates a, FlowGraphUpdates b)
{
return (FlowGraphUpdates)((unsigned int)a | (unsigned int)b);
}

inline constexpr FlowGraphUpdates operator&(FlowGraphUpdates a, FlowGraphUpdates b)
{
return (FlowGraphUpdates)((unsigned int)a & (unsigned int)b);
}

//---------------------------------------------------------------
// Compilation time.
//
Expand Down Expand Up @@ -5140,12 +5158,10 @@ class Compiler
// && postOrder(A) >= postOrder(B) making the computation O(1).
void fgNumberDomTree(DomTreeNode* domTree);

// When the flow graph changes, we need to update the block numbers, predecessor lists, reachability sets,
// When the flow graph changes, we need to update the block numbers, reachability sets,
// dominators, and possibly loops.
void fgUpdateChangedFlowGraph(const bool computePreds = true,
const bool computeDoms = true,
const bool computeReturnBlocks = false,
const bool computeLoops = false);
//
void fgUpdateChangedFlowGraph(FlowGraphUpdates updates);

public:
// Compute the predecessors of the blocks in the control flow graph.
Expand Down
26 changes: 12 additions & 14 deletions src/coreclr/jit/fgopt.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -174,16 +174,19 @@ bool Compiler::fgReachable(BasicBlock* b1, BasicBlock* b2)
// to be recomputed if they know certain things do NOT need to be recomputed.
//
// Arguments:
// computePreds -- `true` if we should recompute predecessors
// computeDoms -- `true` if we should recompute dominators
// computeReturnBlocks -- `true` if we should recompute the list of return blocks
// computeLoops -- `true` if we should recompute the loop table
//
void Compiler::fgUpdateChangedFlowGraph(const bool computePreds,
const bool computeDoms,
const bool computeReturnBlocks,
const bool computeLoops)
// updates -- enum flag set indicating what to update
//
// Notes:
// Always renumbers, computes enter blocks, and computes reachability.
// Optionally rebuilds dominators, return blocks, and computes loop information.
//
void Compiler::fgUpdateChangedFlowGraph(FlowGraphUpdates updates)
{
const bool computeDoms = ((updates & FlowGraphUpdates::COMPUTE_DOMS) == FlowGraphUpdates::COMPUTE_DOMS);
const bool computeReturnBlocks =
((updates & FlowGraphUpdates::COMPUTE_RETURNS) == FlowGraphUpdates::COMPUTE_RETURNS);
const bool computeLoops = ((updates & FlowGraphUpdates::COMPUTE_LOOPS) == FlowGraphUpdates::COMPUTE_LOOPS);

// We need to clear this so we don't hit an assert calling fgRenumberBlocks().
fgDomsComputed = false;

Expand All @@ -194,11 +197,6 @@ void Compiler::fgUpdateChangedFlowGraph(const bool computePreds,

JITDUMP("\nRenumbering the basic blocks for fgUpdateChangeFlowGraph\n");
fgRenumberBlocks();

if (computePreds) // This condition is only here until all phases don't require it.
{
fgComputePreds();
}
fgComputeEnterBlocksSet();
fgComputeReachabilitySets();
if (computeDoms)
Expand Down
57 changes: 12 additions & 45 deletions src/coreclr/jit/flowgraph.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -78,19 +78,9 @@ PhaseStatus Compiler::fgInsertGCPolls()

bool createdPollBlocks = false;

#ifdef DEBUG
if (verbose)
{
printf("*************** In fgInsertGCPolls() for %s\n", info.compFullName);
fgDispBasicBlocks(false);
printf("\n");
}
#endif // DEBUG

BasicBlock* block;

// Walk through the blocks and hunt for a block that needs a GC Poll
for (block = fgFirstBB; block != nullptr; block = block->bbNext)
//
for (BasicBlock* block = fgFirstBB; block != nullptr; block = block->bbNext)
{
compCurBB = block;

Expand All @@ -112,56 +102,36 @@ PhaseStatus Compiler::fgInsertGCPolls()

GCPollType pollType = GCPOLL_INLINE;

// We'd like to inset an inline poll. Below is the list of places where we
// We'd like to insert an inline poll. Below is the list of places where we
// can't or don't want to emit an inline poll. Check all of those. If after all of that we still
// have INLINE, then emit an inline check.

if (opts.OptimizationDisabled())
{
#ifdef DEBUG
if (verbose)
{
printf("Selecting CALL poll in block " FMT_BB " because of debug/minopts\n", block->bbNum);
}
#endif // DEBUG

// Don't split blocks and create inlined polls unless we're optimizing.
//
JITDUMP("Selecting CALL poll in block " FMT_BB " because of debug/minopts\n", block->bbNum);
pollType = GCPOLL_CALL;
}
else if (genReturnBB == block)
{
#ifdef DEBUG
if (verbose)
{
printf("Selecting CALL poll in block " FMT_BB " because it is the single return block\n", block->bbNum);
}
#endif // DEBUG

// we don't want to split the single return block
//
JITDUMP("Selecting CALL poll in block " FMT_BB " because it is the single return block\n", block->bbNum);
pollType = GCPOLL_CALL;
}
else if (BBJ_SWITCH == block->bbJumpKind)
{
#ifdef DEBUG
if (verbose)
{
printf("Selecting CALL poll in block " FMT_BB " because it is a SWITCH block\n", block->bbNum);
}
#endif // DEBUG

// We don't want to deal with all the outgoing edges of a switch block.
//
JITDUMP("Selecting CALL poll in block " FMT_BB " because it is a SWITCH block\n", block->bbNum);
pollType = GCPOLL_CALL;
}
else if ((block->bbFlags & BBF_COLD) != 0)
{
#ifdef DEBUG
if (verbose)
{
printf("Selecting CALL poll in block " FMT_BB " because it is a cold block\n", block->bbNum);
}
#endif // DEBUG

// We don't want to split a cold block.
//
JITDUMP("Selecting CALL poll in block " FMT_BB " because it is a cold block\n", block->bbNum);
pollType = GCPOLL_CALL;
}

Expand All @@ -176,9 +146,7 @@ PhaseStatus Compiler::fgInsertGCPolls()
{
noway_assert(opts.OptimizationEnabled());
fgReorderBlocks(/* useProfileData */ false);
constexpr bool computePreds = true;
constexpr bool computeDoms = false;
fgUpdateChangedFlowGraph(computePreds, computeDoms);
fgUpdateChangedFlowGraph(FlowGraphUpdates::COMPUTE_BASICS);
}

return result;
Expand All @@ -194,7 +162,6 @@ PhaseStatus Compiler::fgInsertGCPolls()
// Return Value:
// If new basic blocks are inserted, the last inserted block; otherwise, the input block.
//

BasicBlock* Compiler::fgCreateGCPoll(GCPollType pollType, BasicBlock* block)
{
bool createdPollBlocks;
Expand Down
3 changes: 1 addition & 2 deletions src/coreclr/jit/loopcloning.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3286,9 +3286,8 @@ PhaseStatus Compiler::optCloneLoops()
if (optLoopsCloned > 0)
{
JITDUMP("Recompute reachability and dominators after loop cloning\n");
constexpr bool computePreds = false;
// TODO: recompute the loop table, to include the slow loop path in the table?
fgUpdateChangedFlowGraph(computePreds);
fgUpdateChangedFlowGraph(FlowGraphUpdates::COMPUTE_DOMS);
}

#ifdef DEBUG
Expand Down
40 changes: 24 additions & 16 deletions src/coreclr/jit/optimizer.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2684,8 +2684,7 @@ void Compiler::optFindNaturalLoops()
}
if (mod)
{
constexpr bool computePreds = true;
fgUpdateChangedFlowGraph(computePreds);
fgUpdateChangedFlowGraph(FlowGraphUpdates::COMPUTE_DOMS);
}

if (false /* pre-header stress */)
Expand All @@ -2698,10 +2697,7 @@ void Compiler::optFindNaturalLoops()

if (fgModified)
{
// The predecessors were maintained in fgCreateLoopPreHeader; don't rebuild them.
constexpr bool computePreds = false;
constexpr bool computeDoms = true;
fgUpdateChangedFlowGraph(computePreds, computeDoms);
fgUpdateChangedFlowGraph(FlowGraphUpdates::COMPUTE_DOMS);
}
}

Expand Down Expand Up @@ -3082,6 +3078,10 @@ bool Compiler::optCanonicalizeLoop(unsigned char loopInd)

BasicBlock* const newH = fgNewBBbefore(BBJ_NONE, t, /*extendRegion*/ true);

fgRemoveRefPred(t, h);
fgAddRefPred(t, newH);
fgAddRefPred(newH, h);

// Anything that flows into sibling will flow here.
// So we use sibling.H as our best guess for weight.
//
Expand Down Expand Up @@ -3259,6 +3259,10 @@ bool Compiler::optCanonicalizeLoopCore(unsigned char loopInd, LoopCanonicalizati
const bool extendRegion = BasicBlock::sameTryRegion(t, b);
BasicBlock* const newT = fgNewBBbefore(BBJ_NONE, t, extendRegion);

fgRemoveRefPred(t, h);
fgAddRefPred(t, newT);
fgAddRefPred(newT, h);

// Initially give newT the same weight as t; we will subtract from
// this for each edge that does not move from t to newT.
//
Expand Down Expand Up @@ -3308,7 +3312,7 @@ bool Compiler::optCanonicalizeLoopCore(unsigned char loopInd, LoopCanonicalizati
JITDUMP("in optCanonicalizeLoop (current): redirect bottom->top backedge " FMT_BB " -> " FMT_BB
" to " FMT_BB " -> " FMT_BB "\n",
topPredBlock->bbNum, t->bbNum, topPredBlock->bbNum, newT->bbNum);
optRedirectBlock(b, blockMap);
optRedirectBlock(b, blockMap, RedirectBlockOption::UpdatePredLists);
}
}
else if (option == LoopCanonicalizationOption::Outer)
Expand Down Expand Up @@ -3340,7 +3344,7 @@ bool Compiler::optCanonicalizeLoopCore(unsigned char loopInd, LoopCanonicalizati
" -> " FMT_BB "\n",
topPredBlock == h ? "head" : "nonloop", topPredBlock == h ? "" : "back", topPredBlock->bbNum,
t->bbNum, topPredBlock->bbNum, newT->bbNum);
optRedirectBlock(topPredBlock, blockMap);
optRedirectBlock(topPredBlock, blockMap, RedirectBlockOption::UpdatePredLists);
}
}
else
Expand Down Expand Up @@ -4464,7 +4468,7 @@ PhaseStatus Compiler::optUnrollLoops()
optRedirectBlock(newBlock, &blockMap, RedirectBlockOption::AddToPredLists);
}

// We fall into to this unroll iteration from the bottom block (first iteration)
// We fall into this unroll iteration from the bottom block (first iteration)
// or from the previous unroll clone of the bottom block (subsequent iterations).
// After doing this, all the newly cloned blocks now have proper flow and pred lists.
//
Expand Down Expand Up @@ -4511,7 +4515,7 @@ PhaseStatus Compiler::optUnrollLoops()

// Scrub all pred list references to block, except for bottom-> bottom->bbNext.
//
for (BasicBlock* succ : block->Succs())
for (BasicBlock* succ : block->Succs(this))
{
if ((block == bottom) && (succ == bottom->bbNext))
{
Expand Down Expand Up @@ -4639,12 +4643,16 @@ PhaseStatus Compiler::optUnrollLoops()

// If we unrolled any nested loops, we rebuild the loop table (including recomputing the
// return blocks list).

constexpr bool computePreds = false;
constexpr bool computeDoms = true;
const bool computeReturnBlocks = anyNestedLoopsUnrolled;
const bool computeLoops = anyNestedLoopsUnrolled;
fgUpdateChangedFlowGraph(computePreds, computeDoms, computeReturnBlocks, computeLoops);
//
if (anyNestedLoopsUnrolled)
{
fgUpdateChangedFlowGraph(FlowGraphUpdates::COMPUTE_DOMS | FlowGraphUpdates::COMPUTE_RETURNS |
FlowGraphUpdates::COMPUTE_LOOPS);
}
else
{
fgUpdateChangedFlowGraph(FlowGraphUpdates::COMPUTE_DOMS);
}

DBEXEC(verbose, fgDispBasicBlocks());
}
Expand Down