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

Don't re-scan previously visited blocks in helperexpansion.cpp #85201

Merged
merged 4 commits into from
Apr 26, 2023
Merged

Don't re-scan previously visited blocks in helperexpansion.cpp #85201

Show file tree
Hide file tree
Changes from 3 commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
35d7df8
Don't re-scan previously visited blocks in helperexpansion.cpp
EgorBo Apr 22, 2023
ca12923
Merge branch 'main' of github.com:dotnet/runtime into minor-tp-improv…
EgorBo Apr 25, 2023
19c4822
Address feedback
EgorBo Apr 25, 2023
46e8ba1
Update helperexpansion.cpp
EgorBo Apr 26, 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
12 changes: 6 additions & 6 deletions src/coreclr/jit/compiler.h
View file
Open in desktop
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
Open in desktop
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)
EgorBo marked this conversation as resolved.
Show resolved Hide resolved
{
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))
EgorBo marked this conversation as resolved.
Show resolved Hide resolved
{
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