Skip to content

Commit

Permalink
Don't re-scan previously visited blocks in helperexpansion.cpp (#85201)
Browse files Browse the repository at this point in the history
  • Loading branch information
@EgorBo
EgorBo committed Apr 26, 2023
1 parent 9a7db55 commit 63a7a13
Show file tree
Hide file tree
Showing 2 changed files with 55 additions and 22 deletions.
12 changes: 6 additions & 6 deletions src/coreclr/jit/compiler.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -5305,20 +5305,20 @@ class Compiler
void SplitTreesRandomly();
void SplitTreesRemoveCommas();

template <bool (Compiler::*ExpansionFunction)(BasicBlock*, Statement*, GenTreeCall*)>
template <bool (Compiler::*ExpansionFunction)(BasicBlock**, Statement*, GenTreeCall*)>
PhaseStatus fgExpandHelper(bool skipRarelyRunBlocks);

template <bool (Compiler::*ExpansionFunction)(BasicBlock*, Statement*, GenTreeCall*)>
bool fgExpandHelperForBlock(BasicBlock* block);
template <bool (Compiler::*ExpansionFunction)(BasicBlock**, Statement*, GenTreeCall*)>
bool fgExpandHelperForBlock(BasicBlock** pBlock);

PhaseStatus fgExpandRuntimeLookups();
bool fgExpandRuntimeLookupsForCall(BasicBlock* block, Statement* stmt, GenTreeCall* call);
bool fgExpandRuntimeLookupsForCall(BasicBlock** pBlock, Statement* stmt, GenTreeCall* call);

PhaseStatus fgExpandThreadLocalAccess();
bool fgExpandThreadLocalAccessForCall(BasicBlock* block, Statement* stmt, GenTreeCall* call);
bool fgExpandThreadLocalAccessForCall(BasicBlock** pBlock, Statement* stmt, GenTreeCall* call);

PhaseStatus fgExpandStaticInit();
bool fgExpandStaticInitForCall(BasicBlock* block, Statement* stmt, GenTreeCall* call);
bool fgExpandStaticInitForCall(BasicBlock** pBlock, Statement* stmt, GenTreeCall* call);

PhaseStatus fgInsertGCPolls();
BasicBlock* fgCreateGCPoll(GCPollType pollType, BasicBlock* block);
Expand Down
65 changes: 49 additions & 16 deletions src/coreclr/jit/helperexpansion.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -93,8 +93,28 @@ PhaseStatus Compiler::fgExpandRuntimeLookups()
return fgExpandHelper<&Compiler::fgExpandRuntimeLookupsForCall>(false);
}

bool Compiler::fgExpandRuntimeLookupsForCall(BasicBlock* block, Statement* stmt, GenTreeCall* call)
//------------------------------------------------------------------------------
// fgExpandRuntimeLookupsForCall : partially expand runtime lookups helper calls
// to add a nullcheck [+ size check] and a fast path
//
// Arguments:
// pBlock - Block containing the helper call to expand. If expansion is performed,
// this is updated to the new block that was an outcome of block splitting.
// stmt - Statement containing the helper call
// call - The helper call
//
// Returns:
// true if a runtime lookup was found and expanded.
//
// Notes:
// The runtime lookup itself is needed to access a handle in code shared between
// generic instantiations. The lookup depends on the typeContext which is only available at
// runtime, and not at compile - time. See ASCII block diagrams in comments below for
// better understanding how this phase expands runtime lookups.
//
bool Compiler::fgExpandRuntimeLookupsForCall(BasicBlock** pBlock, Statement* stmt, GenTreeCall* call)
{
BasicBlock* block = *pBlock;
assert(call->IsHelperCall());

if (!call->IsExpRuntimeLookup())
Expand Down Expand Up @@ -150,6 +170,7 @@ bool Compiler::fgExpandRuntimeLookupsForCall(BasicBlock* block, Statement* stmt,
GenTree** callUse = nullptr;
Statement* newFirstStmt = nullptr;
block = fgSplitBlockBeforeTree(block, stmt, call, &newFirstStmt, &callUse);
*pBlock = block;
assert(prevBb != nullptr && block != nullptr);

// Block ops inserted by the split need to be morphed here since we are after morph.
Expand Down Expand Up @@ -433,9 +454,10 @@ PhaseStatus Compiler::fgExpandThreadLocalAccess()
// that access fields marked with [ThreadLocal].
//
// Arguments:
// block - Block containing the helper call to expand
// stmt - Statement containing the helper call
// call - The helper call
// pBlock - Block containing the helper call to expand. If expansion is performed,
// this is updated to the new block that was an outcome of block splitting.
// stmt - Statement containing the helper call
// call - The helper call
//
//
// Returns:
Expand All @@ -450,8 +472,9 @@ PhaseStatus Compiler::fgExpandThreadLocalAccess()
// If the entry is not present, the helper is called, which would make an entry of current static block
// in the cache.
//
bool Compiler::fgExpandThreadLocalAccessForCall(BasicBlock* block, Statement* stmt, GenTreeCall* call)
bool Compiler::fgExpandThreadLocalAccessForCall(BasicBlock** pBlock, Statement* stmt, GenTreeCall* call)
{
BasicBlock* block = *pBlock;
assert(call->IsHelperCall());
if (!call->IsExpTLSFieldAccess())
{
Expand Down Expand Up @@ -489,6 +512,7 @@ bool Compiler::fgExpandThreadLocalAccessForCall(BasicBlock* block, Statement* st
Statement* newFirstStmt = nullptr;
DebugInfo debugInfo = stmt->GetDebugInfo();
block = fgSplitBlockBeforeTree(block, stmt, call, &newFirstStmt, &callUse);
*pBlock = block;
assert(prevBb != nullptr && block != nullptr);

// Block ops inserted by the split need to be morphed here since we are after morph.
Expand Down Expand Up @@ -682,7 +706,7 @@ bool Compiler::fgExpandThreadLocalAccessForCall(BasicBlock* block, Statement* st
// Returns:
// true if there was any helper that was expanded.
//
template <bool (Compiler::*ExpansionFunction)(BasicBlock*, Statement*, GenTreeCall*)>
template <bool (Compiler::*ExpansionFunction)(BasicBlock**, Statement*, GenTreeCall*)>
PhaseStatus Compiler::fgExpandHelper(bool skipRarelyRunBlocks)
{
PhaseStatus result = PhaseStatus::MODIFIED_NOTHING;
Expand All @@ -695,9 +719,14 @@ PhaseStatus Compiler::fgExpandHelper(bool skipRarelyRunBlocks)
}

// Expand and visit the last block again to find more candidates
while (fgExpandHelperForBlock<ExpansionFunction>(block))
INDEBUG(BasicBlock* origBlock = block);
while (fgExpandHelperForBlock<ExpansionFunction>(&block))
{
result = PhaseStatus::MODIFIED_EVERYTHING;
#ifdef DEBUG
assert(origBlock != block);
origBlock = block;
#endif
}
}

Expand All @@ -715,16 +744,17 @@ PhaseStatus Compiler::fgExpandHelper(bool skipRarelyRunBlocks)
// invoke `fgExpand` if any of the tree node was a helper call.
//
// Arguments:
// block - block to scan for static initializations
// pBlock - Block containing the helper call to expand. If expansion is performed,
// this is updated to the new block that was an outcome of block splitting.
// fgExpand - function that expands the helper call
//
// Returns:
// true if a helper was expanded
//
template <bool (Compiler::*ExpansionFunction)(BasicBlock*, Statement*, GenTreeCall*)>
bool Compiler::fgExpandHelperForBlock(BasicBlock* block)
template <bool (Compiler::*ExpansionFunction)(BasicBlock**, Statement*, GenTreeCall*)>
bool Compiler::fgExpandHelperForBlock(BasicBlock** pBlock)
{
for (Statement* const stmt : block->NonPhiStatements())
for (Statement* const stmt : (*pBlock)->NonPhiStatements())
{
if ((stmt->GetRootNode()->gtFlags & GTF_CALL) == 0)
{
Expand All @@ -739,7 +769,7 @@ bool Compiler::fgExpandHelperForBlock(BasicBlock* block)
continue;
}

if ((this->*ExpansionFunction)(block, stmt, tree->AsCall()))
if ((this->*ExpansionFunction)(pBlock, stmt, tree->AsCall()))
{
return true;
}
Expand Down Expand Up @@ -796,15 +826,17 @@ PhaseStatus Compiler::fgExpandStaticInit()
// Also, see fgExpandStaticInit's comments.
//
// Arguments:
// block - call's block
// stmt - call's statement
// call - call that represents a static initialization
// pBlock - Block containing the helper call to expand. If expansion is performed,
// this is updated to the new block that was an outcome of block splitting.
// stmt - Statement containing the helper call
// call - The helper call
//
// Returns:
// true if a static initialization was expanded
//
bool Compiler::fgExpandStaticInitForCall(BasicBlock* block, Statement* stmt, GenTreeCall* call)
bool Compiler::fgExpandStaticInitForCall(BasicBlock** pBlock, Statement* stmt, GenTreeCall* call)
{
BasicBlock* block = *pBlock;
assert(call->IsHelperCall());

bool isGc = false;
Expand Down Expand Up @@ -848,6 +880,7 @@ bool Compiler::fgExpandStaticInitForCall(BasicBlock* block, Statement* stmt, Gen
GenTree** callUse = nullptr;
Statement* newFirstStmt = nullptr;
block = fgSplitBlockBeforeTree(block, stmt, call, &newFirstStmt, &callUse);
*pBlock = block;
assert(prevBb != nullptr && block != nullptr);

// Block ops inserted by the split need to be morphed here since we are after morph.
Expand Down

0 comments on commit 63a7a13

Please sign in to comment.