From a3097e42057cbd79dd7b8765bb64c2b0630938e3 Mon Sep 17 00:00:00 2001 From: Eugene Rozenfeld Date: Fri, 12 Oct 2018 17:35:28 -0700 Subject: [PATCH] Move ObjectAllocator phase to run right after inlining. (#20377) 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 --- src/jit/compiler.cpp | 5 ----- src/jit/compiler.h | 2 +- src/jit/compphases.h | 2 +- src/jit/morph.cpp | 12 ++++++++++-- src/jit/objectalloc.cpp | 4 +++- src/jit/objectalloc.h | 3 +++ src/jit/phase.h | 13 +++++++++---- 7 files changed, 27 insertions(+), 14 deletions(-) diff --git a/src/jit/compiler.cpp b/src/jit/compiler.cpp index 69bb7e99373d..f15ddae5b1a0 100644 --- a/src/jit/compiler.cpp +++ b/src/jit/compiler.cpp @@ -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 diff --git a/src/jit/compiler.h b/src/jit/compiler.h index 8b77a3e9380f..127914a14cec 100644 --- a/src/jit/compiler.h +++ b/src/jit/compiler.h @@ -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); diff --git a/src/jit/compphases.h b/src/jit/compphases.h index 0a2f999dd67f..7a85fe1e8aa6 100644 --- a/src/jit/compphases.h +++ b/src/jit/compphases.h @@ -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) @@ -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) diff --git a/src/jit/morph.cpp b/src/jit/morph.cpp index ddcf283f9c31..b28acab45a4c 100644 --- a/src/jit/morph.cpp +++ b/src/jit/morph.cpp @@ -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); @@ -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; } @@ -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(); diff --git a/src/jit/objectalloc.cpp b/src/jit/objectalloc.cpp index 93e60ee3c409..c4fdc11c53e7 100644 --- a/src/jit/objectalloc.cpp +++ b/src/jit/objectalloc.cpp @@ -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; } diff --git a/src/jit/objectalloc.h b/src/jit/objectalloc.h index ece050741dd5..d6a859d83c59 100644 --- a/src/jit/objectalloc.h +++ b/src/jit/objectalloc.h @@ -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 diff --git a/src/jit/phase.h b/src/jit/phase.h index d8e294008909..2077f94dd800 100644 --- a/src/jit/phase.h +++ b/src/jit/phase.h @@ -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) { } @@ -23,6 +24,7 @@ class Phase Compiler* comp; const char* name; Phases phase; + bool doChecks; }; inline void Phase::Run() @@ -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. @@ -69,8 +71,11 @@ inline void Phase::PostPhase() } #ifdef DEBUG - comp->fgDebugCheckBBlist(); - comp->fgDebugCheckLinks(); + if (doChecks) + { + comp->fgDebugCheckBBlist(); + comp->fgDebugCheckLinks(); + } #endif // DEBUG }