JIT: convert codegen over to new style phases (#32899)

Convert codegen over to new style phases. To facilitate this, some
of the state that was passed around in locals in `genGenerateCode`
are now fields on the codegen object.

Also add a final "pseudo phase" in case we see very late asserts.

Fixes #32497.

diff --git a/src/coreclr/src/jit/codegen.h b/src/coreclr/src/jit/codegen.h
index 04fd720..dcc8445 100644 (file)
--- a/src/coreclr/src/jit/codegen.h
+++ b/src/coreclr/src/jit/codegen.h
@@ -23,7 +23,7 @@
 #define FOREACH_REGISTER_FILE(file) (file) = &(this->intRegState);
 #endif
 
-class CodeGen : public CodeGenInterface
+class CodeGen final : public CodeGenInterface
 {
     friend class emitter;
     friend class DisAssembler;
@@ -33,6 +33,11 @@ public:
     CodeGen(Compiler* theCompiler);
 
     virtual void genGenerateCode(void** codePtr, ULONG* nativeSizeOfCode);
+
+    void genGenerateMachineCode();
+    void genEmitMachineCode();
+    void genEmitUnwindDebugGCandEH();
+
     // TODO-Cleanup: Abstract out the part of this that finds the addressing mode, and
     // move it to Lower
     virtual bool genCreateAddrMode(GenTree*  addr,
@@ -186,6 +191,12 @@ protected:
     // the current (pending) label ref, a label which has been referenced but not yet seen
     BasicBlock* genPendingCallLabel;
 
+    void**   codePtr;
+    ULONG*   nativeSizeOfCode;
+    unsigned codeSize;
+    void*    coldCodePtr;
+    void*    consPtr;
+
 #ifdef DEBUG
     // Last instr we have displayed for dspInstrs
     unsigned genCurDispOffset;
@@ -242,6 +253,9 @@ protected:
     //
     //-------------------------------------------------------------------------
 
+    unsigned prologSize;
+    unsigned epilogSize;
+
     //
     // Prolog functions and data (there are a few exceptions for more generally used things)
     //
@@ -1492,4 +1506,35 @@ inline void CodeGen::inst_RV_CL(instruction ins, regNumber reg, var_types type)
     inst_RV(ins, reg, type);
 }
 
+/*****************************************************************************/
+
+// A simple phase that just invokes a method on the codegen instance
+//
+class CodeGenPhase final : public Phase
+{
+public:
+    CodeGenPhase(CodeGen* _codeGen, Phases _phase, void (CodeGen::*_action)())
+        : Phase(_codeGen->GetCompiler(), _phase), codeGen(_codeGen), action(_action)
+    {
+    }
+
+protected:
+    virtual void DoPhase() override
+    {
+        (codeGen->*action)();
+    }
+
+private:
+    CodeGen* codeGen;
+    void (CodeGen::*action)();
+};
+
+// Wrapper for using CodeGenPhase
+//
+inline void DoPhase(CodeGen* _codeGen, Phases _phase, void (CodeGen::*_action)())
+{
+    CodeGenPhase phase(_codeGen, _phase, _action);
+    phase.Run();
+}
+
 #endif // _CODEGEN_H_

diff --git a/src/coreclr/src/jit/codegencommon.cpp b/src/coreclr/src/jit/codegencommon.cpp
index 61ad60b..91457cf 100644 (file)
--- a/src/coreclr/src/jit/codegencommon.cpp
+++ b/src/coreclr/src/jit/codegencommon.cpp
@@ -2025,13 +2025,16 @@ void CodeGen::genInsertNopForUnwinder(BasicBlock* block)
 
 #endif // FEATURE_EH_FUNCLETS
 
-/*****************************************************************************
- *
- *  Generate code for the function.
- */
-
+//----------------------------------------------------------------------
+// genGenerateCode: Generate code for the function.
+//
+// Arguments:
+//     codePtr [OUT] - address of generated code
+//     nativeSizeOfCode [OUT] - length of generated code in bytes
+//
 void CodeGen::genGenerateCode(void** codePtr, ULONG* nativeSizeOfCode)
 {
+
 #ifdef DEBUG
     if (verbose)
     {
@@ -2040,12 +2043,19 @@ void CodeGen::genGenerateCode(void** codePtr, ULONG* nativeSizeOfCode)
     }
 #endif
 
-    unsigned codeSize;
-    unsigned prologSize;
-    unsigned epilogSize;
+    this->codePtr          = codePtr;
+    this->nativeSizeOfCode = nativeSizeOfCode;
 
-    void* consPtr;
+    DoPhase(this, PHASE_GENERATE_CODE, &CodeGen::genGenerateMachineCode);
+    DoPhase(this, PHASE_EMIT_CODE, &CodeGen::genEmitMachineCode);
+    DoPhase(this, PHASE_EMIT_GCEH, &CodeGen::genEmitUnwindDebugGCandEH);
+}
 
+//----------------------------------------------------------------------
+// genGenerateMachineCode -- determine which machine instructions to emit
+//
+void CodeGen::genGenerateMachineCode()
+{
 #ifdef DEBUG
     genInterruptibleUsed = true;
 
@@ -2237,7 +2247,13 @@ void CodeGen::genGenerateCode(void** codePtr, ULONG* nativeSizeOfCode)
     GetEmitter()->emitJumpDistBind();
 
     /* The code is now complete and final; it should not change after this. */
+}
 
+//----------------------------------------------------------------------
+// genEmitMachineCode -- emit the actual machine instruction code
+//
+void CodeGen::genEmitMachineCode()
+{
     /* Compute the size of the code sections that we are going to ask the VM
        to allocate. Note that this might not be precisely the size of the
        code we emit, though it's fatal if we emit more code than the size we
@@ -2283,8 +2299,6 @@ void CodeGen::genGenerateCode(void** codePtr, ULONG* nativeSizeOfCode)
 
 #endif // DISPLAY_SIZES
 
-    void* coldCodePtr;
-
     bool trackedStackPtrsContig; // are tracked stk-ptrs contiguous ?
 
 #if defined(TARGET_AMD64) || defined(TARGET_ARM64)
@@ -2296,14 +2310,10 @@ void CodeGen::genGenerateCode(void** codePtr, ULONG* nativeSizeOfCode)
     trackedStackPtrsContig = !compiler->opts.compDbgEnC;
 #endif
 
-    compiler->EndPhase(PHASE_GENERATE_CODE);
-
     codeSize = GetEmitter()->emitEndCodeGen(compiler, trackedStackPtrsContig, GetInterruptible(),
                                             IsFullPtrRegMapRequired(), compiler->compHndBBtabCount, &prologSize,
                                             &epilogSize, codePtr, &coldCodePtr, &consPtr);
 
-    compiler->EndPhase(PHASE_EMIT_CODE);
-
 #ifdef DEBUG
     assert(compiler->compCodeGenDone == false);
 
@@ -2367,7 +2377,13 @@ void CodeGen::genGenerateCode(void** codePtr, ULONG* nativeSizeOfCode)
     // Don't start a method in the last 7 bytes of a 16-byte alignment area
     //   unless we are generating SMALL_CODE
     // noway_assert( (((unsigned)(*codePtr) % 16) <= 8) || (compiler->compCodeOpt() == SMALL_CODE));
+}
 
+//----------------------------------------------------------------------
+// genEmitUnwindDebugGCandEH: emit unwind, debug, gc, and EH info
+//
+void CodeGen::genEmitUnwindDebugGCandEH()
+{
     /* Now that the code is issued, we can finalize and emit the unwind data */
 
     compiler->unwindEmit(*codePtr, coldCodePtr);
@@ -2504,8 +2520,6 @@ void CodeGen::genGenerateCode(void** codePtr, ULONG* nativeSizeOfCode)
     grossNCsize += codeSize + dataSize;
 
 #endif // DISPLAY_SIZES
-
-    compiler->EndPhase(PHASE_EMIT_GCEH);
 }
 
 /*****************************************************************************

diff --git a/src/coreclr/src/jit/codegeninterface.h b/src/coreclr/src/jit/codegeninterface.h
index 5868529..9d4f49f 100644 (file)
--- a/src/coreclr/src/jit/codegeninterface.h
+++ b/src/coreclr/src/jit/codegeninterface.h
@@ -66,6 +66,11 @@ public:
     CodeGenInterface(Compiler* theCompiler);
     virtual void genGenerateCode(void** codePtr, ULONG* nativeSizeOfCode) = 0;
 
+    Compiler* GetCompiler() const
+    {
+        return compiler;
+    }
+
     // genSpillVar is called by compUpdateLifeVar.
     // TODO-Cleanup: We should handle the spill directly in CodeGen, rather than
     // calling it from compUpdateLifeVar.  Then this can be non-virtual.

diff --git a/src/coreclr/src/jit/compiler.cpp b/src/coreclr/src/jit/compiler.cpp
index 60050c6..ffac4e1 100644 (file)
--- a/src/coreclr/src/jit/compiler.cpp
+++ b/src/coreclr/src/jit/compiler.cpp
@@ -4910,6 +4910,10 @@ void Compiler::compCompile(void** methodCodePtr, ULONG* methodCodeSize, JitFlags
     // Generate code
     codeGen->genGenerateCode(methodCodePtr, methodCodeSize);
 
+    // We're done -- set the active phase to the last phase
+    // (which isn't really a phase)
+    mostRecentlyActivePhase = PHASE_POST_EMIT;
+
 #ifdef FEATURE_JIT_METHOD_PERF
     if (pCompJitTimer)
     {

diff --git a/src/coreclr/src/jit/compphases.h b/src/coreclr/src/jit/compphases.h
index 12ef431..f9e0684 100644 (file)
--- a/src/coreclr/src/jit/compphases.h
+++ b/src/coreclr/src/jit/compphases.h
@@ -103,6 +103,7 @@ CompPhaseNameMacro(PHASE_LINEAR_SCAN_RESOLVE,    "LSRA resolve",
 CompPhaseNameMacro(PHASE_GENERATE_CODE,          "Generate code",                  "CODEGEN",  false, -1, false)
 CompPhaseNameMacro(PHASE_EMIT_CODE,              "Emit code",                      "EMIT",     false, -1, false)
 CompPhaseNameMacro(PHASE_EMIT_GCEH,              "Emit GC+EH tables",              "EMT-GCEH", false, -1, false)
+CompPhaseNameMacro(PHASE_POST_EMIT,              "Post-Emit",                      "POST-EMIT", false, -1, false)
 
 #if MEASURE_CLRAPI_CALLS
 // The following is a "pseudo-phase" - it aggregates timing info

diff --git a/src/coreclr/src/jit/phase.h b/src/coreclr/src/jit/phase.h
index 42b9529..29a3399 100644 (file)
--- a/src/coreclr/src/jit/phase.h
+++ b/src/coreclr/src/jit/phase.h
@@ -48,7 +48,7 @@ private:
     A action;
 };
 
-// Wrapper for using ActionPhase
+// Wrappers for using ActionPhase
 //
 template <typename A>
 void DoPhase(Compiler* _compiler, Phases _phase, A _action)