Skip to content

Commit 5177e08

Browse files
authored
JIT: GDV Cleanup Phase (#116992)
If a method has GDVs and the JIT is able to to improve types during inlining, run a pass after inlining that tries to resolve GDVs using those improved types. This typically happens when the object tested in a GDV is returned from some kind of factory method where the exact type becomes apparent only after inlining. Contributes to #25448.
1 parent 40e6d1c commit 5177e08

File tree

7 files changed

+83
-2
lines changed

7 files changed

+83
-2
lines changed

src/coreclr/jit/compiler.cpp

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4412,6 +4412,10 @@ void Compiler::compCompile(void** methodCodePtr, uint32_t* methodCodeSize, JitFl
44124412

44134413
if (opts.OptimizationEnabled())
44144414
{
4415+
// Try and resolve GDV checks if improved types were found during inlining
4416+
//
4417+
DoPhase(this, PHASE_RESOLVE_GDVS, &Compiler::fgResolveGDVs);
4418+
44154419
// Build post-order and remove dead blocks
44164420
//
44174421
DoPhase(this, PHASE_DFS_BLOCKS1, &Compiler::fgDfsBlocksAndRemove);

src/coreclr/jit/compiler.h

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4503,6 +4503,8 @@ class Compiler
45034503
return compiler->impEnumeratorGdvLocalMap;
45044504
}
45054505

4506+
bool hasUpdatedTypeLocals = false;
4507+
45064508
#define SMALL_STACK_SIZE 16 // number of elements in impSmallStack
45074509

45084510
struct SavedStack // used to save/restore stack contents.
@@ -5339,6 +5341,8 @@ class Compiler
53395341

53405342
PhaseStatus fgInline();
53415343

5344+
PhaseStatus fgResolveGDVs();
5345+
53425346
PhaseStatus fgRemoveEmptyTry();
53435347

53445348
PhaseStatus fgRemoveEmptyTryCatchOrTryFault();

src/coreclr/jit/compphases.h

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -31,6 +31,7 @@ CompPhaseNameMacro(PHASE_POST_IMPORT, "Post-import",
3131
CompPhaseNameMacro(PHASE_IBCPREP, "Profile instrumentation prep", false, -1, false)
3232
CompPhaseNameMacro(PHASE_IBCINSTR, "Profile instrumentation", false, -1, false)
3333
CompPhaseNameMacro(PHASE_INCPROFILE, "Profile incorporation", false, -1, false)
34+
CompPhaseNameMacro(PHASE_RESOLVE_GDVS, "Resolve GDV Checks", false, -1, false)
3435
CompPhaseNameMacro(PHASE_MORPH_INIT, "Morph - Init", false, -1, false)
3536
CompPhaseNameMacro(PHASE_MORPH_INLINE, "Morph - Inlining", false, -1, true)
3637
CompPhaseNameMacro(PHASE_MORPH_ADD_INTERNAL, "Morph - Add internal blocks", false, -1, true)

src/coreclr/jit/fginline.cpp

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -712,7 +712,8 @@ class SubstitutePlaceholdersAndDevirtualizeWalker : public GenTreeVisitor<Substi
712712
if (newClass != NO_CLASS_HANDLE)
713713
{
714714
m_compiler->lvaUpdateClass(lclNum, newClass, isExact);
715-
m_madeChanges = true;
715+
m_madeChanges = true;
716+
m_compiler->hasUpdatedTypeLocals = true;
716717
}
717718
}
718719
}

src/coreclr/jit/fgopt.cpp

Lines changed: 65 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -5794,3 +5794,68 @@ bool Compiler::fgCanMoveFirstStatementIntoPred(bool early, Statement* firstStmt,
57945794

57955795
return true;
57965796
}
5797+
5798+
//-------------------------------------------------------------
5799+
// fgResolveGDVs: Try and resolve GDV checks
5800+
//
5801+
// Returns:
5802+
// Suitable phase status.
5803+
//
5804+
PhaseStatus Compiler::fgResolveGDVs()
5805+
{
5806+
if (!opts.OptimizationEnabled())
5807+
{
5808+
return PhaseStatus::MODIFIED_NOTHING;
5809+
}
5810+
5811+
if (!doesMethodHaveGuardedDevirtualization())
5812+
{
5813+
return PhaseStatus::MODIFIED_NOTHING;
5814+
}
5815+
5816+
if (!hasUpdatedTypeLocals)
5817+
{
5818+
return PhaseStatus::MODIFIED_NOTHING;
5819+
}
5820+
5821+
bool madeChanges = false;
5822+
5823+
for (BasicBlock* const block : Blocks())
5824+
{
5825+
if (!block->KindIs(BBJ_COND))
5826+
{
5827+
continue;
5828+
}
5829+
5830+
GuardInfo info;
5831+
if (ObjectAllocator::IsGuard(block, &info))
5832+
{
5833+
assert(block == info.m_block);
5834+
LclVarDsc* const lclDsc = lvaGetDesc(info.m_local);
5835+
5836+
if (lclDsc->lvClassIsExact && lclDsc->lvSingleDef && (lclDsc->lvClassHnd == info.m_type))
5837+
{
5838+
JITDUMP("GDV in " FMT_BB " can be resolved; type is now known exactly\n", block->bbNum);
5839+
5840+
bool const isCondTrue = info.m_relop->OperIs(GT_EQ);
5841+
FlowEdge* const retainedEdge = isCondTrue ? block->GetTrueEdge() : block->GetFalseEdge();
5842+
FlowEdge* const removedEdge = isCondTrue ? block->GetFalseEdge() : block->GetTrueEdge();
5843+
5844+
JITDUMP("The conditional jump becomes an unconditional jump to " FMT_BB "\n",
5845+
retainedEdge->getDestinationBlock()->bbNum);
5846+
5847+
fgRemoveRefPred(removedEdge);
5848+
block->SetKindAndTargetEdge(BBJ_ALWAYS, retainedEdge);
5849+
fgRepairProfileCondToUncond(block, retainedEdge, removedEdge);
5850+
5851+
// The GDV relop will typically be side effecting so just
5852+
// leave it in place for later cleanup.
5853+
//
5854+
info.m_stmt->SetRootNode(info.m_relop);
5855+
madeChanges = true;
5856+
}
5857+
}
5858+
}
5859+
5860+
return madeChanges ? PhaseStatus::MODIFIED_EVERYTHING : PhaseStatus::MODIFIED_NOTHING;
5861+
}

src/coreclr/jit/objectalloc.cpp

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3204,6 +3204,9 @@ GenTree* ObjectAllocator::IsGuard(BasicBlock* block, GuardInfo* info)
32043204
bool isNonNull = false;
32053205
bool isExact = false;
32063206
info->m_type = (CORINFO_CLASS_HANDLE)op2->AsIntCon()->gtCompileTimeHandle;
3207+
info->m_block = block;
3208+
info->m_stmt = stmt;
3209+
info->m_relop = tree;
32073210

32083211
JITDUMP("... " FMT_BB " is guard for V%02u\n", block->bbNum, info->m_local);
32093212
return tree;

src/coreclr/jit/objectalloc.h

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -64,6 +64,8 @@ struct GuardInfo
6464
unsigned m_local = BAD_VAR_NUM;
6565
CORINFO_CLASS_HANDLE m_type = NO_CLASS_HANDLE;
6666
BasicBlock* m_block = nullptr;
67+
Statement* m_stmt = nullptr;
68+
GenTree* m_relop = nullptr;
6769
};
6870

6971
// Describes a guarded enumerator cloning candidate
@@ -204,6 +206,8 @@ class ObjectAllocator final : public Phase
204206
const char** reason,
205207
bool preliminaryCheck = false);
206208

209+
static GenTree* IsGuard(BasicBlock* block, GuardInfo* info);
210+
207211
protected:
208212
virtual PhaseStatus DoPhase() override;
209213

@@ -260,7 +264,6 @@ class ObjectAllocator final : public Phase
260264
bool CheckForGuardedUse(BasicBlock* block, GenTree* tree, unsigned lclNum);
261265
bool CheckForEnumeratorUse(unsigned lclNum, unsigned dstLclNum);
262266
bool IsGuarded(BasicBlock* block, GenTree* tree, GuardInfo* info, bool testOutcome);
263-
GenTree* IsGuard(BasicBlock* block, GuardInfo* info);
264267
unsigned NewPseudoIndex();
265268

266269
bool CanHavePseudos()

0 commit comments

Comments
 (0)