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.

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

Convert more jit phases to opt into common post phase checks #74041

Merged
merged 13 commits into from
Aug 19, 2022
177 changes: 12 additions & 165 deletions src/coreclr/jit/compiler.cpp
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
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 @@ -10372,10 +10375,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
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