Skip to content

Commit

Permalink
Move ObjectAllocator phase to run right after inlining. (dotnet#20377)
Browse files Browse the repository at this point in the history
This change will support object stack allocation for the following reasons:

1. Objects should be allocated on the stack before struct promotion phase
so that their fields have a chance to be promoted.
2. Eventually object stack allocation will be performed in the same phase
as inlining since inlining heuristics will need to be aware of object stack allocation
opportunities.

I verified no x64 diffs with jit-diffs --frameworks --tests --pmi
  • Loading branch information
erozenfeld authored and A-And committed Nov 20, 2018
1 parent bf44e6d commit a3097e4
Show file tree
Hide file tree
Showing 7 changed files with 27 additions and 14 deletions.
5 changes: 0 additions & 5 deletions src/jit/compiler.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -4665,11 +4665,6 @@ void Compiler::compCompile(void** methodCodePtr, ULONG* methodCodeSize, JitFlags
EndPhase(PHASE_COMPUTE_REACHABILITY);
}

// Transform each GT_ALLOCOBJ node into either an allocation helper call or
// local variable allocation on the stack.
ObjectAllocator objectAllocator(this);
objectAllocator.Run();

if (!opts.MinOpts() && !opts.compDbgCode)
{
/* Perform loop inversion (i.e. transform "while" loops into
Expand Down
2 changes: 1 addition & 1 deletion src/jit/compiler.h
Original file line number Diff line number Diff line change
Expand Up @@ -5012,7 +5012,7 @@ class Compiler

GenTree* fgMorphCastIntoHelper(GenTree* tree, int helper, GenTree* oper);

GenTree* fgMorphIntoHelperCall(GenTree* tree, int helper, GenTreeArgList* args);
GenTree* fgMorphIntoHelperCall(GenTree* tree, int helper, GenTreeArgList* args, bool morphArgs = true);

GenTree* fgMorphStackArgForVarArgs(unsigned lclNum, var_types varType, unsigned lclOffs);

Expand Down
2 changes: 1 addition & 1 deletion src/jit/compphases.h
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,7 @@ CompPhaseNameMacro(PHASE_IMPORTATION, "Importation",
CompPhaseNameMacro(PHASE_POST_IMPORT, "Post-import", "POST-IMP", false, -1, false)
CompPhaseNameMacro(PHASE_MORPH_INIT, "Morph - Init", "MOR-INIT" ,false, -1, false)
CompPhaseNameMacro(PHASE_MORPH_INLINE, "Morph - Inlining", "MOR-INL", false, -1, true)
CompPhaseNameMacro(PHASE_ALLOCATE_OBJECTS, "Allocate Objects", "ALLOC-OBJ", false, -1, false)
CompPhaseNameMacro(PHASE_EMPTY_TRY, "Remove empty try", "EMPTYTRY", false, -1, false)
CompPhaseNameMacro(PHASE_EMPTY_FINALLY, "Remove empty finally", "EMPTYFIN", false, -1, false)
CompPhaseNameMacro(PHASE_MERGE_FINALLY_CHAINS, "Merge callfinally chains", "MRGCFCHN", false, -1, false)
Expand All @@ -46,7 +47,6 @@ CompPhaseNameMacro(PHASE_CREATE_FUNCLETS, "Create EH funclets",
#endif // FEATURE_EH_FUNCLETS
CompPhaseNameMacro(PHASE_OPTIMIZE_LAYOUT, "Optimize layout", "LAYOUT", false, -1, false)
CompPhaseNameMacro(PHASE_COMPUTE_REACHABILITY, "Compute blocks reachability", "BL_REACH", false, -1, false)
CompPhaseNameMacro(PHASE_ALLOCATE_OBJECTS, "Allocate Objects", "ALLOC-OBJ",false, -1, false)
CompPhaseNameMacro(PHASE_OPTIMIZE_LOOPS, "Optimize loops", "LOOP-OPT", false, -1, false)
CompPhaseNameMacro(PHASE_CLONE_LOOPS, "Clone loops", "LP-CLONE", false, -1, false)
CompPhaseNameMacro(PHASE_UNROLL_LOOPS, "Unroll loops", "UNROLL", false, -1, false)
Expand Down
12 changes: 10 additions & 2 deletions src/jit/morph.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -58,7 +58,7 @@ GenTree* Compiler::fgMorphCastIntoHelper(GenTree* tree, int helper, GenTree* ope
* the given argument list.
*/

GenTree* Compiler::fgMorphIntoHelperCall(GenTree* tree, int helper, GenTreeArgList* args)
GenTree* Compiler::fgMorphIntoHelperCall(GenTree* tree, int helper, GenTreeArgList* args, bool morphArgs)
{
// The helper call ought to be semantically equivalent to the original node, so preserve its VN.
tree->ChangeOper(GT_CALL, GenTree::PRESERVE_VN);
Expand Down Expand Up @@ -112,7 +112,10 @@ GenTree* Compiler::fgMorphIntoHelperCall(GenTree* tree, int helper, GenTreeArgLi

/* Perform the morphing */

tree = fgMorphArgs(tree->AsCall());
if (morphArgs)
{
tree = fgMorphArgs(tree->AsCall());
}

return tree;
}
Expand Down Expand Up @@ -16881,6 +16884,11 @@ void Compiler::fgMorph()

EndPhase(PHASE_MORPH_INLINE);

// Transform each GT_ALLOCOBJ node into either an allocation helper call or
// local variable allocation on the stack.
ObjectAllocator objectAllocator(this); // PHASE_ALLOCATE_OBJECTS
objectAllocator.Run();

/* Add any internal blocks/trees we may need */

fgAddInternal();
Expand Down
4 changes: 3 additions & 1 deletion src/jit/objectalloc.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -155,7 +155,9 @@ GenTree* ObjectAllocator::MorphAllocObjNodeIntoHelperCall(GenTreeAllocObj* alloc

GenTree* op1 = allocObj->gtGetOp1();

GenTree* helperCall = comp->fgMorphIntoHelperCall(allocObj, allocObj->gtNewHelper, comp->gtNewArgList(op1));
const bool morphArgs = false;
GenTree* helperCall =
comp->fgMorphIntoHelperCall(allocObj, allocObj->gtNewHelper, comp->gtNewArgList(op1), morphArgs);

return helperCall;
}
Expand Down
3 changes: 3 additions & 0 deletions src/jit/objectalloc.h
Original file line number Diff line number Diff line change
Expand Up @@ -53,6 +53,9 @@ inline ObjectAllocator::ObjectAllocator(Compiler* comp)
, m_IsObjectStackAllocationEnabled(false)
, m_AnalysisDone(false)
{
// Disable checks since this phase runs before fgComputePreds phase.
// Checks are not expected to pass before fgComputePreds.
doChecks = false;
}

inline bool ObjectAllocator::IsObjectStackAllocationEnabled() const
Expand Down
13 changes: 9 additions & 4 deletions src/jit/phase.h
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,8 @@ class Phase
virtual void Run();

protected:
Phase(Compiler* _comp, const char* _name, Phases _phase = PHASE_NUMBER_OF) : comp(_comp), name(_name), phase(_phase)
Phase(Compiler* _comp, const char* _name, Phases _phase = PHASE_NUMBER_OF)
: comp(_comp), name(_name), phase(_phase), doChecks(true)
{
}

Expand All @@ -23,6 +24,7 @@ class Phase
Compiler* comp;
const char* name;
Phases phase;
bool doChecks;
};

inline void Phase::Run()
Expand All @@ -42,7 +44,7 @@ inline void Phase::PrePhase()
comp->fgDispBasicBlocks(true);
}

if (comp->expensiveDebugCheckLevel >= 2)
if (doChecks && comp->expensiveDebugCheckLevel >= 2)
{
// If everyone used the Phase class, this would duplicate the PostPhase() from the previous phase.
// But, not everyone does, so go ahead and do the check here, too.
Expand All @@ -69,8 +71,11 @@ inline void Phase::PostPhase()
}

#ifdef DEBUG
comp->fgDebugCheckBBlist();
comp->fgDebugCheckLinks();
if (doChecks)
{
comp->fgDebugCheckBBlist();
comp->fgDebugCheckLinks();
}
#endif // DEBUG
}

Expand Down

0 comments on commit a3097e4

Please sign in to comment.