Skip to content

Commit

Permalink
Convert more jit phases to opt into common post phase checks (#74041)
Browse files Browse the repository at this point in the history 
* Rework the post importation phase
* Rework the morph init phase
* Rework fgAddInternal
* Rework flow graph update phases, compute preds, and update finally flags
* Rework struct promotion phase
* Rework local morph
* Rework morph implicit byrefs
* Rework GS cookie phase
* Rework compute block and edge weights
* Rework create funclets
* no funclets on x86, so no phase
* account for some jit stress changes
* review feedback
  • Loading branch information
@AndyAyersMS
AndyAyersMS authored Aug 19, 2022
1 parent e6b5e67 commit 9cd1a32
Show file tree
Hide file tree
Showing 13 changed files with 454 additions and 393 deletions.
177 changes: 12 additions & 165 deletions src/coreclr/jit/compiler.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -4361,37 +4361,10 @@ void Compiler::compCompile(void** methodCodePtr, uint32_t* methodCodeSize, JitFl
//
DoPhase(this, PHASE_INDXCALL, &Compiler::fgTransformIndirectCalls);

// PostImportPhase: cleanup inlinees
// Cleanup un-imported BBs, cleanup un-imported or
// partially imported try regions, add OSR step blocks.
//
auto postImportPhase = [this]() {

// If this is a viable inline candidate
if (compIsForInlining() && !compDonotInline())
{
// Filter out unimported BBs in the inlinee
//
fgPostImportationCleanup();

// Update type of return spill temp if we have gathered
// better info when importing the inlinee, and the return
// spill temp is single def.
if (fgNeedReturnSpillTemp())
{
CORINFO_CLASS_HANDLE retExprClassHnd = impInlineInfo->retExprClassHnd;
if (retExprClassHnd != nullptr)
{
LclVarDsc* returnSpillVarDsc = lvaGetDesc(lvaInlineeReturnSpillTemp);

if (returnSpillVarDsc->lvSingleDef)
{
lvaUpdateClass(lvaInlineeReturnSpillTemp, retExprClassHnd,
impInlineInfo->retExprClassHndIsExact);
}
}
}
}
};
DoPhase(this, PHASE_POST_IMPORT, postImportPhase);
DoPhase(this, PHASE_POST_IMPORT, &Compiler::fgPostImportationCleanup);

// If we're importing for inlining, we're done.
if (compIsForInlining())
Expand Down Expand Up @@ -4422,101 +4395,9 @@ void Compiler::compCompile(void** methodCodePtr, uint32_t* methodCodeSize, JitFl
return;
}

#if !FEATURE_EH
// If we aren't yet supporting EH in a compiler bring-up, remove as many EH handlers as possible, so
// we can pass tests that contain try/catch EH, but don't actually throw any exceptions.
fgRemoveEH();
#endif // !FEATURE_EH

// We could allow ESP frames. Just need to reserve space for
// pushing EBP if the method becomes an EBP-frame after an edit.
// Note that requiring a EBP Frame disallows double alignment. Thus if we change this
// we either have to disallow double alignment for E&C some other way or handle it in EETwain.

if (opts.compDbgEnC)
{
codeGen->setFramePointerRequired(true);

// We don't care about localloc right now. If we do support it,
// EECodeManager::FixContextForEnC() needs to handle it smartly
// in case the localloc was actually executed.
//
// compLocallocUsed = true;
}

// Start phases that are broadly called morphing, and includes
// global morph, as well as other phases that massage the trees so
// that we can generate code out of them.
// Prepare for the morph phases
//
auto morphInitPhase = [this]() {

// Initialize the BlockSet epoch
NewBasicBlockEpoch();

fgOutgoingArgTemps = nullptr;

// Insert call to class constructor as the first basic block if
// we were asked to do so.
if (info.compCompHnd->initClass(nullptr /* field */, nullptr /* method */,
impTokenLookupContextHandle /* context */) &
CORINFO_INITCLASS_USE_HELPER)
{
fgEnsureFirstBBisScratch();
fgNewStmtAtBeg(fgFirstBB, fgInitThisClass());
}

#ifdef DEBUG
if (opts.compGcChecks)
{
for (unsigned i = 0; i < info.compArgsCount; i++)
{
if (lvaGetDesc(i)->TypeGet() == TYP_REF)
{
// confirm that the argument is a GC pointer (for debugging (GC stress))
GenTree* op = gtNewLclvNode(i, TYP_REF);
op = gtNewHelperCallNode(CORINFO_HELP_CHECK_OBJ, TYP_VOID, op);

fgEnsureFirstBBisScratch();
fgNewStmtAtEnd(fgFirstBB, op);

if (verbose)
{
printf("\ncompGcChecks tree:\n");
gtDispTree(op);
}
}
}
}
#endif // DEBUG

#if defined(DEBUG) && defined(TARGET_XARCH)
if (opts.compStackCheckOnRet)
{
lvaReturnSpCheck = lvaGrabTempWithImplicitUse(false DEBUGARG("ReturnSpCheck"));
lvaSetVarDoNotEnregister(lvaReturnSpCheck, DoNotEnregisterReason::ReturnSpCheck);
lvaGetDesc(lvaReturnSpCheck)->lvType = TYP_I_IMPL;
}
#endif // defined(DEBUG) && defined(TARGET_XARCH)

#if defined(DEBUG) && defined(TARGET_X86)
if (opts.compStackCheckOnCall)
{
lvaCallSpCheck = lvaGrabTempWithImplicitUse(false DEBUGARG("CallSpCheck"));
lvaGetDesc(lvaCallSpCheck)->lvType = TYP_I_IMPL;
}
#endif // defined(DEBUG) && defined(TARGET_X86)

// Update flow graph after importation.
// Removes un-imported blocks, trims EH, and ensures correct OSR entry flow.
//
fgPostImportationCleanup();
};
DoPhase(this, PHASE_MORPH_INIT, morphInitPhase);

#ifdef DEBUG
// Inliner could add basic blocks. Check that the flowgraph data is up-to-date
fgDebugCheckBBlist(false, false);
#endif // DEBUG
DoPhase(this, PHASE_MORPH_INIT, &Compiler::fgMorphInit);

// Inline callee methods into this root method
//
Expand Down Expand Up @@ -4650,24 +4531,16 @@ void Compiler::compCompile(void** methodCodePtr, uint32_t* methodCodeSize, JitFl

// Run an early flow graph simplification pass
//
auto earlyUpdateFlowGraphPhase = [this]() {
constexpr bool doTailDup = false;
fgUpdateFlowGraph(doTailDup);
};
DoPhase(this, PHASE_EARLY_UPDATE_FLOW_GRAPH, earlyUpdateFlowGraphPhase);
DoPhase(this, PHASE_EARLY_UPDATE_FLOW_GRAPH, &Compiler::fgUpdateFlowGraphPhase);
}

// Promote struct locals
//
auto promoteStructsPhase = [this]() {
DoPhase(this, PHASE_PROMOTE_STRUCTS, &Compiler::fgPromoteStructs);

// For x64 and ARM64 we need to mark irregular parameters
lvaRefCountState = RCS_EARLY;
fgResetImplicitByRefRefCount();

fgPromoteStructs();
};
DoPhase(this, PHASE_PROMOTE_STRUCTS, promoteStructsPhase);
// Enable early ref counting of locals
//
lvaRefCountState = RCS_EARLY;

// Figure out what locals are address-taken.
//
Expand Down Expand Up @@ -4729,29 +4602,7 @@ void Compiler::compCompile(void** methodCodePtr, uint32_t* methodCodeSize, JitFl

// GS security checks for unsafe buffers
//
auto gsPhase = [this]() {
unsigned prevBBCount = fgBBcount;
if (getNeedsGSSecurityCookie())
{
gsGSChecksInitCookie();

if (compGSReorderStackLayout)
{
gsCopyShadowParams();
}

// If we needed to create any new BasicBlocks then renumber the blocks
if (fgBBcount > prevBBCount)
{
fgRenumberBlocks();
}
}
else
{
JITDUMP("No GS security needed\n");
}
};
DoPhase(this, PHASE_GS_COOKIE, gsPhase);
DoPhase(this, PHASE_GS_COOKIE, &Compiler::gsPhase);

// Compute the block and edge weights
//
Expand Down Expand Up @@ -4949,11 +4800,7 @@ void Compiler::compCompile(void** methodCodePtr, uint32_t* methodCodeSize, JitFl
{
// update the flowgraph if we modified it during the optimization phase
//
auto optUpdateFlowGraphPhase = [this]() {
constexpr bool doTailDup = false;
fgUpdateFlowGraph(doTailDup);
};
DoPhase(this, PHASE_OPT_UPDATE_FLOW_GRAPH, optUpdateFlowGraphPhase);
DoPhase(this, PHASE_OPT_UPDATE_FLOW_GRAPH, &Compiler::fgUpdateFlowGraphPhase);

// Recompute the edge weight if we have modified the flow graph
//
Expand Down
40 changes: 22 additions & 18 deletions src/coreclr/jit/compiler.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2220,7 +2220,7 @@ class Compiler
bool fgNormalizeEHCase2();
bool fgNormalizeEHCase3();

void fgCreateFiltersForGenericExceptions();
bool fgCreateFiltersForGenericExceptions();

void fgCheckForLoopsInHandlers();

Expand Down Expand Up @@ -4339,7 +4339,7 @@ class Compiler
}

BasicBlock* fgNewBasicBlock(BBjumpKinds jumpKind);
void fgEnsureFirstBBisScratch();
bool fgEnsureFirstBBisScratch();
bool fgFirstBBisScratch();
bool fgBBisScratch(BasicBlock* block);

Expand Down Expand Up @@ -4461,6 +4461,8 @@ class Compiler

PhaseStatus fgTransformPatchpoints();

PhaseStatus fgMorphInit();

PhaseStatus fgInline();

PhaseStatus fgRemoveEmptyTry();
Expand Down Expand Up @@ -4522,7 +4524,7 @@ class Compiler
// The number of separate return points in the method.
unsigned fgReturnCount;

void fgAddInternal();
PhaseStatus fgAddInternal();

enum class FoldResult
{
Expand Down Expand Up @@ -5222,7 +5224,7 @@ class Compiler

unsigned fgGetNestingLevel(BasicBlock* block, unsigned* pFinallyNesting = nullptr);

void fgPostImportationCleanup();
PhaseStatus fgPostImportationCleanup();

void fgRemoveStmt(BasicBlock* block, Statement* stmt DEBUGARG(bool isUnlink = false));
void fgUnlinkStmt(BasicBlock* block, Statement* stmt);
Expand Down Expand Up @@ -5275,8 +5277,8 @@ class Compiler
bool fgIsIntraHandlerPred(BasicBlock* predBlock, BasicBlock* block);
bool fgAnyIntraHandlerPreds(BasicBlock* block);
void fgInsertFuncletPrologBlock(BasicBlock* block);
void fgCreateFuncletPrologBlocks();
void fgCreateFunclets();
void fgCreateFuncletPrologBlocks();
PhaseStatus fgCreateFunclets();
#else // !FEATURE_EH_FUNCLETS
bool fgRelocateEHRegions();
#endif // !FEATURE_EH_FUNCLETS
Expand All @@ -5301,10 +5303,10 @@ class Compiler
#ifdef DEBUG
void fgPrintEdgeWeights();
#endif
void fgComputeBlockAndEdgeWeights();
weight_t fgComputeMissingBlockWeights();
void fgComputeCalledCount(weight_t returnWeight);
void fgComputeEdgeWeights();
PhaseStatus fgComputeBlockAndEdgeWeights();
bool fgComputeMissingBlockWeights(weight_t* returnWeight);
bool fgComputeCalledCount(weight_t returnWeight);
PhaseStatus fgComputeEdgeWeights();

bool fgReorderBlocks(bool useProfile);

Expand All @@ -5316,7 +5318,8 @@ class Compiler

bool fgIsForwardBranch(BasicBlock* bJump, BasicBlock* bSrc = nullptr);

bool fgUpdateFlowGraph(bool doTailDup = false);
bool fgUpdateFlowGraph(bool doTailDup = false, bool isPhase = false);
PhaseStatus fgUpdateFlowGraphPhase();

PhaseStatus fgFindOperOrder();

Expand Down Expand Up @@ -5896,7 +5899,7 @@ class Compiler
static fgWalkPreFn fgDebugCheckForTransformableIndirectCalls;
#endif

void fgPromoteStructs();
PhaseStatus fgPromoteStructs();
void fgMorphStructField(GenTree* tree, GenTree* parent);
void fgMorphLocalField(GenTree* tree, GenTree* parent);

Expand All @@ -5905,12 +5908,12 @@ class Compiler

// Change implicit byrefs' types from struct to pointer, and for any that were
// promoted, create new promoted struct temps.
void fgRetypeImplicitByRefArgs();
PhaseStatus fgRetypeImplicitByRefArgs();

// Clear up annotations for any struct promotion temps created for implicit byrefs.
void fgMarkDemotedImplicitByRefArgs();

void fgMarkAddressExposedLocals();
PhaseStatus fgMarkAddressExposedLocals();
void fgMarkAddressExposedLocals(Statement* stmt);

PhaseStatus fgForwardSub();
Expand Down Expand Up @@ -10402,10 +10405,11 @@ XXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXX
GSCookie gsGlobalSecurityCookieVal; // Value of global cookie if addr is NULL
ShadowParamVarInfo* gsShadowVarInfo; // Table used by shadow param analysis code

void gsGSChecksInitCookie(); // Grabs cookie variable
void gsCopyShadowParams(); // Identify vulnerable params and create dhadow copies
bool gsFindVulnerableParams(); // Shadow param analysis code
void gsParamsToShadows(); // Insert copy code and replave param uses by shadow
PhaseStatus gsPhase();
void gsGSChecksInitCookie(); // Grabs cookie variable
void gsCopyShadowParams(); // Identify vulnerable params and create dhadow copies
bool gsFindVulnerableParams(); // Shadow param analysis code
void gsParamsToShadows(); // Insert copy code and replave param uses by shadow

static fgWalkPreFn gsMarkPtrsAndAssignGroups; // Shadow param analysis tree-walk
static fgWalkPreFn gsReplaceShadowParams; // Shadow param replacement tree-walk
Expand Down
8 changes: 5 additions & 3 deletions src/coreclr/jit/fgbasic.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -231,7 +231,7 @@ BasicBlock* Compiler::fgNewBasicBlock(BBjumpKinds jumpKind)
// fgEnsureFirstBBisScratch: Ensure that fgFirstBB is a scratch BasicBlock
//
// Returns:
// Nothing. May allocate a new block and alter the value of fgFirstBB.
// True, if a new basic block was allocated.
//
// Notes:
// This should be called before adding on-entry initialization code to
Expand All @@ -249,12 +249,12 @@ BasicBlock* Compiler::fgNewBasicBlock(BBjumpKinds jumpKind)
//
// Can be called at any time, and can be called multiple times.
//
void Compiler::fgEnsureFirstBBisScratch()
bool Compiler::fgEnsureFirstBBisScratch()
{
// Have we already allocated a scratch block?
if (fgFirstBBisScratch())
{
return;
return false;
}

assert(fgFirstBBScratch == nullptr);
Expand Down Expand Up @@ -303,6 +303,8 @@ void Compiler::fgEnsureFirstBBisScratch()
printf("New scratch " FMT_BB "\n", block->bbNum);
}
#endif

return true;
}

//------------------------------------------------------------------------
Expand Down
Loading

0 comments on commit 9cd1a32

Please sign in to comment.