From 6d23ef4d68bbcdb38fdc22218d1073c5083ac6a1 Mon Sep 17 00:00:00 2001 From: Andy Ayers Date: Wed, 6 Nov 2024 10:00:20 -0800 Subject: [PATCH] JIT: some deabstraction prerequisites (#109575) Mark `IEnumerable.GetEnumerator` as intrinsic and fix jit side issues that ensued. Run object allocation in RPO. Since we're computing a DFS, remove any unreachable blocks. --- src/coreclr/jit/compiler.cpp | 11 ++++++-- src/coreclr/jit/compphases.h | 3 ++- src/coreclr/jit/importercalls.cpp | 25 +++++++++++-------- src/coreclr/jit/objectalloc.cpp | 23 ++++++++++++----- .../System/Collections/Generic/IEnumerable.cs | 2 ++ 5 files changed, 45 insertions(+), 19 deletions(-) diff --git a/src/coreclr/jit/compiler.cpp b/src/coreclr/jit/compiler.cpp index 3d2f568301bfa6..b1bf0a3cfc7273 100644 --- a/src/coreclr/jit/compiler.cpp +++ b/src/coreclr/jit/compiler.cpp @@ -4622,6 +4622,13 @@ void Compiler::compCompile(void** methodCodePtr, uint32_t* methodCodeSize, JitFl activePhaseChecks &= ~PhaseChecks::CHECK_PROFILE; activePhaseChecks |= PhaseChecks::CHECK_LIKELIHOODS; + if (opts.OptimizationEnabled()) + { + // Build post-order and remove dead blocks + // + DoPhase(this, PHASE_DFS_BLOCKS1, &Compiler::fgDfsBlocksAndRemove); + } + // Transform each GT_ALLOCOBJ node into either an allocation helper call or // local variable allocation on the stack. ObjectAllocator objectAllocator(this); // PHASE_ALLOCATE_OBJECTS @@ -4694,7 +4701,7 @@ void Compiler::compCompile(void** methodCodePtr, uint32_t* methodCodeSize, JitFl // Build post-order and remove dead blocks // - DoPhase(this, PHASE_DFS_BLOCKS, &Compiler::fgDfsBlocksAndRemove); + DoPhase(this, PHASE_DFS_BLOCKS2, &Compiler::fgDfsBlocksAndRemove); fgNodeThreading = NodeThreading::AllLocals; } @@ -4795,7 +4802,7 @@ void Compiler::compCompile(void** methodCodePtr, uint32_t* methodCodeSize, JitFl // Compute DFS tree and remove all unreachable blocks. // - DoPhase(this, PHASE_DFS_BLOCKS2, &Compiler::fgDfsBlocksAndRemove); + DoPhase(this, PHASE_DFS_BLOCKS3, &Compiler::fgDfsBlocksAndRemove); // Discover and classify natural loops (e.g. mark iterative loops as such). Also marks loop blocks // and sets bbWeight to the loop nesting levels. diff --git a/src/coreclr/jit/compphases.h b/src/coreclr/jit/compphases.h index 1e9b09a69abcd4..fb76d24c5bf0b4 100644 --- a/src/coreclr/jit/compphases.h +++ b/src/coreclr/jit/compphases.h @@ -42,8 +42,9 @@ CompPhaseNameMacro(PHASE_MERGE_FINALLY_CHAINS, "Merge callfinally chains", CompPhaseNameMacro(PHASE_CLONE_FINALLY, "Clone finally", false, -1, false) CompPhaseNameMacro(PHASE_UPDATE_FINALLY_FLAGS, "Update finally target flags", false, -1, false) CompPhaseNameMacro(PHASE_EARLY_UPDATE_FLOW_GRAPH, "Update flow graph early pass", false, -1, false) -CompPhaseNameMacro(PHASE_DFS_BLOCKS, "DFS blocks and remove dead code",false, -1, false) +CompPhaseNameMacro(PHASE_DFS_BLOCKS1, "DFS blocks and remove dead code 1",false, -1, false) CompPhaseNameMacro(PHASE_DFS_BLOCKS2, "DFS blocks and remove dead code 2",false, -1, false) +CompPhaseNameMacro(PHASE_DFS_BLOCKS3, "DFS blocks and remove dead code 3",false, -1, false) CompPhaseNameMacro(PHASE_STR_ADRLCL, "Morph - Structs/AddrExp", false, -1, false) CompPhaseNameMacro(PHASE_EARLY_LIVENESS, "Early liveness", false, -1, false) CompPhaseNameMacro(PHASE_PHYSICAL_PROMOTION, "Physical promotion", false, -1, false) diff --git a/src/coreclr/jit/importercalls.cpp b/src/coreclr/jit/importercalls.cpp index e25d63aa22074e..4251b7c0c6cf19 100644 --- a/src/coreclr/jit/importercalls.cpp +++ b/src/coreclr/jit/importercalls.cpp @@ -969,15 +969,20 @@ var_types Compiler::impImportCall(OPCODE opcode, // inlinees. rawILOffset); - // Devirtualization may change which method gets invoked. Update our local cache. - // - methHnd = callInfo->hMethod; + const bool wasDevirtualized = !call->AsCall()->IsVirtual(); - // If we devirtualized to an intrinsic, assume this is one of the special cases. - // - if ((callInfo->methodFlags & CORINFO_FLG_INTRINSIC) != 0) + if (wasDevirtualized) { - call->AsCall()->gtCallMoreFlags |= GTF_CALL_M_SPECIAL_INTRINSIC; + // Devirtualization may change which method gets invoked. Update our local cache. + // + methHnd = callInfo->hMethod; + + // If we devirtualized to an intrinsic, assume this is one of the special cases. + // + if ((callInfo->methodFlags & CORINFO_FLG_INTRINSIC) != 0) + { + call->AsCall()->gtCallMoreFlags |= GTF_CALL_M_SPECIAL_INTRINSIC; + } } } else if (call->AsCall()->IsDelegateInvoke()) @@ -1152,8 +1157,8 @@ var_types Compiler::impImportCall(OPCODE opcode, assert(isImplicitTailCall); // It is possible that a call node is both an inline candidate and marked - // for opportunistic tail calling. In-lining happens before morhphing of - // trees. If in-lining of an in-line candidate gets aborted for whatever + // for opportunistic tail calling. Inlining happens before morphing of + // trees. If inlining of an inline candidate gets aborted for whatever // reason, it will survive to the morphing stage at which point it will be // transformed into a tail call after performing additional checks. @@ -1364,7 +1369,7 @@ var_types Compiler::impImportCall(OPCODE opcode, assert(call->IsCall()); GenTreeCall* const origCall = call->AsCall(); - // If the call is a special intrisic, we may know a more exact return type. + // If the call is a special intrinsic, we may know a more exact return type. // if (origCall->IsSpecialIntrinsic()) { diff --git a/src/coreclr/jit/objectalloc.cpp b/src/coreclr/jit/objectalloc.cpp index 66837d6d280727..ed68aa93231972 100644 --- a/src/coreclr/jit/objectalloc.cpp +++ b/src/coreclr/jit/objectalloc.cpp @@ -33,6 +33,7 @@ PhaseStatus ObjectAllocator::DoPhase() if ((comp->optMethodFlags & OMF_HAS_NEWOBJ) == 0) { JITDUMP("no newobjs in this method; punting\n"); + comp->fgInvalidateDfsTree(); return PhaseStatus::MODIFIED_NOTHING; } @@ -67,14 +68,15 @@ PhaseStatus ObjectAllocator::DoPhase() if (didStackAllocate) { + assert(enabled); ComputeStackObjectPointers(&m_bitVecTraits); RewriteUses(); - return PhaseStatus::MODIFIED_EVERYTHING; - } - else - { - return PhaseStatus::MODIFIED_NOTHING; } + + // This phase always changes the IR. It may also modify the flow graph. + // + comp->fgInvalidateDfsTree(); + return PhaseStatus::MODIFIED_EVERYTHING; } //------------------------------------------------------------------------------ @@ -247,8 +249,17 @@ void ObjectAllocator::MarkEscapingVarsAndBuildConnGraph() } } - for (BasicBlock* const block : comp->Blocks()) + // We should have computed the DFS tree already. + // + FlowGraphDfsTree* const dfs = comp->m_dfsTree; + assert(dfs != nullptr); + + // Walk in RPO + // + for (unsigned i = dfs->GetPostOrderCount(); i != 0; i--) { + BasicBlock* const block = dfs->GetPostOrder(i - 1); + for (Statement* const stmt : block->Statements()) { BuildConnGraphVisitor buildConnGraphVisitor(this); diff --git a/src/libraries/System.Private.CoreLib/src/System/Collections/Generic/IEnumerable.cs b/src/libraries/System.Private.CoreLib/src/System/Collections/Generic/IEnumerable.cs index 1cdfaeb34a0f85..6ca3a5d3252a63 100644 --- a/src/libraries/System.Private.CoreLib/src/System/Collections/Generic/IEnumerable.cs +++ b/src/libraries/System.Private.CoreLib/src/System/Collections/Generic/IEnumerable.cs @@ -4,6 +4,7 @@ #if MONO using System.Diagnostics.CodeAnalysis; #endif +using System.Runtime.CompilerServices; namespace System.Collections.Generic { @@ -16,6 +17,7 @@ public interface IEnumerable : IEnumerable #if MONO [DynamicDependency(nameof(Array.InternalArray__IEnumerable_GetEnumerator) + "``1 ", typeof(Array))] #endif + [Intrinsic] new IEnumerator GetEnumerator(); } }