From 63a7a13190448f1ba5b3b449f1b9628d042d209d Mon Sep 17 00:00:00 2001 From: Egor Bogatov Date: Wed, 26 Apr 2023 07:34:01 +0200 Subject: [PATCH] Don't re-scan previously visited blocks in helperexpansion.cpp (#85201) --- src/coreclr/jit/compiler.h | 12 +++---- src/coreclr/jit/helperexpansion.cpp | 65 ++++++++++++++++++++++++++++--------- 2 files changed, 55 insertions(+), 22 deletions(-) diff --git a/src/coreclr/jit/compiler.h b/src/coreclr/jit/compiler.h index 718967d..d05f9d1 100644 --- a/src/coreclr/jit/compiler.h +++ b/src/coreclr/jit/compiler.h @@ -5305,20 +5305,20 @@ public: void SplitTreesRandomly(); void SplitTreesRemoveCommas(); - template + template PhaseStatus fgExpandHelper(bool skipRarelyRunBlocks); - template - bool fgExpandHelperForBlock(BasicBlock* block); + template + 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); diff --git a/src/coreclr/jit/helperexpansion.cpp b/src/coreclr/jit/helperexpansion.cpp index 2ce9482..e614e78 100644 --- a/src/coreclr/jit/helperexpansion.cpp +++ b/src/coreclr/jit/helperexpansion.cpp @@ -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()) @@ -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. @@ -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: @@ -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()) { @@ -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. @@ -682,7 +706,7 @@ bool Compiler::fgExpandThreadLocalAccessForCall(BasicBlock* block, Statement* st // Returns: // true if there was any helper that was expanded. // -template +template PhaseStatus Compiler::fgExpandHelper(bool skipRarelyRunBlocks) { PhaseStatus result = PhaseStatus::MODIFIED_NOTHING; @@ -695,9 +719,14 @@ PhaseStatus Compiler::fgExpandHelper(bool skipRarelyRunBlocks) } // Expand and visit the last block again to find more candidates - while (fgExpandHelperForBlock(block)) + INDEBUG(BasicBlock* origBlock = block); + while (fgExpandHelperForBlock(&block)) { result = PhaseStatus::MODIFIED_EVERYTHING; +#ifdef DEBUG + assert(origBlock != block); + origBlock = block; +#endif } } @@ -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::fgExpandHelperForBlock(BasicBlock* block) +template +bool Compiler::fgExpandHelperForBlock(BasicBlock** pBlock) { - for (Statement* const stmt : block->NonPhiStatements()) + for (Statement* const stmt : (*pBlock)->NonPhiStatements()) { if ((stmt->GetRootNode()->gtFlags & GTF_CALL) == 0) { @@ -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; } @@ -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; @@ -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. -- 2.7.4