From db90a849eab60d9843b8151ab14ba998faee3825 Mon Sep 17 00:00:00 2001 From: Bruce Forstall Date: Wed, 3 Jan 2024 23:37:14 -0800 Subject: [PATCH] Refine usage of late disassembler and emitter unit tests (#96467) * Refine usage of late disassembler and emitter unit tests Introduce `DOTNET_JitEmitUnitTests`. Set this to the function or functions into which you want the unit tests to be written. E.g., `DOTNET_JitEmitUnitTests=Main` or `DOTNET_JitEmitUnitTests=*`. Rename `DOTNET_JitDumpEmitUnitTests` to `DOTNET_JitEmitUnitTestsSections`. Make late disassembler work for altjit: use the "RW" address for the generated code. * Update src/coreclr/jit/codegenlinear.cpp Co-authored-by: Kunal Pathak --------- Co-authored-by: Kunal Pathak --- src/coreclr/jit/codegen.h | 3 +++ src/coreclr/jit/codegencommon.cpp | 24 +++++++++++++++-------- src/coreclr/jit/codegenlinear.cpp | 18 +++++++++++------ src/coreclr/jit/codegenxarch.cpp | 1 - src/coreclr/jit/disasm.cpp | 17 ++++++++++------ src/coreclr/jit/disasm.h | 7 ++++++- src/coreclr/jit/emit.cpp | 32 ++++++++++++++++++++----------- src/coreclr/jit/emitpub.h | 5 ++++- src/coreclr/jit/jitconfigvalues.h | 10 ++++++---- src/coreclr/jit/utils.cpp | 6 +++--- 10 files changed, 82 insertions(+), 41 deletions(-) diff --git a/src/coreclr/jit/codegen.h b/src/coreclr/jit/codegen.h index 7917c0dd608d69..19a7b901d7c672 100644 --- a/src/coreclr/jit/codegen.h +++ b/src/coreclr/jit/codegen.h @@ -190,10 +190,13 @@ class CodeGen final : public CodeGenInterface BasicBlock* genPendingCallLabel; void** codePtr; + void* codePtrRW; uint32_t* nativeSizeOfCode; unsigned codeSize; void* coldCodePtr; + void* coldCodePtrRW; void* consPtr; + void* consPtrRW; // Last instr we have displayed for dspInstrs unsigned genCurDispOffset; diff --git a/src/coreclr/jit/codegencommon.cpp b/src/coreclr/jit/codegencommon.cpp index 3089b8e725e136..5cde37749329c3 100644 --- a/src/coreclr/jit/codegencommon.cpp +++ b/src/coreclr/jit/codegencommon.cpp @@ -1990,9 +1990,10 @@ void CodeGen::genEmitMachineCode() printf("; BEGIN METHOD %s\n", compiler->eeGetMethodFullName(compiler->info.compMethodHnd)); } - codeSize = GetEmitter()->emitEndCodeGen(compiler, trackedStackPtrsContig, GetInterruptible(), - IsFullPtrRegMapRequired(), compiler->compHndBBtabCount, &prologSize, - &epilogSize, codePtr, &coldCodePtr, &consPtr DEBUGARG(&instrCount)); + codeSize = + GetEmitter()->emitEndCodeGen(compiler, trackedStackPtrsContig, GetInterruptible(), IsFullPtrRegMapRequired(), + compiler->compHndBBtabCount, &prologSize, &epilogSize, codePtr, &codePtrRW, + &coldCodePtr, &coldCodePtrRW, &consPtr, &consPtrRW DEBUGARG(&instrCount)); #ifdef DEBUG assert(compiler->compCodeGenDone == false); @@ -2085,7 +2086,7 @@ void CodeGen::genEmitUnwindDebugGCandEH() genSetScopeInfo(); -#ifdef LATE_DISASM +#if defined(LATE_DISASM) || defined(DEBUG) unsigned finalHotCodeSize; unsigned finalColdCodeSize; if (compiler->fgFirstColdBlock != nullptr) @@ -2107,14 +2108,21 @@ void CodeGen::genEmitUnwindDebugGCandEH() finalHotCodeSize = codeSize; finalColdCodeSize = 0; } - getDisAssembler().disAsmCode((BYTE*)*codePtr, finalHotCodeSize, (BYTE*)coldCodePtr, finalColdCodeSize); +#endif // defined(LATE_DISASM) || defined(DEBUG) + +#ifdef LATE_DISASM + getDisAssembler().disAsmCode((BYTE*)*codePtr, (BYTE*)codePtrRW, finalHotCodeSize, (BYTE*)coldCodePtr, + (BYTE*)coldCodePtrRW, finalColdCodeSize); #endif // LATE_DISASM #ifdef DEBUG if (JitConfig.JitRawHexCode().contains(compiler->info.compMethodHnd, compiler->info.compClassHnd, &compiler->info.compMethodInfo->args)) { - BYTE* addr = (BYTE*)*codePtr + compiler->GetEmitter()->writeableOffset; + // NOTE: code in cold region is not supported. + + BYTE* dumpAddr = (BYTE*)codePtrRW; + size_t dumpSize = finalHotCodeSize; const WCHAR* rawHexCodeFilePath = JitConfig.JitRawHexCodeFile(); if (rawHexCodeFilePath) @@ -2124,7 +2132,7 @@ void CodeGen::genEmitUnwindDebugGCandEH() if (ec == 0) { assert(hexDmpf); - hexDump(hexDmpf, addr, codeSize); + hexDump(hexDmpf, dumpAddr, dumpSize); fclose(hexDmpf); } } @@ -2133,7 +2141,7 @@ void CodeGen::genEmitUnwindDebugGCandEH() FILE* dmpf = jitstdout(); fprintf(dmpf, "Generated native code for %s:\n", compiler->info.compFullName); - hexDump(dmpf, addr, codeSize); + hexDump(dmpf, dumpAddr, dumpSize); fprintf(dmpf, "\n\n"); } } diff --git a/src/coreclr/jit/codegenlinear.cpp b/src/coreclr/jit/codegenlinear.cpp index 18390b146268e5..fb009cca8a5d86 100644 --- a/src/coreclr/jit/codegenlinear.cpp +++ b/src/coreclr/jit/codegenlinear.cpp @@ -2672,12 +2672,12 @@ void CodeGen::genCodeForSetcc(GenTreeCC* setcc) #endif // !TARGET_LOONGARCH64 && !TARGET_RISCV64 /***************************************************************************** - * Unit testing of the emitter: If JitDumpEmitUnitTests is set, generate - * a bunch of instructions, then: - * Use DOTNET_JitLateDisasm=* to see if the late disassembler thinks the instructions are the same as we do. - * Or, use DOTNET_JitRawHexCode and DOTNET_JitRawHexCodeFile and disassemble the output file. + * Unit testing of the emitter: If JitEmitUnitTests is set for this function, generate + * a bunch of instructions, then either: + * 1. Use DOTNET_JitLateDisasm=* to see if the late disassembler thinks the instructions are the same as we do. Or, + * 2. Use DOTNET_JitRawHexCode and DOTNET_JitRawHexCodeFile and disassemble the output file with an external disassembler. * - * Possible values for JitDumpEmitUnitTests: + * Possible values for JitEmitUnitTestsSections: * Amd64: all, sse2 * Arm64: all, general, advsimd, sve */ @@ -2686,7 +2686,13 @@ void CodeGen::genCodeForSetcc(GenTreeCC* setcc) void CodeGen::genEmitterUnitTests() { - const WCHAR* unitTestSection = JitConfig.JitDumpEmitUnitTests(); + if (!JitConfig.JitEmitUnitTests().contains(compiler->info.compMethodHnd, compiler->info.compClassHnd, + &compiler->info.compMethodInfo->args)) + { + return; + } + + const WCHAR* unitTestSection = JitConfig.JitEmitUnitTestsSections(); if (unitTestSection == nullptr) { diff --git a/src/coreclr/jit/codegenxarch.cpp b/src/coreclr/jit/codegenxarch.cpp index ce2b104e4157cd..43553c42018482 100644 --- a/src/coreclr/jit/codegenxarch.cpp +++ b/src/coreclr/jit/codegenxarch.cpp @@ -9037,7 +9037,6 @@ void CodeGen::genEmitHelperCall(unsigned helper, int argSize, emitAttr retSize, void CodeGen::genAmd64EmitterUnitTestsSse2() { - assert(verbose); emitter* theEmitter = GetEmitter(); // diff --git a/src/coreclr/jit/disasm.cpp b/src/coreclr/jit/disasm.cpp index 8d7111d015ce7d..2a49f9d8cb55c5 100644 --- a/src/coreclr/jit/disasm.cpp +++ b/src/coreclr/jit/disasm.cpp @@ -1464,7 +1464,12 @@ void DisAssembler::disRecordRelocation(size_t relocAddr, size_t targetAddr) * Disassemble the code which has been generated */ -void DisAssembler::disAsmCode(BYTE* hotCodePtr, size_t hotCodeSize, BYTE* coldCodePtr, size_t coldCodeSize) +void DisAssembler::disAsmCode(BYTE* hotCodePtr, + BYTE* hotCodePtrRW, + size_t hotCodeSize, + BYTE* coldCodePtr, + BYTE* coldCodePtrRW, + size_t coldCodeSize) { if (!disComp->opts.doLateDisasm) { @@ -1511,7 +1516,7 @@ void DisAssembler::disAsmCode(BYTE* hotCodePtr, size_t hotCodeSize, BYTE* coldCo fprintf(disAsmFile, "************************** %hs:%hs size 0x%04IX **************************\n\n", disCurClassName, disCurMethodName, hotCodeSize); - fprintf(disAsmFile, "Base address : %ph\n", dspAddr(hotCodePtr)); + fprintf(disAsmFile, "Base address : %ph (RW: %ph)\n", dspAddr(hotCodePtr), dspAddr(hotCodePtrRW)); } else { @@ -1519,14 +1524,14 @@ void DisAssembler::disAsmCode(BYTE* hotCodePtr, size_t hotCodeSize, BYTE* coldCo "************************** %hs:%hs hot size 0x%04IX cold size 0x%04IX **************************\n\n", disCurClassName, disCurMethodName, hotCodeSize, coldCodeSize); - fprintf(disAsmFile, "Hot address : %ph\n", dspAddr(hotCodePtr)); - fprintf(disAsmFile, "Cold address : %ph\n", dspAddr(coldCodePtr)); + fprintf(disAsmFile, "Hot address : %ph (RW: %ph)\n", dspAddr(hotCodePtr), dspAddr(hotCodePtrRW)); + fprintf(disAsmFile, "Cold address : %ph (RW: %ph)\n", dspAddr(coldCodePtr), dspAddr(coldCodePtrRW)); } disStartAddr = 0; - disHotCodeBlock = (size_t)hotCodePtr; + disHotCodeBlock = (size_t)hotCodePtrRW; disHotCodeSize = hotCodeSize; - disColdCodeBlock = (size_t)coldCodePtr; + disColdCodeBlock = (size_t)coldCodePtrRW; disColdCodeSize = coldCodeSize; disTotalCodeSize = disHotCodeSize + disColdCodeSize; diff --git a/src/coreclr/jit/disasm.h b/src/coreclr/jit/disasm.h index f56028b3ae4179..3e8560f95ae6d2 100644 --- a/src/coreclr/jit/disasm.h +++ b/src/coreclr/jit/disasm.h @@ -93,7 +93,12 @@ class DisAssembler void disOpenForLateDisAsm(const char* curMethodName, const char* curClassName, PCCOR_SIGNATURE sig); // Disassemble a buffer: called after code for a method is generated. - void disAsmCode(BYTE* hotCodePtr, size_t hotCodeSize, BYTE* coldCodePtr, size_t coldCodeSize); + void disAsmCode(BYTE* hotCodePtr, + BYTE* hotCodePtrRW, + size_t hotCodeSize, + BYTE* coldCodePtr, + BYTE* coldCodePtrRW, + size_t coldCodeSize); // Register an address to be associated with a method handle. void disSetMethod(size_t addr, CORINFO_METHOD_HANDLE methHnd); diff --git a/src/coreclr/jit/emit.cpp b/src/coreclr/jit/emit.cpp index 27056d0d710382..89b2b983f1711b 100644 --- a/src/coreclr/jit/emit.cpp +++ b/src/coreclr/jit/emit.cpp @@ -6612,19 +6612,23 @@ void emitter::emitComputeCodeSizes() } //------------------------------------------------------------------------ -// emitEndCodeGen: called at end of code generation to create code, data, and gc info +// emitEndCodeGen: called at end of code generation to create code, data, and GC info // // Arguments: -// comp - compiler instance +// comp - compiler instance // contTrkPtrLcls - true if tracked stack pointers are contiguous on the stack -// fullInt - true if method has fully interruptible gc reporting -// fullPtrMap - true if gc reporting should use full register pointer map -// xcptnsCount - number of EH clauses to report for the method -// prologSize [OUT] - prolog size in bytes -// epilogSize [OUT] - epilog size in bytes (see notes) -// codeAddr [OUT] - address of the code buffer -// coldCodeAddr [OUT] - address of the cold code buffer (if any) -// consAddr [OUT] - address of the read only constant buffer (if any) +// fullyInt - true if method has fully interruptible GC reporting +// fullPtrMap - true if gc reporting should use full register pointer map +// xcptnsCount - number of EH clauses to report for the method +// prologSize - [OUT] prolog size in bytes +// epilogSize - [OUT] epilog size in bytes (see notes) +// codeAddr - [OUT] address of the code buffer +// codeAddrRW - [OUT] Read/write address of the code buffer +// coldCodeAddr - [OUT] address of the cold code buffer (if any) +// coldCodeAddrRW - [OUT] Read/write address of the cold code buffer (if any) +// consAddr - [OUT] address of the read only constant buffer (if any) +// consAddrRW - [OUT] Read/write address of the read only constant buffer (if any) +// instrCount - [OUT] [DEBUG ONLY] number of instructions generated. // // Notes: // Currently, in methods with multiple epilogs, all epilogs must have the same @@ -6642,8 +6646,11 @@ unsigned emitter::emitEndCodeGen(Compiler* comp, unsigned* prologSize, unsigned* epilogSize, void** codeAddr, + void** codeAddrRW, void** coldCodeAddr, - void** consAddr DEBUGARG(unsigned* instrCount)) + void** coldCodeAddrRW, + void** consAddr, + void** consAddrRW DEBUGARG(unsigned* instrCount)) { #ifdef DEBUG if (emitComp->verbose) @@ -6900,8 +6907,11 @@ unsigned emitter::emitEndCodeGen(Compiler* comp, /* Give the block addresses to the caller and other functions here */ *codeAddr = emitCodeBlock = codeBlock; + *codeAddrRW = codeBlockRW; *coldCodeAddr = emitColdCodeBlock = coldCodeBlock; + *coldCodeAddrRW = coldCodeBlockRW; *consAddr = emitConsBlock = consBlock; + *consAddrRW = consBlockRW; /* Nothing has been pushed on the stack */ CLANG_FORMAT_COMMENT_ANCHOR; diff --git a/src/coreclr/jit/emitpub.h b/src/coreclr/jit/emitpub.h index 9fcb29f42b7dbc..674b98a7f4bacd 100644 --- a/src/coreclr/jit/emitpub.h +++ b/src/coreclr/jit/emitpub.h @@ -30,8 +30,11 @@ unsigned emitEndCodeGen(Compiler* comp, unsigned* prologSize, unsigned* epilogSize, void** codeAddr, + void** codeAddrRW, void** coldCodeAddr, - void** consAddr DEBUGARG(unsigned* instrCount)); + void** coldCodeAddrRW, + void** consAddr, + void** consAddrRW DEBUGARG(unsigned* instrCount)); /************************************************************************/ /* Method prolog and epilog */ diff --git a/src/coreclr/jit/jitconfigvalues.h b/src/coreclr/jit/jitconfigvalues.h index 504542bb926ca2..ca672606eca40d 100644 --- a/src/coreclr/jit/jitconfigvalues.h +++ b/src/coreclr/jit/jitconfigvalues.h @@ -199,8 +199,10 @@ CONFIG_METHODSET(JitGCDump, W("JitGCDump")) CONFIG_METHODSET(JitDebugDump, W("JitDebugDump")) CONFIG_METHODSET(JitHalt, W("JitHalt")) // Emits break instruction into jitted code CONFIG_METHODSET(JitInclude, W("JitInclude")) -CONFIG_METHODSET(JitLateDisasm, W("JitLateDisasm")) -CONFIG_METHODSET(JitMinOptsName, W("JITMinOptsName")) // Forces MinOpts for a named function +CONFIG_METHODSET(JitLateDisasm, W("JitLateDisasm")) // Generate late disassembly for the specified methods. +CONFIG_STRING(JitLateDisasmTo, W("JitLateDisasmTo")) // If set, sends late disassembly output to this file instead of + // stdout/JitStdOutFile. +CONFIG_METHODSET(JitMinOptsName, W("JITMinOptsName")) // Forces MinOpts for a named function CONFIG_METHODSET(JitNoProcedureSplitting, W("JitNoProcedureSplitting")) // Disallow procedure splitting for specified // methods CONFIG_METHODSET(JitNoProcedureSplittingEH, W("JitNoProcedureSplittingEH")) // Disallow procedure splitting for @@ -235,7 +237,6 @@ CONFIG_INTEGER(JitDumpFgBlockOrder, W("JitDumpFgBlockOrder"), 0) // 0 == bbNext // order CONFIG_INTEGER(JitDumpFgMemorySsa, W("JitDumpFgMemorySsa"), 0) // non-zero: show memory phis + SSA/VNs -CONFIG_STRING(JitLateDisasmTo, W("JITLateDisasmTo")) CONFIG_STRING(JitRange, W("JitRange")) CONFIG_STRING(JitStressModeNames, W("JitStressModeNames")) // Internal Jit stress mode: stress using the given set of // stress mode names, e.g. STRESS_REGS, STRESS_TAILCALL @@ -243,7 +244,8 @@ CONFIG_STRING(JitStressModeNamesNot, W("JitStressModeNamesNot")) // Internal Jit // given set of stress mode names, e.g. STRESS_REGS, // STRESS_TAILCALL CONFIG_STRING(JitStressRange, W("JitStressRange")) // Internal Jit stress mode -CONFIG_STRING(JitDumpEmitUnitTests, W("JitDumpEmitUnitTests")) // Dump unit tests from Emit +CONFIG_METHODSET(JitEmitUnitTests, W("JitEmitUnitTests")) // Generate emitter unit tests in the specified functions +CONFIG_STRING(JitEmitUnitTestsSections, W("JitEmitUnitTestsSections")) // Generate this set of unit tests /// /// JIT Hardware Intrinsics diff --git a/src/coreclr/jit/utils.cpp b/src/coreclr/jit/utils.cpp index 50885fb94da722..56e7673e5b8dd1 100644 --- a/src/coreclr/jit/utils.cpp +++ b/src/coreclr/jit/utils.cpp @@ -1321,14 +1321,14 @@ int SimpleSprintf_s(_In_reads_(cbBufSize - (pWriteStart - pBufStart)) char* pWri void hexDump(FILE* dmpf, BYTE* addr, size_t size) { - if (!size) + if (size == 0) { return; } - assert(addr); + assert(addr != nullptr); - for (unsigned i = 0; i < size; i++) + for (size_t i = 0; i < size; i++) { fprintf(dmpf, "%02X", *addr++); }