From 7e5317ee981f75b9098f255948b4035ca223ac2c Mon Sep 17 00:00:00 2001 From: Kunal Pathak Date: Mon, 6 Jul 2020 13:56:37 -0700 Subject: [PATCH 01/16] logging --- src/coreclr/src/jit/codegen.h | 2 +- src/coreclr/src/jit/emit.cpp | 4 ++ src/coreclr/src/jit/emitarm.cpp | 26 +++++--- src/coreclr/src/jit/instr.cpp | 2 +- src/coreclr/src/jit/lclvars.cpp | 60 +++++++++++++++++++ .../tests/src/JIT/jit64/mcc/common/common.il | 2 +- 6 files changed, 86 insertions(+), 10 deletions(-) diff --git a/src/coreclr/src/jit/codegen.h b/src/coreclr/src/jit/codegen.h index 2c71468717624d..ed2c8a9efcc45d 100644 --- a/src/coreclr/src/jit/codegen.h +++ b/src/coreclr/src/jit/codegen.h @@ -1407,7 +1407,7 @@ XXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXX void inst_SA_IV(instruction ins, unsigned ofs, int val, var_types type); void inst_RV_ST( - instruction ins, regNumber reg, TempDsc* tmp, unsigned ofs, var_types type, emitAttr size = EA_UNKNOWN); + instruction ins, regNumber reg, TempDsc* tmp, unsigned ofs, var_types type, emitAttr size = EA_UNKNOWN); // delete this method void inst_FS_ST(instruction ins, emitAttr size, TempDsc* tmp, unsigned ofs); void inst_TT(instruction ins, GenTree* tree, unsigned offs = 0, int shfv = 0, emitAttr size = EA_UNKNOWN); diff --git a/src/coreclr/src/jit/emit.cpp b/src/coreclr/src/jit/emit.cpp index 2613cd453e61a5..69b715e55820b3 100644 --- a/src/coreclr/src/jit/emit.cpp +++ b/src/coreclr/src/jit/emit.cpp @@ -4920,6 +4920,10 @@ unsigned emitter::emitEndCodeGen(Compiler* comp, else { printf("\nG_M%03u_IG%02u:\n", emitComp->compMethodID, ig->igNum); + if (ig->igNum == 1720) + { + printf("\n--------debug break--------\n"); + } } } #endif // DEBUG diff --git a/src/coreclr/src/jit/emitarm.cpp b/src/coreclr/src/jit/emitarm.cpp index b5ed5e83e08ac8..b7c43254cf909f 100644 --- a/src/coreclr/src/jit/emitarm.cpp +++ b/src/coreclr/src/jit/emitarm.cpp @@ -1677,6 +1677,10 @@ void emitter::emitIns_R_I( instruction ins, emitAttr attr, regNumber reg, target_ssize_t imm, insFlags flags /* = INS_FLAGS_DONT_CARE */) { + if (imm == 4088) + { + printf("------debug break inside emitIns_R_I"); + } insFormat fmt = IF_NONE; insFlags sf = INS_FLAGS_DONT_CARE; @@ -3523,7 +3527,7 @@ void emitter::emitIns_R_S(instruction ins, emitAttr attr, regNumber reg1, int va switch (ins) { - case INS_add: + case INS_add: // don't need INS_add. no one calls it. case INS_ldr: case INS_ldrh: case INS_ldrb: @@ -3553,12 +3557,20 @@ void emitter::emitIns_R_S(instruction ins, emitAttr attr, regNumber reg1, int va int disp; unsigned undisp; + if (offs == 1208) + { + printf("----debug break - this is it 2B!----"); + } + base = emitComp->lvaFrameAddress(varx, emitComp->funCurrentFunc()->funKind != FUNC_ROOT, ®2, offs, CodeGen::instIsFP(ins)); disp = base + offs; undisp = unsigned_abs(disp); - + if (undisp == 0x1000) + { + printf("----debug break - this is it!----"); + } if (CodeGen::instIsFP(ins)) { // all fp mem ops take 8 bit immediate, multiplied by 4, plus sign @@ -3600,10 +3612,10 @@ void emitter::emitIns_R_S(instruction ins, emitAttr attr, regNumber reg1, int va // Load disp into a register regNumber rsvdReg = codeGen->rsGetRsvdReg(); emitIns_genStackOffset(rsvdReg, varx, offs, /* isFloatUsage */ false); - fmt = IF_T2_E0; + fmt = IF_T2_E0; // not sure if here too there could be case where we set the regBase inside emitIns_genStackOffset() but then don't use it further while generating the offset. } } - else if (ins == INS_add) + else if (ins == INS_add) // no one calls this case. { if (isLowRegister(reg1) && (reg2 == REG_SP) && ((disp & 0x03fc) == disp)) { @@ -3626,13 +3638,13 @@ void emitter::emitIns_R_S(instruction ins, emitAttr attr, regNumber reg1, int va // Load disp into a register regNumber rsvdReg = codeGen->rsGetRsvdReg(); emitIns_genStackOffset(rsvdReg, varx, offs, /* isFloatUsage */ false); - emitIns_R_R_R(ins, attr, reg1, reg2, rsvdReg); + emitIns_R_R_R(ins, attr, reg1, reg2, rsvdReg); // not sure if here too there could be case where we set the regBase inside emitIns_genStackOffset() but then don't use it further while generating the offset. return; } } else if (ins == INS_movw || ins == INS_movt) { - fmt = IF_T2_N; + fmt = IF_T2_N; // assert (reg2 is not used in next instruction which can be add) } assert((fmt == IF_T1_J2) || (fmt == IF_T2_E0) || (fmt == IF_T2_H0) || (fmt == IF_T2_K1) || (fmt == IF_T2_L0) || @@ -3737,7 +3749,7 @@ void emitter::emitIns_S_R(instruction ins, emitAttr attr, regNumber reg1, int va { regNumber rsvdReg = codeGen->rsGetRsvdReg(); emitIns_genStackOffset(rsvdReg, varx, offs, /* isFloatUsage */ true); - emitIns_R_R(INS_add, EA_4BYTE, rsvdReg, reg2); + emitIns_R_R(INS_add, EA_4BYTE, rsvdReg, reg2); // not sure if here too there could be case where we set the regBase inside emitIns_genStackOffset() but then don't use it further while generating the offset. emitIns_R_R_I(ins, attr, reg1, rsvdReg, 0); return; } diff --git a/src/coreclr/src/jit/instr.cpp b/src/coreclr/src/jit/instr.cpp index ec776cbea9a911..7fd2ddf118b20d 100644 --- a/src/coreclr/src/jit/instr.cpp +++ b/src/coreclr/src/jit/instr.cpp @@ -1308,7 +1308,7 @@ void CodeGen::inst_RV_ST(instruction ins, emitAttr size, regNumber reg, GenTree* inst_RV_TT(ins, reg, tree, 0, size); } -void CodeGen::inst_RV_ST(instruction ins, regNumber reg, TempDsc* tmp, unsigned ofs, var_types type, emitAttr size) +void CodeGen::inst_RV_ST(instruction ins, regNumber reg, TempDsc* tmp, unsigned ofs, var_types type, emitAttr size) // delete this method { if (size == EA_UNKNOWN) { diff --git a/src/coreclr/src/jit/lclvars.cpp b/src/coreclr/src/jit/lclvars.cpp index f8757328e85130..01cc83271b2867 100644 --- a/src/coreclr/src/jit/lclvars.cpp +++ b/src/coreclr/src/jit/lclvars.cpp @@ -5931,6 +5931,10 @@ void Compiler::lvaAssignVirtualFrameOffsetsToLocals() // Allocate a pointer sized stack slot, since we may need to double align here // when lvaDoneFrameLayout == FINAL_FRAME_LAYOUT // + if (compMethodID == 39739) + { + printf("lvaAssignVirtualFrameOffsetsToLocal1 : %u\n", compLclFrameSize); + } lvaIncrementFrameSize(TARGET_POINTER_SIZE); stkOffs -= TARGET_POINTER_SIZE; @@ -5941,6 +5945,10 @@ void Compiler::lvaAssignVirtualFrameOffsetsToLocals() // offsets that we calculate for the stack frame will always // be greater (or equal) to what they can be in the final layout. // + if (compMethodID == 39739) + { + printf("lvaAssignVirtualFrameOffsetsToLocal2 : %u\n", compLclFrameSize); + } lvaIncrementFrameSize(TARGET_POINTER_SIZE); stkOffs -= TARGET_POINTER_SIZE; } @@ -5948,6 +5956,10 @@ void Compiler::lvaAssignVirtualFrameOffsetsToLocals() { if (((stkOffs + preSpillSize) % (2 * TARGET_POINTER_SIZE)) != 0) { + if (compMethodID == 39739) + { + printf("lvaAssignVirtualFrameOffsetsToLocal3 : %u\n", compLclFrameSize); + } lvaIncrementFrameSize(TARGET_POINTER_SIZE); stkOffs -= TARGET_POINTER_SIZE; } @@ -5990,6 +6002,10 @@ void Compiler::lvaAssignVirtualFrameOffsetsToLocals() noway_assert(codeGen->isFramePointerUsed()); #endif // For CORINFO_CALLCONV_PARAMTYPE (if needed) + if (compMethodID == 39739) + { + printf("lvaAssignVirtualFrameOffsetsToLocal4 : %u\n", compLclFrameSize); + } lvaIncrementFrameSize(TARGET_POINTER_SIZE); stkOffs -= TARGET_POINTER_SIZE; lvaCachedGenericContextArgOffs = stkOffs; @@ -5998,6 +6014,10 @@ void Compiler::lvaAssignVirtualFrameOffsetsToLocals() else if (lvaKeepAliveAndReportThis()) { // When "this" is also used as generic context arg. + if (compMethodID == 39739) + { + printf("lvaAssignVirtualFrameOffsetsToLocal5 : %u\n", compLclFrameSize); + } lvaIncrementFrameSize(TARGET_POINTER_SIZE); stkOffs -= TARGET_POINTER_SIZE; lvaCachedGenericContextArgOffs = stkOffs; @@ -6346,6 +6366,10 @@ void Compiler::lvaAssignVirtualFrameOffsetsToLocals() // stack slot since we may need to double align this LclVar // when lvaDoneFrameLayout == FINAL_FRAME_LAYOUT // + if (compMethodID == 39739) + { + printf("lvaAssignVirtualFrameOffsetsToLocal6 : %u\n", compLclFrameSize); + } lvaIncrementFrameSize(TARGET_POINTER_SIZE); stkOffs -= TARGET_POINTER_SIZE; } @@ -6353,6 +6377,10 @@ void Compiler::lvaAssignVirtualFrameOffsetsToLocals() { if (((stkOffs + preSpillSize) % (2 * TARGET_POINTER_SIZE)) != 0) { + if (compMethodID == 39739) + { + printf("lvaAssignVirtualFrameOffsetsToLocal7 : %u\n", compLclFrameSize); + } lvaIncrementFrameSize(TARGET_POINTER_SIZE); stkOffs -= TARGET_POINTER_SIZE; } @@ -6448,6 +6476,10 @@ void Compiler::lvaAssignVirtualFrameOffsetsToLocals() // Allocate a pointer sized stack slot, since we may need to double align here // when lvaDoneFrameLayout == FINAL_FRAME_LAYOUT // + if (compMethodID == 39739) + { + printf("lvaAssignVirtualFrameOffsetsToLocal8 : %u\n", compLclFrameSize); + } lvaIncrementFrameSize(TARGET_POINTER_SIZE); stkOffs -= TARGET_POINTER_SIZE; @@ -6460,6 +6492,10 @@ void Compiler::lvaAssignVirtualFrameOffsetsToLocals() // calculate for the stack frame are always greater than they will // be in the final layout. // + if (compMethodID == 39739) + { + printf("lvaAssignVirtualFrameOffsetsToLocal9 : %u\n", compLclFrameSize); + } lvaIncrementFrameSize(TARGET_POINTER_SIZE); stkOffs -= TARGET_POINTER_SIZE; } @@ -6468,6 +6504,10 @@ void Compiler::lvaAssignVirtualFrameOffsetsToLocals() { if (((stkOffs + preSpillSize) % (2 * TARGET_POINTER_SIZE)) != 0) { + if (compMethodID == 39739) + { + printf("lvaAssignVirtualFrameOffsetsToLocal10 : %u\n", compLclFrameSize); + } lvaIncrementFrameSize(TARGET_POINTER_SIZE); stkOffs -= TARGET_POINTER_SIZE; } @@ -6615,6 +6655,10 @@ int Compiler::lvaAllocLocalAndSetVirtualOffset(unsigned lclNum, unsigned size, i /* Reserve space on the stack by bumping the frame size */ + if (compMethodID == 39739) + { + printf("lvaAllocLocalAndSetVirtualOffset : %u\n", compLclFrameSize); + } lvaIncrementFrameSize(size); stkOffs -= size; lvaTable[lclNum].lvStkOffs = stkOffs; @@ -6739,6 +6783,10 @@ void Compiler::lvaAlignFrame() if (regPushedCountAligned != lclFrameSizeAligned) { + if (compMethodID == 39739) + { + printf("lvaAlignFrame : %u\n", compLclFrameSize); + } lvaIncrementFrameSize(TARGET_POINTER_SIZE); } @@ -6897,6 +6945,10 @@ int Compiler::lvaAllocateTemps(int stkOffs, bool mustDoubleAlign) if (((stkOffs + preSpillSize) % (2 * TARGET_POINTER_SIZE)) != 0) { spillTempSize += TARGET_POINTER_SIZE; + if (compMethodID == 39739) + { + printf("lvaAllocateTemp1 : %u\n", compLclFrameSize); + } lvaIncrementFrameSize(TARGET_POINTER_SIZE); stkOffs -= TARGET_POINTER_SIZE; } @@ -6905,6 +6957,10 @@ int Compiler::lvaAllocateTemps(int stkOffs, bool mustDoubleAlign) } spillTempSize += size; + if (compMethodID == 39739) + { + printf("lvaAllocateTemp2 : %u\n", compLclFrameSize); + } lvaIncrementFrameSize(size); stkOffs -= size; temp->tdSetTempOffs(stkOffs); @@ -6918,6 +6974,10 @@ int Compiler::lvaAllocateTemps(int stkOffs, bool mustDoubleAlign) { unsigned size = lvaGetMaxSpillTempSize(); + if (compMethodID == 39739) + { + printf("lvaAllocateTemp3 : %u\n", compLclFrameSize); + } lvaIncrementFrameSize(size); stkOffs -= size; } diff --git a/src/coreclr/tests/src/JIT/jit64/mcc/common/common.il b/src/coreclr/tests/src/JIT/jit64/mcc/common/common.il index dad1e37692638e..eb951451c915a1 100644 --- a/src/coreclr/tests/src/JIT/jit64/mcc/common/common.il +++ b/src/coreclr/tests/src/JIT/jit64/mcc/common/common.il @@ -96,7 +96,7 @@ // Code size 34 (0x22) .maxstack 8 IL_0000: ldarg.0 - IL_0001: ldstr "FAILED: [field {0}] => expected = {1}, actual = {2}" + IL_0001: ldstr "FAILED: [field {0}] => actual = {1}, expected = {2}" IL_0006: ldarg.1 IL_0007: ldarg.3 IL_0008: box [mscorlib]System.Double From 06ebea44248aba544d0450340a939085d0f8d0c1 Mon Sep 17 00:00:00 2001 From: Kunal Pathak Date: Mon, 6 Jul 2020 16:28:44 -0700 Subject: [PATCH 02/16] the fix --- src/coreclr/src/jit/emitarm.cpp | 26 ++++++++++++++++++++------ src/coreclr/src/jit/emitarm.h | 4 ++-- 2 files changed, 22 insertions(+), 8 deletions(-) diff --git a/src/coreclr/src/jit/emitarm.cpp b/src/coreclr/src/jit/emitarm.cpp index b7c43254cf909f..3bb2d12b7641ed 100644 --- a/src/coreclr/src/jit/emitarm.cpp +++ b/src/coreclr/src/jit/emitarm.cpp @@ -3518,7 +3518,7 @@ void emitter::emitIns_S(instruction ins, emitAttr attr, int varx, int offs) * * Add an instruction referencing a register and a stack-based local variable. */ -void emitter::emitIns_R_S(instruction ins, emitAttr attr, regNumber reg1, int varx, int offs) +void emitter::emitIns_R_S(instruction ins, emitAttr attr, regNumber reg1, int varx, int offs, regNumber* baseReg) { if (ins == INS_mov) { @@ -3564,6 +3564,10 @@ void emitter::emitIns_R_S(instruction ins, emitAttr attr, regNumber reg1, int va base = emitComp->lvaFrameAddress(varx, emitComp->funCurrentFunc()->funKind != FUNC_ROOT, ®2, offs, CodeGen::instIsFP(ins)); + if (baseReg != nullptr) + { + *baseReg = reg2; + } disp = base + offs; undisp = unsigned_abs(disp); @@ -3587,8 +3591,11 @@ void emitter::emitIns_R_S(instruction ins, emitAttr attr, regNumber reg1, int va else { regNumber rsvdReg = codeGen->rsGetRsvdReg(); - emitIns_genStackOffset(rsvdReg, varx, offs, /* isFloatUsage */ true); - emitIns_R_R(INS_add, EA_4BYTE, rsvdReg, reg2); + regNumber baseRegUsed; + emitIns_genStackOffset(rsvdReg, varx, offs, /* isFloatUsage */ true, &baseRegUsed); + + // Make sure to use updated baseReg + emitIns_R_R(INS_add, EA_4BYTE, rsvdReg, baseRegUsed); emitIns_R_R_I(ins, attr, reg1, rsvdReg, 0); return; } @@ -3675,7 +3682,7 @@ void emitter::emitIns_R_S(instruction ins, emitAttr attr, regNumber reg1, int va } // generate the offset of &varx + offs into a register -void emitter::emitIns_genStackOffset(regNumber r, int varx, int offs, bool isFloatUsage) +void emitter::emitIns_genStackOffset(regNumber r, int varx, int offs, bool isFloatUsage, regNumber* baseReg) { regNumber regBase; int base; @@ -3685,11 +3692,18 @@ void emitter::emitIns_genStackOffset(regNumber r, int varx, int offs, bool isFlo emitComp->lvaFrameAddress(varx, emitComp->funCurrentFunc()->funKind != FUNC_ROOT, ®Base, offs, isFloatUsage); disp = base + offs; - emitIns_R_S(INS_movw, EA_4BYTE, r, varx, offs); + emitIns_R_S(INS_movw, EA_4BYTE, r, varx, offs, ®Base); if ((disp & 0xffff) != disp) { - emitIns_R_S(INS_movt, EA_4BYTE, r, varx, offs); + regNumber regBaseUsedInMovT; + emitIns_R_S(INS_movt, EA_4BYTE, r, varx, offs, ®BaseUsedInMovT); + assert(regBase == regBaseUsedInMovT); + } + + if (baseReg != nullptr) + { + *baseReg = regBase; } } diff --git a/src/coreclr/src/jit/emitarm.h b/src/coreclr/src/jit/emitarm.h index fe6a7a0c0790a7..a1eec9a894fe44 100644 --- a/src/coreclr/src/jit/emitarm.h +++ b/src/coreclr/src/jit/emitarm.h @@ -267,11 +267,11 @@ void emitIns_C(instruction ins, emitAttr attr, CORINFO_FIELD_HANDLE fdlHnd, int void emitIns_S(instruction ins, emitAttr attr, int varx, int offs); -void emitIns_genStackOffset(regNumber r, int varx, int offs, bool isFloatUsage); +void emitIns_genStackOffset(regNumber r, int varx, int offs, bool isFloatUsage, regNumber* baseReg = nullptr); void emitIns_S_R(instruction ins, emitAttr attr, regNumber ireg, int varx, int offs); -void emitIns_R_S(instruction ins, emitAttr attr, regNumber ireg, int varx, int offs); +void emitIns_R_S(instruction ins, emitAttr attr, regNumber ireg, int varx, int offs, regNumber* baseReg = nullptr); void emitIns_S_I(instruction ins, emitAttr attr, int varx, int offs, int val); From 3b3b91d4c479e913e2fe3a10e37d8bae20ec068c Mon Sep 17 00:00:00 2001 From: Kunal Pathak Date: Mon, 6 Jul 2020 16:31:25 -0700 Subject: [PATCH 03/16] revert lclvars.cpp changes --- src/coreclr/src/jit/lclvars.cpp | 148 ++++++++++---------------------- 1 file changed, 44 insertions(+), 104 deletions(-) diff --git a/src/coreclr/src/jit/lclvars.cpp b/src/coreclr/src/jit/lclvars.cpp index 01cc83271b2867..184b19ff46531a 100644 --- a/src/coreclr/src/jit/lclvars.cpp +++ b/src/coreclr/src/jit/lclvars.cpp @@ -292,9 +292,9 @@ void Compiler::lvaInitTypeRef() } if ( // If there already exist unsafe buffers, don't mark more structs as unsafe - // as that will cause them to be placed along with the real unsafe buffers, - // unnecessarily exposing them to overruns. This can affect GS tests which - // intentionally do buffer-overruns. + // as that will cause them to be placed along with the real unsafe buffers, + // unnecessarily exposing them to overruns. This can affect GS tests which + // intentionally do buffer-overruns. !getNeedsGSSecurityCookie() && // GS checks require the stack to be re-ordered, which can't be done with EnC !opts.compDbgEnC && compStressCompile(STRESS_UNSAFE_BUFFER_CHECKS, 25)) @@ -352,7 +352,7 @@ void Compiler::lvaInitArgs(InitVarDscInfo* varDscInfo) /* If we have a hidden return-buffer parameter, that comes here */ lvaInitRetBuffArg(varDscInfo); -//====================================================================== + //====================================================================== #if USER_ARGS_COME_LAST //@GENERICS: final instantiation-info argument for shared generic methods @@ -559,9 +559,9 @@ void Compiler::lvaInitRetBuffArg(InitVarDscInfo* varDscInfo) /*****************************************************************************/ void Compiler::lvaInitUserArgs(InitVarDscInfo* varDscInfo) { -//------------------------------------------------------------------------- -// Walk the function signature for the explicit arguments -//------------------------------------------------------------------------- + //------------------------------------------------------------------------- + // Walk the function signature for the explicit arguments + //------------------------------------------------------------------------- #if defined(TARGET_X86) // Only (some of) the implicit args are enregistered for varargs @@ -1033,8 +1033,8 @@ void Compiler::lvaInitUserArgs(InitVarDscInfo* varDscInfo) #if defined(TARGET_X86) varDsc->lvStkOffs = compArgSize; #else // !TARGET_X86 - // TODO-CQ: We shouldn't have to go as far as to declare these - // address-exposed -- DoNotEnregister should suffice. + // TODO-CQ: We shouldn't have to go as far as to declare these + // address-exposed -- DoNotEnregister should suffice. lvaSetVarAddrExposed(varDscInfo->varNum); #endif // !TARGET_X86 } @@ -1313,7 +1313,7 @@ void Compiler::lvaInitVarDsc(LclVarDsc* varDsc, varDsc->lvIsImplicitByRef = 0; #endif // defined(TARGET_AMD64) || defined(TARGET_ARM64) -// Set the lvType (before this point it is TYP_UNDEF). + // Set the lvType (before this point it is TYP_UNDEF). #ifdef FEATURE_HFA varDsc->SetHfaType(TYP_UNDEF); @@ -2198,7 +2198,7 @@ void Compiler::StructPromotionHelper::PromoteStructVar(unsigned lclNum) compiler->compFloatingPointUsed = true; } -// Now grab the temp for the field local. + // Now grab the temp for the field local. #ifdef DEBUG char buf[200]; @@ -3394,8 +3394,8 @@ void Compiler::lvaSortByRefCount() if (varDsc->lvAddrExposed) { varDsc->lvTracked = 0; - assert(varDsc->lvType != TYP_STRUCT || - varDsc->lvDoNotEnregister); // For structs, should have set this when we set lvAddrExposed. + assert(varDsc->lvType != TYP_STRUCT || varDsc->lvDoNotEnregister); // For structs, should have set this when + // we set lvAddrExposed. } else if (varTypeIsStruct(varDsc)) { @@ -3549,8 +3549,8 @@ void LclVarDsc::lvaDisqualifyVar() #endif // ASSERTION_PROP /********************************************************************************** -* Get stack size of the varDsc. -*/ + * Get stack size of the varDsc. + */ size_t LclVarDsc::lvArgStackSize() const { // Make sure this will have a stack size @@ -3591,8 +3591,8 @@ size_t LclVarDsc::lvArgStackSize() const } /********************************************************************************** -* Get type of a variable when passed as an argument. -*/ + * Get type of a variable when passed as an argument. + */ var_types LclVarDsc::lvaArgType() { var_types type = TypeGet(); @@ -4368,11 +4368,11 @@ inline void Compiler::lvaIncrementFrameSize(unsigned size) } /**************************************************************************** -* -* Return true if absolute offsets of temps are larger than vars, or in other -* words, did we allocate temps before of after vars. The /GS buffer overrun -* checks want temps to be at low stack addresses than buffers -*/ + * + * Return true if absolute offsets of temps are larger than vars, or in other + * words, did we allocate temps before of after vars. The /GS buffer overrun + * checks want temps to be at low stack addresses than buffers + */ bool Compiler::lvaTempsHaveLargerOffsetThanVars() { #ifdef TARGET_ARM @@ -4391,10 +4391,10 @@ bool Compiler::lvaTempsHaveLargerOffsetThanVars() } /**************************************************************************** -* -* Return an upper bound estimate for the size of the compiler spill temps -* -*/ + * + * Return an upper bound estimate for the size of the compiler spill temps + * + */ unsigned Compiler::lvaGetMaxSpillTempSize() { unsigned result = 0; @@ -5002,7 +5002,7 @@ void Compiler::lvaFixVirtualFrameOffsets() #endif ) #endif // !defined(TARGET_AMD64) - ) + ) { doAssignStkOffs = false; // Not on frame or an incomming stack arg } @@ -5311,8 +5311,8 @@ void Compiler::lvaAssignVirtualFrameOffsetsToArgs() // ret address slot, stack frame padding, alloca instructions, etc. // Note: This is the implementation for UNIX_AMD64 System V platforms. // -int Compiler::lvaAssignVirtualFrameOffsetToArg(unsigned lclNum, - unsigned argSize, +int Compiler::lvaAssignVirtualFrameOffsetToArg(unsigned lclNum, + unsigned argSize, int argOffs UNIX_AMD64_ABI_ONLY_ARG(int* callerArgOffset)) { noway_assert(lclNum < info.compArgsCount); @@ -5665,8 +5665,8 @@ int Compiler::lvaAssignVirtualFrameOffsetToArg(unsigned lclNum, (codeGen->regSet.rsMaskPreSpillAlign & genRegMask(REG_ARG_LAST)); noway_assert(cond); - noway_assert(sizeofPreSpillRegArgs <= - argOffs + TARGET_POINTER_SIZE); // at most one register of alignment + noway_assert(sizeofPreSpillRegArgs <= argOffs + TARGET_POINTER_SIZE); // at most one register of + // alignment } argOffs = sizeofPreSpillRegArgs; } @@ -5707,8 +5707,8 @@ int Compiler::lvaAssignVirtualFrameOffsetToArg(unsigned lclNum, if ((varDsc->TypeGet() == TYP_LONG) && varDsc->lvPromoted) { noway_assert(varDsc->lvFieldCnt == 2); - fieldVarNum = varDsc->lvFieldLclStart; - lvaTable[fieldVarNum].lvStkOffs = varDsc->lvStkOffs; + fieldVarNum = varDsc->lvFieldLclStart; + lvaTable[fieldVarNum].lvStkOffs = varDsc->lvStkOffs; lvaTable[fieldVarNum + 1].lvStkOffs = varDsc->lvStkOffs + genTypeSize(TYP_INT); } else @@ -5836,8 +5836,8 @@ void Compiler::lvaAssignVirtualFrameOffsetsToLocals() stkOffs -= initialStkOffs; } - if (codeGen->IsSaveFpLrWithAllCalleeSavedRegisters() || - !isFramePointerUsed()) // Note that currently we always have a frame pointer + if (codeGen->IsSaveFpLrWithAllCalleeSavedRegisters() || !isFramePointerUsed()) // Note that currently we always have + // a frame pointer { stkOffs -= compCalleeRegsPushed * REGSIZE_BYTES; } @@ -5931,10 +5931,6 @@ void Compiler::lvaAssignVirtualFrameOffsetsToLocals() // Allocate a pointer sized stack slot, since we may need to double align here // when lvaDoneFrameLayout == FINAL_FRAME_LAYOUT // - if (compMethodID == 39739) - { - printf("lvaAssignVirtualFrameOffsetsToLocal1 : %u\n", compLclFrameSize); - } lvaIncrementFrameSize(TARGET_POINTER_SIZE); stkOffs -= TARGET_POINTER_SIZE; @@ -5945,10 +5941,6 @@ void Compiler::lvaAssignVirtualFrameOffsetsToLocals() // offsets that we calculate for the stack frame will always // be greater (or equal) to what they can be in the final layout. // - if (compMethodID == 39739) - { - printf("lvaAssignVirtualFrameOffsetsToLocal2 : %u\n", compLclFrameSize); - } lvaIncrementFrameSize(TARGET_POINTER_SIZE); stkOffs -= TARGET_POINTER_SIZE; } @@ -5956,10 +5948,6 @@ void Compiler::lvaAssignVirtualFrameOffsetsToLocals() { if (((stkOffs + preSpillSize) % (2 * TARGET_POINTER_SIZE)) != 0) { - if (compMethodID == 39739) - { - printf("lvaAssignVirtualFrameOffsetsToLocal3 : %u\n", compLclFrameSize); - } lvaIncrementFrameSize(TARGET_POINTER_SIZE); stkOffs -= TARGET_POINTER_SIZE; } @@ -6002,10 +5990,6 @@ void Compiler::lvaAssignVirtualFrameOffsetsToLocals() noway_assert(codeGen->isFramePointerUsed()); #endif // For CORINFO_CALLCONV_PARAMTYPE (if needed) - if (compMethodID == 39739) - { - printf("lvaAssignVirtualFrameOffsetsToLocal4 : %u\n", compLclFrameSize); - } lvaIncrementFrameSize(TARGET_POINTER_SIZE); stkOffs -= TARGET_POINTER_SIZE; lvaCachedGenericContextArgOffs = stkOffs; @@ -6014,10 +5998,6 @@ void Compiler::lvaAssignVirtualFrameOffsetsToLocals() else if (lvaKeepAliveAndReportThis()) { // When "this" is also used as generic context arg. - if (compMethodID == 39739) - { - printf("lvaAssignVirtualFrameOffsetsToLocal5 : %u\n", compLclFrameSize); - } lvaIncrementFrameSize(TARGET_POINTER_SIZE); stkOffs -= TARGET_POINTER_SIZE; lvaCachedGenericContextArgOffs = stkOffs; @@ -6366,10 +6346,6 @@ void Compiler::lvaAssignVirtualFrameOffsetsToLocals() // stack slot since we may need to double align this LclVar // when lvaDoneFrameLayout == FINAL_FRAME_LAYOUT // - if (compMethodID == 39739) - { - printf("lvaAssignVirtualFrameOffsetsToLocal6 : %u\n", compLclFrameSize); - } lvaIncrementFrameSize(TARGET_POINTER_SIZE); stkOffs -= TARGET_POINTER_SIZE; } @@ -6377,10 +6353,6 @@ void Compiler::lvaAssignVirtualFrameOffsetsToLocals() { if (((stkOffs + preSpillSize) % (2 * TARGET_POINTER_SIZE)) != 0) { - if (compMethodID == 39739) - { - printf("lvaAssignVirtualFrameOffsetsToLocal7 : %u\n", compLclFrameSize); - } lvaIncrementFrameSize(TARGET_POINTER_SIZE); stkOffs -= TARGET_POINTER_SIZE; } @@ -6476,10 +6448,6 @@ void Compiler::lvaAssignVirtualFrameOffsetsToLocals() // Allocate a pointer sized stack slot, since we may need to double align here // when lvaDoneFrameLayout == FINAL_FRAME_LAYOUT // - if (compMethodID == 39739) - { - printf("lvaAssignVirtualFrameOffsetsToLocal8 : %u\n", compLclFrameSize); - } lvaIncrementFrameSize(TARGET_POINTER_SIZE); stkOffs -= TARGET_POINTER_SIZE; @@ -6492,10 +6460,6 @@ void Compiler::lvaAssignVirtualFrameOffsetsToLocals() // calculate for the stack frame are always greater than they will // be in the final layout. // - if (compMethodID == 39739) - { - printf("lvaAssignVirtualFrameOffsetsToLocal9 : %u\n", compLclFrameSize); - } lvaIncrementFrameSize(TARGET_POINTER_SIZE); stkOffs -= TARGET_POINTER_SIZE; } @@ -6504,10 +6468,6 @@ void Compiler::lvaAssignVirtualFrameOffsetsToLocals() { if (((stkOffs + preSpillSize) % (2 * TARGET_POINTER_SIZE)) != 0) { - if (compMethodID == 39739) - { - printf("lvaAssignVirtualFrameOffsetsToLocal10 : %u\n", compLclFrameSize); - } lvaIncrementFrameSize(TARGET_POINTER_SIZE); stkOffs -= TARGET_POINTER_SIZE; } @@ -6528,8 +6488,8 @@ void Compiler::lvaAssignVirtualFrameOffsetsToLocals() #endif // FEATURE_EH_FUNCLETS && defined(TARGET_AMD64) #ifdef TARGET_ARM64 - if (!codeGen->IsSaveFpLrWithAllCalleeSavedRegisters() && - isFramePointerUsed()) // Note that currently we always have a frame pointer + if (!codeGen->IsSaveFpLrWithAllCalleeSavedRegisters() && isFramePointerUsed()) // Note that currently we always have + // a frame pointer { // Create space for saving FP and LR. stkOffs -= 2 * REGSIZE_BYTES; @@ -6655,10 +6615,6 @@ int Compiler::lvaAllocLocalAndSetVirtualOffset(unsigned lclNum, unsigned size, i /* Reserve space on the stack by bumping the frame size */ - if (compMethodID == 39739) - { - printf("lvaAllocLocalAndSetVirtualOffset : %u\n", compLclFrameSize); - } lvaIncrementFrameSize(size); stkOffs -= size; lvaTable[lclNum].lvStkOffs = stkOffs; @@ -6764,7 +6720,7 @@ void Compiler::lvaAlignFrame() // Ensure that the stack is always 16-byte aligned by grabbing an unused QWORD // if needed. bool regPushedCountAligned = (compCalleeRegsPushed % (16 / REGSIZE_BYTES)) == 0; - bool lclFrameSizeAligned = (compLclFrameSize % 16) == 0; + bool lclFrameSizeAligned = (compLclFrameSize % 16) == 0; // If this isn't the final frame layout, assume we have to push an extra QWORD // Just so the offsets are true upper limits. @@ -6777,16 +6733,12 @@ void Compiler::lvaAlignFrame() // Ensure that stack offsets will be double-aligned by grabbing an unused DWORD if needed. // - bool lclFrameSizeAligned = (compLclFrameSize % sizeof(double)) == 0; + bool lclFrameSizeAligned = (compLclFrameSize % sizeof(double)) == 0; bool regPushedCountAligned = ((compCalleeRegsPushed + genCountBits(codeGen->regSet.rsMaskPreSpillRegs(true))) % (sizeof(double) / TARGET_POINTER_SIZE)) == 0; if (regPushedCountAligned != lclFrameSizeAligned) { - if (compMethodID == 39739) - { - printf("lvaAlignFrame : %u\n", compLclFrameSize); - } lvaIncrementFrameSize(TARGET_POINTER_SIZE); } @@ -6863,7 +6815,7 @@ void Compiler::lvaAssignFrameOffsetsToPromotedStructs() && !varDsc->lvIsParam #endif // !defined(TARGET_ARM) #endif // !UNIX_AMD64_ABI - ) + ) { LclVarDsc* parentvarDsc = &lvaTable[varDsc->lvParentLcl]; lvaPromotionType promotionType = lvaGetPromotionType(parentvarDsc); @@ -6945,10 +6897,6 @@ int Compiler::lvaAllocateTemps(int stkOffs, bool mustDoubleAlign) if (((stkOffs + preSpillSize) % (2 * TARGET_POINTER_SIZE)) != 0) { spillTempSize += TARGET_POINTER_SIZE; - if (compMethodID == 39739) - { - printf("lvaAllocateTemp1 : %u\n", compLclFrameSize); - } lvaIncrementFrameSize(TARGET_POINTER_SIZE); stkOffs -= TARGET_POINTER_SIZE; } @@ -6957,10 +6905,6 @@ int Compiler::lvaAllocateTemps(int stkOffs, bool mustDoubleAlign) } spillTempSize += size; - if (compMethodID == 39739) - { - printf("lvaAllocateTemp2 : %u\n", compLclFrameSize); - } lvaIncrementFrameSize(size); stkOffs -= size; temp->tdSetTempOffs(stkOffs); @@ -6974,10 +6918,6 @@ int Compiler::lvaAllocateTemps(int stkOffs, bool mustDoubleAlign) { unsigned size = lvaGetMaxSpillTempSize(); - if (compMethodID == 39739) - { - printf("lvaAllocateTemp3 : %u\n", compLclFrameSize); - } lvaIncrementFrameSize(size); stkOffs -= size; } @@ -7026,7 +6966,7 @@ void Compiler::lvaDumpFrameLocation(unsigned lclNum) offset = lvaFrameAddress(lclNum, compLocallocUsed, &baseReg, 0, /* isFloatUsage */ false); #else bool EBPbased; - offset = lvaFrameAddress(lclNum, &EBPbased); + offset = lvaFrameAddress(lclNum, &EBPbased); baseReg = EBPbased ? REG_FPBASE : REG_SPBASE; #endif @@ -7287,9 +7227,9 @@ void Compiler::lvaDumpEntry(unsigned lclNum, FrameLayoutState curState, size_t r } /***************************************************************************** -* -* dump the lvaTable -*/ + * + * dump the lvaTable + */ void Compiler::lvaTableDump(FrameLayoutState curState) { From 3b1c038f7afdc5ff98680ec7e3ff81ce843ba9f0 Mon Sep 17 00:00:00 2001 From: Kunal Pathak Date: Mon, 6 Jul 2020 16:32:04 -0700 Subject: [PATCH 04/16] Revert "revert lclvars.cpp changes" This reverts commit d39af7084687e8fe5e6d4f71674ec84d36a88340. --- src/coreclr/src/jit/lclvars.cpp | 148 ++++++++++++++++++++++---------- 1 file changed, 104 insertions(+), 44 deletions(-) diff --git a/src/coreclr/src/jit/lclvars.cpp b/src/coreclr/src/jit/lclvars.cpp index 184b19ff46531a..01cc83271b2867 100644 --- a/src/coreclr/src/jit/lclvars.cpp +++ b/src/coreclr/src/jit/lclvars.cpp @@ -292,9 +292,9 @@ void Compiler::lvaInitTypeRef() } if ( // If there already exist unsafe buffers, don't mark more structs as unsafe - // as that will cause them to be placed along with the real unsafe buffers, - // unnecessarily exposing them to overruns. This can affect GS tests which - // intentionally do buffer-overruns. + // as that will cause them to be placed along with the real unsafe buffers, + // unnecessarily exposing them to overruns. This can affect GS tests which + // intentionally do buffer-overruns. !getNeedsGSSecurityCookie() && // GS checks require the stack to be re-ordered, which can't be done with EnC !opts.compDbgEnC && compStressCompile(STRESS_UNSAFE_BUFFER_CHECKS, 25)) @@ -352,7 +352,7 @@ void Compiler::lvaInitArgs(InitVarDscInfo* varDscInfo) /* If we have a hidden return-buffer parameter, that comes here */ lvaInitRetBuffArg(varDscInfo); - //====================================================================== +//====================================================================== #if USER_ARGS_COME_LAST //@GENERICS: final instantiation-info argument for shared generic methods @@ -559,9 +559,9 @@ void Compiler::lvaInitRetBuffArg(InitVarDscInfo* varDscInfo) /*****************************************************************************/ void Compiler::lvaInitUserArgs(InitVarDscInfo* varDscInfo) { - //------------------------------------------------------------------------- - // Walk the function signature for the explicit arguments - //------------------------------------------------------------------------- +//------------------------------------------------------------------------- +// Walk the function signature for the explicit arguments +//------------------------------------------------------------------------- #if defined(TARGET_X86) // Only (some of) the implicit args are enregistered for varargs @@ -1033,8 +1033,8 @@ void Compiler::lvaInitUserArgs(InitVarDscInfo* varDscInfo) #if defined(TARGET_X86) varDsc->lvStkOffs = compArgSize; #else // !TARGET_X86 - // TODO-CQ: We shouldn't have to go as far as to declare these - // address-exposed -- DoNotEnregister should suffice. + // TODO-CQ: We shouldn't have to go as far as to declare these + // address-exposed -- DoNotEnregister should suffice. lvaSetVarAddrExposed(varDscInfo->varNum); #endif // !TARGET_X86 } @@ -1313,7 +1313,7 @@ void Compiler::lvaInitVarDsc(LclVarDsc* varDsc, varDsc->lvIsImplicitByRef = 0; #endif // defined(TARGET_AMD64) || defined(TARGET_ARM64) - // Set the lvType (before this point it is TYP_UNDEF). +// Set the lvType (before this point it is TYP_UNDEF). #ifdef FEATURE_HFA varDsc->SetHfaType(TYP_UNDEF); @@ -2198,7 +2198,7 @@ void Compiler::StructPromotionHelper::PromoteStructVar(unsigned lclNum) compiler->compFloatingPointUsed = true; } - // Now grab the temp for the field local. +// Now grab the temp for the field local. #ifdef DEBUG char buf[200]; @@ -3394,8 +3394,8 @@ void Compiler::lvaSortByRefCount() if (varDsc->lvAddrExposed) { varDsc->lvTracked = 0; - assert(varDsc->lvType != TYP_STRUCT || varDsc->lvDoNotEnregister); // For structs, should have set this when - // we set lvAddrExposed. + assert(varDsc->lvType != TYP_STRUCT || + varDsc->lvDoNotEnregister); // For structs, should have set this when we set lvAddrExposed. } else if (varTypeIsStruct(varDsc)) { @@ -3549,8 +3549,8 @@ void LclVarDsc::lvaDisqualifyVar() #endif // ASSERTION_PROP /********************************************************************************** - * Get stack size of the varDsc. - */ +* Get stack size of the varDsc. +*/ size_t LclVarDsc::lvArgStackSize() const { // Make sure this will have a stack size @@ -3591,8 +3591,8 @@ size_t LclVarDsc::lvArgStackSize() const } /********************************************************************************** - * Get type of a variable when passed as an argument. - */ +* Get type of a variable when passed as an argument. +*/ var_types LclVarDsc::lvaArgType() { var_types type = TypeGet(); @@ -4368,11 +4368,11 @@ inline void Compiler::lvaIncrementFrameSize(unsigned size) } /**************************************************************************** - * - * Return true if absolute offsets of temps are larger than vars, or in other - * words, did we allocate temps before of after vars. The /GS buffer overrun - * checks want temps to be at low stack addresses than buffers - */ +* +* Return true if absolute offsets of temps are larger than vars, or in other +* words, did we allocate temps before of after vars. The /GS buffer overrun +* checks want temps to be at low stack addresses than buffers +*/ bool Compiler::lvaTempsHaveLargerOffsetThanVars() { #ifdef TARGET_ARM @@ -4391,10 +4391,10 @@ bool Compiler::lvaTempsHaveLargerOffsetThanVars() } /**************************************************************************** - * - * Return an upper bound estimate for the size of the compiler spill temps - * - */ +* +* Return an upper bound estimate for the size of the compiler spill temps +* +*/ unsigned Compiler::lvaGetMaxSpillTempSize() { unsigned result = 0; @@ -5002,7 +5002,7 @@ void Compiler::lvaFixVirtualFrameOffsets() #endif ) #endif // !defined(TARGET_AMD64) - ) + ) { doAssignStkOffs = false; // Not on frame or an incomming stack arg } @@ -5311,8 +5311,8 @@ void Compiler::lvaAssignVirtualFrameOffsetsToArgs() // ret address slot, stack frame padding, alloca instructions, etc. // Note: This is the implementation for UNIX_AMD64 System V platforms. // -int Compiler::lvaAssignVirtualFrameOffsetToArg(unsigned lclNum, - unsigned argSize, +int Compiler::lvaAssignVirtualFrameOffsetToArg(unsigned lclNum, + unsigned argSize, int argOffs UNIX_AMD64_ABI_ONLY_ARG(int* callerArgOffset)) { noway_assert(lclNum < info.compArgsCount); @@ -5665,8 +5665,8 @@ int Compiler::lvaAssignVirtualFrameOffsetToArg(unsigned lclNum, (codeGen->regSet.rsMaskPreSpillAlign & genRegMask(REG_ARG_LAST)); noway_assert(cond); - noway_assert(sizeofPreSpillRegArgs <= argOffs + TARGET_POINTER_SIZE); // at most one register of - // alignment + noway_assert(sizeofPreSpillRegArgs <= + argOffs + TARGET_POINTER_SIZE); // at most one register of alignment } argOffs = sizeofPreSpillRegArgs; } @@ -5707,8 +5707,8 @@ int Compiler::lvaAssignVirtualFrameOffsetToArg(unsigned lclNum, if ((varDsc->TypeGet() == TYP_LONG) && varDsc->lvPromoted) { noway_assert(varDsc->lvFieldCnt == 2); - fieldVarNum = varDsc->lvFieldLclStart; - lvaTable[fieldVarNum].lvStkOffs = varDsc->lvStkOffs; + fieldVarNum = varDsc->lvFieldLclStart; + lvaTable[fieldVarNum].lvStkOffs = varDsc->lvStkOffs; lvaTable[fieldVarNum + 1].lvStkOffs = varDsc->lvStkOffs + genTypeSize(TYP_INT); } else @@ -5836,8 +5836,8 @@ void Compiler::lvaAssignVirtualFrameOffsetsToLocals() stkOffs -= initialStkOffs; } - if (codeGen->IsSaveFpLrWithAllCalleeSavedRegisters() || !isFramePointerUsed()) // Note that currently we always have - // a frame pointer + if (codeGen->IsSaveFpLrWithAllCalleeSavedRegisters() || + !isFramePointerUsed()) // Note that currently we always have a frame pointer { stkOffs -= compCalleeRegsPushed * REGSIZE_BYTES; } @@ -5931,6 +5931,10 @@ void Compiler::lvaAssignVirtualFrameOffsetsToLocals() // Allocate a pointer sized stack slot, since we may need to double align here // when lvaDoneFrameLayout == FINAL_FRAME_LAYOUT // + if (compMethodID == 39739) + { + printf("lvaAssignVirtualFrameOffsetsToLocal1 : %u\n", compLclFrameSize); + } lvaIncrementFrameSize(TARGET_POINTER_SIZE); stkOffs -= TARGET_POINTER_SIZE; @@ -5941,6 +5945,10 @@ void Compiler::lvaAssignVirtualFrameOffsetsToLocals() // offsets that we calculate for the stack frame will always // be greater (or equal) to what they can be in the final layout. // + if (compMethodID == 39739) + { + printf("lvaAssignVirtualFrameOffsetsToLocal2 : %u\n", compLclFrameSize); + } lvaIncrementFrameSize(TARGET_POINTER_SIZE); stkOffs -= TARGET_POINTER_SIZE; } @@ -5948,6 +5956,10 @@ void Compiler::lvaAssignVirtualFrameOffsetsToLocals() { if (((stkOffs + preSpillSize) % (2 * TARGET_POINTER_SIZE)) != 0) { + if (compMethodID == 39739) + { + printf("lvaAssignVirtualFrameOffsetsToLocal3 : %u\n", compLclFrameSize); + } lvaIncrementFrameSize(TARGET_POINTER_SIZE); stkOffs -= TARGET_POINTER_SIZE; } @@ -5990,6 +6002,10 @@ void Compiler::lvaAssignVirtualFrameOffsetsToLocals() noway_assert(codeGen->isFramePointerUsed()); #endif // For CORINFO_CALLCONV_PARAMTYPE (if needed) + if (compMethodID == 39739) + { + printf("lvaAssignVirtualFrameOffsetsToLocal4 : %u\n", compLclFrameSize); + } lvaIncrementFrameSize(TARGET_POINTER_SIZE); stkOffs -= TARGET_POINTER_SIZE; lvaCachedGenericContextArgOffs = stkOffs; @@ -5998,6 +6014,10 @@ void Compiler::lvaAssignVirtualFrameOffsetsToLocals() else if (lvaKeepAliveAndReportThis()) { // When "this" is also used as generic context arg. + if (compMethodID == 39739) + { + printf("lvaAssignVirtualFrameOffsetsToLocal5 : %u\n", compLclFrameSize); + } lvaIncrementFrameSize(TARGET_POINTER_SIZE); stkOffs -= TARGET_POINTER_SIZE; lvaCachedGenericContextArgOffs = stkOffs; @@ -6346,6 +6366,10 @@ void Compiler::lvaAssignVirtualFrameOffsetsToLocals() // stack slot since we may need to double align this LclVar // when lvaDoneFrameLayout == FINAL_FRAME_LAYOUT // + if (compMethodID == 39739) + { + printf("lvaAssignVirtualFrameOffsetsToLocal6 : %u\n", compLclFrameSize); + } lvaIncrementFrameSize(TARGET_POINTER_SIZE); stkOffs -= TARGET_POINTER_SIZE; } @@ -6353,6 +6377,10 @@ void Compiler::lvaAssignVirtualFrameOffsetsToLocals() { if (((stkOffs + preSpillSize) % (2 * TARGET_POINTER_SIZE)) != 0) { + if (compMethodID == 39739) + { + printf("lvaAssignVirtualFrameOffsetsToLocal7 : %u\n", compLclFrameSize); + } lvaIncrementFrameSize(TARGET_POINTER_SIZE); stkOffs -= TARGET_POINTER_SIZE; } @@ -6448,6 +6476,10 @@ void Compiler::lvaAssignVirtualFrameOffsetsToLocals() // Allocate a pointer sized stack slot, since we may need to double align here // when lvaDoneFrameLayout == FINAL_FRAME_LAYOUT // + if (compMethodID == 39739) + { + printf("lvaAssignVirtualFrameOffsetsToLocal8 : %u\n", compLclFrameSize); + } lvaIncrementFrameSize(TARGET_POINTER_SIZE); stkOffs -= TARGET_POINTER_SIZE; @@ -6460,6 +6492,10 @@ void Compiler::lvaAssignVirtualFrameOffsetsToLocals() // calculate for the stack frame are always greater than they will // be in the final layout. // + if (compMethodID == 39739) + { + printf("lvaAssignVirtualFrameOffsetsToLocal9 : %u\n", compLclFrameSize); + } lvaIncrementFrameSize(TARGET_POINTER_SIZE); stkOffs -= TARGET_POINTER_SIZE; } @@ -6468,6 +6504,10 @@ void Compiler::lvaAssignVirtualFrameOffsetsToLocals() { if (((stkOffs + preSpillSize) % (2 * TARGET_POINTER_SIZE)) != 0) { + if (compMethodID == 39739) + { + printf("lvaAssignVirtualFrameOffsetsToLocal10 : %u\n", compLclFrameSize); + } lvaIncrementFrameSize(TARGET_POINTER_SIZE); stkOffs -= TARGET_POINTER_SIZE; } @@ -6488,8 +6528,8 @@ void Compiler::lvaAssignVirtualFrameOffsetsToLocals() #endif // FEATURE_EH_FUNCLETS && defined(TARGET_AMD64) #ifdef TARGET_ARM64 - if (!codeGen->IsSaveFpLrWithAllCalleeSavedRegisters() && isFramePointerUsed()) // Note that currently we always have - // a frame pointer + if (!codeGen->IsSaveFpLrWithAllCalleeSavedRegisters() && + isFramePointerUsed()) // Note that currently we always have a frame pointer { // Create space for saving FP and LR. stkOffs -= 2 * REGSIZE_BYTES; @@ -6615,6 +6655,10 @@ int Compiler::lvaAllocLocalAndSetVirtualOffset(unsigned lclNum, unsigned size, i /* Reserve space on the stack by bumping the frame size */ + if (compMethodID == 39739) + { + printf("lvaAllocLocalAndSetVirtualOffset : %u\n", compLclFrameSize); + } lvaIncrementFrameSize(size); stkOffs -= size; lvaTable[lclNum].lvStkOffs = stkOffs; @@ -6720,7 +6764,7 @@ void Compiler::lvaAlignFrame() // Ensure that the stack is always 16-byte aligned by grabbing an unused QWORD // if needed. bool regPushedCountAligned = (compCalleeRegsPushed % (16 / REGSIZE_BYTES)) == 0; - bool lclFrameSizeAligned = (compLclFrameSize % 16) == 0; + bool lclFrameSizeAligned = (compLclFrameSize % 16) == 0; // If this isn't the final frame layout, assume we have to push an extra QWORD // Just so the offsets are true upper limits. @@ -6733,12 +6777,16 @@ void Compiler::lvaAlignFrame() // Ensure that stack offsets will be double-aligned by grabbing an unused DWORD if needed. // - bool lclFrameSizeAligned = (compLclFrameSize % sizeof(double)) == 0; + bool lclFrameSizeAligned = (compLclFrameSize % sizeof(double)) == 0; bool regPushedCountAligned = ((compCalleeRegsPushed + genCountBits(codeGen->regSet.rsMaskPreSpillRegs(true))) % (sizeof(double) / TARGET_POINTER_SIZE)) == 0; if (regPushedCountAligned != lclFrameSizeAligned) { + if (compMethodID == 39739) + { + printf("lvaAlignFrame : %u\n", compLclFrameSize); + } lvaIncrementFrameSize(TARGET_POINTER_SIZE); } @@ -6815,7 +6863,7 @@ void Compiler::lvaAssignFrameOffsetsToPromotedStructs() && !varDsc->lvIsParam #endif // !defined(TARGET_ARM) #endif // !UNIX_AMD64_ABI - ) + ) { LclVarDsc* parentvarDsc = &lvaTable[varDsc->lvParentLcl]; lvaPromotionType promotionType = lvaGetPromotionType(parentvarDsc); @@ -6897,6 +6945,10 @@ int Compiler::lvaAllocateTemps(int stkOffs, bool mustDoubleAlign) if (((stkOffs + preSpillSize) % (2 * TARGET_POINTER_SIZE)) != 0) { spillTempSize += TARGET_POINTER_SIZE; + if (compMethodID == 39739) + { + printf("lvaAllocateTemp1 : %u\n", compLclFrameSize); + } lvaIncrementFrameSize(TARGET_POINTER_SIZE); stkOffs -= TARGET_POINTER_SIZE; } @@ -6905,6 +6957,10 @@ int Compiler::lvaAllocateTemps(int stkOffs, bool mustDoubleAlign) } spillTempSize += size; + if (compMethodID == 39739) + { + printf("lvaAllocateTemp2 : %u\n", compLclFrameSize); + } lvaIncrementFrameSize(size); stkOffs -= size; temp->tdSetTempOffs(stkOffs); @@ -6918,6 +6974,10 @@ int Compiler::lvaAllocateTemps(int stkOffs, bool mustDoubleAlign) { unsigned size = lvaGetMaxSpillTempSize(); + if (compMethodID == 39739) + { + printf("lvaAllocateTemp3 : %u\n", compLclFrameSize); + } lvaIncrementFrameSize(size); stkOffs -= size; } @@ -6966,7 +7026,7 @@ void Compiler::lvaDumpFrameLocation(unsigned lclNum) offset = lvaFrameAddress(lclNum, compLocallocUsed, &baseReg, 0, /* isFloatUsage */ false); #else bool EBPbased; - offset = lvaFrameAddress(lclNum, &EBPbased); + offset = lvaFrameAddress(lclNum, &EBPbased); baseReg = EBPbased ? REG_FPBASE : REG_SPBASE; #endif @@ -7227,9 +7287,9 @@ void Compiler::lvaDumpEntry(unsigned lclNum, FrameLayoutState curState, size_t r } /***************************************************************************** - * - * dump the lvaTable - */ +* +* dump the lvaTable +*/ void Compiler::lvaTableDump(FrameLayoutState curState) { From 73c986dcb9f2d3bdef13feb88fc72d2aa7ea8500 Mon Sep 17 00:00:00 2001 From: Kunal Pathak Date: Mon, 6 Jul 2020 16:32:47 -0700 Subject: [PATCH 05/16] wip --- src/coreclr/src/jit/codegen.h | 2 -- src/coreclr/src/jit/emit.cpp | 4 ---- 2 files changed, 6 deletions(-) diff --git a/src/coreclr/src/jit/codegen.h b/src/coreclr/src/jit/codegen.h index ed2c8a9efcc45d..653cfa2bfbaeaf 100644 --- a/src/coreclr/src/jit/codegen.h +++ b/src/coreclr/src/jit/codegen.h @@ -1406,8 +1406,6 @@ XXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXX void inst_SA_RV(instruction ins, unsigned ofs, regNumber reg, var_types type); void inst_SA_IV(instruction ins, unsigned ofs, int val, var_types type); - void inst_RV_ST( - instruction ins, regNumber reg, TempDsc* tmp, unsigned ofs, var_types type, emitAttr size = EA_UNKNOWN); // delete this method void inst_FS_ST(instruction ins, emitAttr size, TempDsc* tmp, unsigned ofs); void inst_TT(instruction ins, GenTree* tree, unsigned offs = 0, int shfv = 0, emitAttr size = EA_UNKNOWN); diff --git a/src/coreclr/src/jit/emit.cpp b/src/coreclr/src/jit/emit.cpp index 69b715e55820b3..2613cd453e61a5 100644 --- a/src/coreclr/src/jit/emit.cpp +++ b/src/coreclr/src/jit/emit.cpp @@ -4920,10 +4920,6 @@ unsigned emitter::emitEndCodeGen(Compiler* comp, else { printf("\nG_M%03u_IG%02u:\n", emitComp->compMethodID, ig->igNum); - if (ig->igNum == 1720) - { - printf("\n--------debug break--------\n"); - } } } #endif // DEBUG From 58d2bba62fade1230f2fbd72c3e82f0749ef6c3d Mon Sep 17 00:00:00 2001 From: Kunal Pathak Date: Mon, 6 Jul 2020 16:33:45 -0700 Subject: [PATCH 06/16] revert lclvars.cpp changes --- src/coreclr/src/jit/lclvars.cpp | 148 ++++++++++---------------------- 1 file changed, 44 insertions(+), 104 deletions(-) diff --git a/src/coreclr/src/jit/lclvars.cpp b/src/coreclr/src/jit/lclvars.cpp index 01cc83271b2867..184b19ff46531a 100644 --- a/src/coreclr/src/jit/lclvars.cpp +++ b/src/coreclr/src/jit/lclvars.cpp @@ -292,9 +292,9 @@ void Compiler::lvaInitTypeRef() } if ( // If there already exist unsafe buffers, don't mark more structs as unsafe - // as that will cause them to be placed along with the real unsafe buffers, - // unnecessarily exposing them to overruns. This can affect GS tests which - // intentionally do buffer-overruns. + // as that will cause them to be placed along with the real unsafe buffers, + // unnecessarily exposing them to overruns. This can affect GS tests which + // intentionally do buffer-overruns. !getNeedsGSSecurityCookie() && // GS checks require the stack to be re-ordered, which can't be done with EnC !opts.compDbgEnC && compStressCompile(STRESS_UNSAFE_BUFFER_CHECKS, 25)) @@ -352,7 +352,7 @@ void Compiler::lvaInitArgs(InitVarDscInfo* varDscInfo) /* If we have a hidden return-buffer parameter, that comes here */ lvaInitRetBuffArg(varDscInfo); -//====================================================================== + //====================================================================== #if USER_ARGS_COME_LAST //@GENERICS: final instantiation-info argument for shared generic methods @@ -559,9 +559,9 @@ void Compiler::lvaInitRetBuffArg(InitVarDscInfo* varDscInfo) /*****************************************************************************/ void Compiler::lvaInitUserArgs(InitVarDscInfo* varDscInfo) { -//------------------------------------------------------------------------- -// Walk the function signature for the explicit arguments -//------------------------------------------------------------------------- + //------------------------------------------------------------------------- + // Walk the function signature for the explicit arguments + //------------------------------------------------------------------------- #if defined(TARGET_X86) // Only (some of) the implicit args are enregistered for varargs @@ -1033,8 +1033,8 @@ void Compiler::lvaInitUserArgs(InitVarDscInfo* varDscInfo) #if defined(TARGET_X86) varDsc->lvStkOffs = compArgSize; #else // !TARGET_X86 - // TODO-CQ: We shouldn't have to go as far as to declare these - // address-exposed -- DoNotEnregister should suffice. + // TODO-CQ: We shouldn't have to go as far as to declare these + // address-exposed -- DoNotEnregister should suffice. lvaSetVarAddrExposed(varDscInfo->varNum); #endif // !TARGET_X86 } @@ -1313,7 +1313,7 @@ void Compiler::lvaInitVarDsc(LclVarDsc* varDsc, varDsc->lvIsImplicitByRef = 0; #endif // defined(TARGET_AMD64) || defined(TARGET_ARM64) -// Set the lvType (before this point it is TYP_UNDEF). + // Set the lvType (before this point it is TYP_UNDEF). #ifdef FEATURE_HFA varDsc->SetHfaType(TYP_UNDEF); @@ -2198,7 +2198,7 @@ void Compiler::StructPromotionHelper::PromoteStructVar(unsigned lclNum) compiler->compFloatingPointUsed = true; } -// Now grab the temp for the field local. + // Now grab the temp for the field local. #ifdef DEBUG char buf[200]; @@ -3394,8 +3394,8 @@ void Compiler::lvaSortByRefCount() if (varDsc->lvAddrExposed) { varDsc->lvTracked = 0; - assert(varDsc->lvType != TYP_STRUCT || - varDsc->lvDoNotEnregister); // For structs, should have set this when we set lvAddrExposed. + assert(varDsc->lvType != TYP_STRUCT || varDsc->lvDoNotEnregister); // For structs, should have set this when + // we set lvAddrExposed. } else if (varTypeIsStruct(varDsc)) { @@ -3549,8 +3549,8 @@ void LclVarDsc::lvaDisqualifyVar() #endif // ASSERTION_PROP /********************************************************************************** -* Get stack size of the varDsc. -*/ + * Get stack size of the varDsc. + */ size_t LclVarDsc::lvArgStackSize() const { // Make sure this will have a stack size @@ -3591,8 +3591,8 @@ size_t LclVarDsc::lvArgStackSize() const } /********************************************************************************** -* Get type of a variable when passed as an argument. -*/ + * Get type of a variable when passed as an argument. + */ var_types LclVarDsc::lvaArgType() { var_types type = TypeGet(); @@ -4368,11 +4368,11 @@ inline void Compiler::lvaIncrementFrameSize(unsigned size) } /**************************************************************************** -* -* Return true if absolute offsets of temps are larger than vars, or in other -* words, did we allocate temps before of after vars. The /GS buffer overrun -* checks want temps to be at low stack addresses than buffers -*/ + * + * Return true if absolute offsets of temps are larger than vars, or in other + * words, did we allocate temps before of after vars. The /GS buffer overrun + * checks want temps to be at low stack addresses than buffers + */ bool Compiler::lvaTempsHaveLargerOffsetThanVars() { #ifdef TARGET_ARM @@ -4391,10 +4391,10 @@ bool Compiler::lvaTempsHaveLargerOffsetThanVars() } /**************************************************************************** -* -* Return an upper bound estimate for the size of the compiler spill temps -* -*/ + * + * Return an upper bound estimate for the size of the compiler spill temps + * + */ unsigned Compiler::lvaGetMaxSpillTempSize() { unsigned result = 0; @@ -5002,7 +5002,7 @@ void Compiler::lvaFixVirtualFrameOffsets() #endif ) #endif // !defined(TARGET_AMD64) - ) + ) { doAssignStkOffs = false; // Not on frame or an incomming stack arg } @@ -5311,8 +5311,8 @@ void Compiler::lvaAssignVirtualFrameOffsetsToArgs() // ret address slot, stack frame padding, alloca instructions, etc. // Note: This is the implementation for UNIX_AMD64 System V platforms. // -int Compiler::lvaAssignVirtualFrameOffsetToArg(unsigned lclNum, - unsigned argSize, +int Compiler::lvaAssignVirtualFrameOffsetToArg(unsigned lclNum, + unsigned argSize, int argOffs UNIX_AMD64_ABI_ONLY_ARG(int* callerArgOffset)) { noway_assert(lclNum < info.compArgsCount); @@ -5665,8 +5665,8 @@ int Compiler::lvaAssignVirtualFrameOffsetToArg(unsigned lclNum, (codeGen->regSet.rsMaskPreSpillAlign & genRegMask(REG_ARG_LAST)); noway_assert(cond); - noway_assert(sizeofPreSpillRegArgs <= - argOffs + TARGET_POINTER_SIZE); // at most one register of alignment + noway_assert(sizeofPreSpillRegArgs <= argOffs + TARGET_POINTER_SIZE); // at most one register of + // alignment } argOffs = sizeofPreSpillRegArgs; } @@ -5707,8 +5707,8 @@ int Compiler::lvaAssignVirtualFrameOffsetToArg(unsigned lclNum, if ((varDsc->TypeGet() == TYP_LONG) && varDsc->lvPromoted) { noway_assert(varDsc->lvFieldCnt == 2); - fieldVarNum = varDsc->lvFieldLclStart; - lvaTable[fieldVarNum].lvStkOffs = varDsc->lvStkOffs; + fieldVarNum = varDsc->lvFieldLclStart; + lvaTable[fieldVarNum].lvStkOffs = varDsc->lvStkOffs; lvaTable[fieldVarNum + 1].lvStkOffs = varDsc->lvStkOffs + genTypeSize(TYP_INT); } else @@ -5836,8 +5836,8 @@ void Compiler::lvaAssignVirtualFrameOffsetsToLocals() stkOffs -= initialStkOffs; } - if (codeGen->IsSaveFpLrWithAllCalleeSavedRegisters() || - !isFramePointerUsed()) // Note that currently we always have a frame pointer + if (codeGen->IsSaveFpLrWithAllCalleeSavedRegisters() || !isFramePointerUsed()) // Note that currently we always have + // a frame pointer { stkOffs -= compCalleeRegsPushed * REGSIZE_BYTES; } @@ -5931,10 +5931,6 @@ void Compiler::lvaAssignVirtualFrameOffsetsToLocals() // Allocate a pointer sized stack slot, since we may need to double align here // when lvaDoneFrameLayout == FINAL_FRAME_LAYOUT // - if (compMethodID == 39739) - { - printf("lvaAssignVirtualFrameOffsetsToLocal1 : %u\n", compLclFrameSize); - } lvaIncrementFrameSize(TARGET_POINTER_SIZE); stkOffs -= TARGET_POINTER_SIZE; @@ -5945,10 +5941,6 @@ void Compiler::lvaAssignVirtualFrameOffsetsToLocals() // offsets that we calculate for the stack frame will always // be greater (or equal) to what they can be in the final layout. // - if (compMethodID == 39739) - { - printf("lvaAssignVirtualFrameOffsetsToLocal2 : %u\n", compLclFrameSize); - } lvaIncrementFrameSize(TARGET_POINTER_SIZE); stkOffs -= TARGET_POINTER_SIZE; } @@ -5956,10 +5948,6 @@ void Compiler::lvaAssignVirtualFrameOffsetsToLocals() { if (((stkOffs + preSpillSize) % (2 * TARGET_POINTER_SIZE)) != 0) { - if (compMethodID == 39739) - { - printf("lvaAssignVirtualFrameOffsetsToLocal3 : %u\n", compLclFrameSize); - } lvaIncrementFrameSize(TARGET_POINTER_SIZE); stkOffs -= TARGET_POINTER_SIZE; } @@ -6002,10 +5990,6 @@ void Compiler::lvaAssignVirtualFrameOffsetsToLocals() noway_assert(codeGen->isFramePointerUsed()); #endif // For CORINFO_CALLCONV_PARAMTYPE (if needed) - if (compMethodID == 39739) - { - printf("lvaAssignVirtualFrameOffsetsToLocal4 : %u\n", compLclFrameSize); - } lvaIncrementFrameSize(TARGET_POINTER_SIZE); stkOffs -= TARGET_POINTER_SIZE; lvaCachedGenericContextArgOffs = stkOffs; @@ -6014,10 +5998,6 @@ void Compiler::lvaAssignVirtualFrameOffsetsToLocals() else if (lvaKeepAliveAndReportThis()) { // When "this" is also used as generic context arg. - if (compMethodID == 39739) - { - printf("lvaAssignVirtualFrameOffsetsToLocal5 : %u\n", compLclFrameSize); - } lvaIncrementFrameSize(TARGET_POINTER_SIZE); stkOffs -= TARGET_POINTER_SIZE; lvaCachedGenericContextArgOffs = stkOffs; @@ -6366,10 +6346,6 @@ void Compiler::lvaAssignVirtualFrameOffsetsToLocals() // stack slot since we may need to double align this LclVar // when lvaDoneFrameLayout == FINAL_FRAME_LAYOUT // - if (compMethodID == 39739) - { - printf("lvaAssignVirtualFrameOffsetsToLocal6 : %u\n", compLclFrameSize); - } lvaIncrementFrameSize(TARGET_POINTER_SIZE); stkOffs -= TARGET_POINTER_SIZE; } @@ -6377,10 +6353,6 @@ void Compiler::lvaAssignVirtualFrameOffsetsToLocals() { if (((stkOffs + preSpillSize) % (2 * TARGET_POINTER_SIZE)) != 0) { - if (compMethodID == 39739) - { - printf("lvaAssignVirtualFrameOffsetsToLocal7 : %u\n", compLclFrameSize); - } lvaIncrementFrameSize(TARGET_POINTER_SIZE); stkOffs -= TARGET_POINTER_SIZE; } @@ -6476,10 +6448,6 @@ void Compiler::lvaAssignVirtualFrameOffsetsToLocals() // Allocate a pointer sized stack slot, since we may need to double align here // when lvaDoneFrameLayout == FINAL_FRAME_LAYOUT // - if (compMethodID == 39739) - { - printf("lvaAssignVirtualFrameOffsetsToLocal8 : %u\n", compLclFrameSize); - } lvaIncrementFrameSize(TARGET_POINTER_SIZE); stkOffs -= TARGET_POINTER_SIZE; @@ -6492,10 +6460,6 @@ void Compiler::lvaAssignVirtualFrameOffsetsToLocals() // calculate for the stack frame are always greater than they will // be in the final layout. // - if (compMethodID == 39739) - { - printf("lvaAssignVirtualFrameOffsetsToLocal9 : %u\n", compLclFrameSize); - } lvaIncrementFrameSize(TARGET_POINTER_SIZE); stkOffs -= TARGET_POINTER_SIZE; } @@ -6504,10 +6468,6 @@ void Compiler::lvaAssignVirtualFrameOffsetsToLocals() { if (((stkOffs + preSpillSize) % (2 * TARGET_POINTER_SIZE)) != 0) { - if (compMethodID == 39739) - { - printf("lvaAssignVirtualFrameOffsetsToLocal10 : %u\n", compLclFrameSize); - } lvaIncrementFrameSize(TARGET_POINTER_SIZE); stkOffs -= TARGET_POINTER_SIZE; } @@ -6528,8 +6488,8 @@ void Compiler::lvaAssignVirtualFrameOffsetsToLocals() #endif // FEATURE_EH_FUNCLETS && defined(TARGET_AMD64) #ifdef TARGET_ARM64 - if (!codeGen->IsSaveFpLrWithAllCalleeSavedRegisters() && - isFramePointerUsed()) // Note that currently we always have a frame pointer + if (!codeGen->IsSaveFpLrWithAllCalleeSavedRegisters() && isFramePointerUsed()) // Note that currently we always have + // a frame pointer { // Create space for saving FP and LR. stkOffs -= 2 * REGSIZE_BYTES; @@ -6655,10 +6615,6 @@ int Compiler::lvaAllocLocalAndSetVirtualOffset(unsigned lclNum, unsigned size, i /* Reserve space on the stack by bumping the frame size */ - if (compMethodID == 39739) - { - printf("lvaAllocLocalAndSetVirtualOffset : %u\n", compLclFrameSize); - } lvaIncrementFrameSize(size); stkOffs -= size; lvaTable[lclNum].lvStkOffs = stkOffs; @@ -6764,7 +6720,7 @@ void Compiler::lvaAlignFrame() // Ensure that the stack is always 16-byte aligned by grabbing an unused QWORD // if needed. bool regPushedCountAligned = (compCalleeRegsPushed % (16 / REGSIZE_BYTES)) == 0; - bool lclFrameSizeAligned = (compLclFrameSize % 16) == 0; + bool lclFrameSizeAligned = (compLclFrameSize % 16) == 0; // If this isn't the final frame layout, assume we have to push an extra QWORD // Just so the offsets are true upper limits. @@ -6777,16 +6733,12 @@ void Compiler::lvaAlignFrame() // Ensure that stack offsets will be double-aligned by grabbing an unused DWORD if needed. // - bool lclFrameSizeAligned = (compLclFrameSize % sizeof(double)) == 0; + bool lclFrameSizeAligned = (compLclFrameSize % sizeof(double)) == 0; bool regPushedCountAligned = ((compCalleeRegsPushed + genCountBits(codeGen->regSet.rsMaskPreSpillRegs(true))) % (sizeof(double) / TARGET_POINTER_SIZE)) == 0; if (regPushedCountAligned != lclFrameSizeAligned) { - if (compMethodID == 39739) - { - printf("lvaAlignFrame : %u\n", compLclFrameSize); - } lvaIncrementFrameSize(TARGET_POINTER_SIZE); } @@ -6863,7 +6815,7 @@ void Compiler::lvaAssignFrameOffsetsToPromotedStructs() && !varDsc->lvIsParam #endif // !defined(TARGET_ARM) #endif // !UNIX_AMD64_ABI - ) + ) { LclVarDsc* parentvarDsc = &lvaTable[varDsc->lvParentLcl]; lvaPromotionType promotionType = lvaGetPromotionType(parentvarDsc); @@ -6945,10 +6897,6 @@ int Compiler::lvaAllocateTemps(int stkOffs, bool mustDoubleAlign) if (((stkOffs + preSpillSize) % (2 * TARGET_POINTER_SIZE)) != 0) { spillTempSize += TARGET_POINTER_SIZE; - if (compMethodID == 39739) - { - printf("lvaAllocateTemp1 : %u\n", compLclFrameSize); - } lvaIncrementFrameSize(TARGET_POINTER_SIZE); stkOffs -= TARGET_POINTER_SIZE; } @@ -6957,10 +6905,6 @@ int Compiler::lvaAllocateTemps(int stkOffs, bool mustDoubleAlign) } spillTempSize += size; - if (compMethodID == 39739) - { - printf("lvaAllocateTemp2 : %u\n", compLclFrameSize); - } lvaIncrementFrameSize(size); stkOffs -= size; temp->tdSetTempOffs(stkOffs); @@ -6974,10 +6918,6 @@ int Compiler::lvaAllocateTemps(int stkOffs, bool mustDoubleAlign) { unsigned size = lvaGetMaxSpillTempSize(); - if (compMethodID == 39739) - { - printf("lvaAllocateTemp3 : %u\n", compLclFrameSize); - } lvaIncrementFrameSize(size); stkOffs -= size; } @@ -7026,7 +6966,7 @@ void Compiler::lvaDumpFrameLocation(unsigned lclNum) offset = lvaFrameAddress(lclNum, compLocallocUsed, &baseReg, 0, /* isFloatUsage */ false); #else bool EBPbased; - offset = lvaFrameAddress(lclNum, &EBPbased); + offset = lvaFrameAddress(lclNum, &EBPbased); baseReg = EBPbased ? REG_FPBASE : REG_SPBASE; #endif @@ -7287,9 +7227,9 @@ void Compiler::lvaDumpEntry(unsigned lclNum, FrameLayoutState curState, size_t r } /***************************************************************************** -* -* dump the lvaTable -*/ + * + * dump the lvaTable + */ void Compiler::lvaTableDump(FrameLayoutState curState) { From fb5226cbdb3b450e06512d7298dc5996164095c1 Mon Sep 17 00:00:00 2001 From: Kunal Pathak Date: Mon, 6 Jul 2020 16:37:24 -0700 Subject: [PATCH 07/16] deleted inst_RV_ST() --- src/coreclr/src/jit/instr.cpp | 34 ---------------------------------- 1 file changed, 34 deletions(-) diff --git a/src/coreclr/src/jit/instr.cpp b/src/coreclr/src/jit/instr.cpp index 7fd2ddf118b20d..364273efddc4aa 100644 --- a/src/coreclr/src/jit/instr.cpp +++ b/src/coreclr/src/jit/instr.cpp @@ -1308,40 +1308,6 @@ void CodeGen::inst_RV_ST(instruction ins, emitAttr size, regNumber reg, GenTree* inst_RV_TT(ins, reg, tree, 0, size); } -void CodeGen::inst_RV_ST(instruction ins, regNumber reg, TempDsc* tmp, unsigned ofs, var_types type, emitAttr size) // delete this method -{ - if (size == EA_UNKNOWN) - { - size = emitActualTypeSize(type); - } - -#ifdef TARGET_ARM - switch (ins) - { - case INS_mov: - assert(!"Please call ins_Load(type) to get the load instruction"); - break; - - case INS_add: - case INS_ldr: - case INS_ldrh: - case INS_ldrb: - case INS_ldrsh: - case INS_ldrsb: - case INS_lea: - case INS_vldr: - GetEmitter()->emitIns_R_S(ins, size, reg, tmp->tdTempNum(), ofs); - break; - - default: - assert(!"Default inst_RV_ST case not supported for Arm"); - break; - } -#else // !TARGET_ARM - GetEmitter()->emitIns_R_S(ins, size, reg, tmp->tdTempNum(), ofs); -#endif // !TARGET_ARM -} - void CodeGen::inst_mov_RV_ST(regNumber reg, GenTree* tree) { /* Figure out the size of the value being loaded */ From 83c18c933148a2a3ce531cd555a2551b334e46fe Mon Sep 17 00:00:00 2001 From: Kunal Pathak Date: Mon, 6 Jul 2020 16:42:40 -0700 Subject: [PATCH 08/16] removing logging, added some asserts --- src/coreclr/src/jit/emitarm.cpp | 78 ++++++++++++++++----------------- 1 file changed, 39 insertions(+), 39 deletions(-) diff --git a/src/coreclr/src/jit/emitarm.cpp b/src/coreclr/src/jit/emitarm.cpp index 3bb2d12b7641ed..def6ac17ebaa44 100644 --- a/src/coreclr/src/jit/emitarm.cpp +++ b/src/coreclr/src/jit/emitarm.cpp @@ -1677,10 +1677,6 @@ void emitter::emitIns_R_I( instruction ins, emitAttr attr, regNumber reg, target_ssize_t imm, insFlags flags /* = INS_FLAGS_DONT_CARE */) { - if (imm == 4088) - { - printf("------debug break inside emitIns_R_I"); - } insFormat fmt = IF_NONE; insFlags sf = INS_FLAGS_DONT_CARE; @@ -3527,7 +3523,6 @@ void emitter::emitIns_R_S(instruction ins, emitAttr attr, regNumber reg1, int va switch (ins) { - case INS_add: // don't need INS_add. no one calls it. case INS_ldr: case INS_ldrh: case INS_ldrb: @@ -3557,10 +3552,10 @@ void emitter::emitIns_R_S(instruction ins, emitAttr attr, regNumber reg1, int va int disp; unsigned undisp; - if (offs == 1208) - { - printf("----debug break - this is it 2B!----"); - } + //if (offs == 1208) + //{ + // printf("----debug break - this is it 2B!----"); + //} base = emitComp->lvaFrameAddress(varx, emitComp->funCurrentFunc()->funKind != FUNC_ROOT, ®2, offs, CodeGen::instIsFP(ins)); @@ -3571,10 +3566,10 @@ void emitter::emitIns_R_S(instruction ins, emitAttr attr, regNumber reg1, int va disp = base + offs; undisp = unsigned_abs(disp); - if (undisp == 0x1000) - { - printf("----debug break - this is it!----"); - } + //if (undisp == 0x1000) + //{ + // printf("----debug break - this is it!----"); + //} if (CodeGen::instIsFP(ins)) { // all fp mem ops take 8 bit immediate, multiplied by 4, plus sign @@ -3624,30 +3619,31 @@ void emitter::emitIns_R_S(instruction ins, emitAttr attr, regNumber reg1, int va } else if (ins == INS_add) // no one calls this case. { - if (isLowRegister(reg1) && (reg2 == REG_SP) && ((disp & 0x03fc) == disp)) - { - fmt = IF_T1_J2; - } - else if (undisp <= 0x0fff) - { - if (disp < 0) - { - ins = INS_sub; - disp = -disp; - } - // add/sub => addw/subw instruction - // Note that even when using the w prefix the immediate is still only 12 bits? - ins = (ins == INS_add) ? INS_addw : INS_subw; - fmt = IF_T2_M0; - } - else - { - // Load disp into a register - regNumber rsvdReg = codeGen->rsGetRsvdReg(); - emitIns_genStackOffset(rsvdReg, varx, offs, /* isFloatUsage */ false); - emitIns_R_R_R(ins, attr, reg1, reg2, rsvdReg); // not sure if here too there could be case where we set the regBase inside emitIns_genStackOffset() but then don't use it further while generating the offset. - return; - } + assert(!"someone is calling emitIns_R_S(INS_add)"); + //if (isLowRegister(reg1) && (reg2 == REG_SP) && ((disp & 0x03fc) == disp)) + //{ + // fmt = IF_T1_J2; + //} + //else if (undisp <= 0x0fff) + //{ + // if (disp < 0) + // { + // ins = INS_sub; + // disp = -disp; + // } + // // add/sub => addw/subw instruction + // // Note that even when using the w prefix the immediate is still only 12 bits? + // ins = (ins == INS_add) ? INS_addw : INS_subw; + // fmt = IF_T2_M0; + //} + //else + //{ + // // Load disp into a register + // regNumber rsvdReg = codeGen->rsGetRsvdReg(); + // emitIns_genStackOffset(rsvdReg, varx, offs, /* isFloatUsage */ false); + // emitIns_R_R_R(ins, attr, reg1, reg2, rsvdReg); // not sure if here too there could be case where we set the regBase inside emitIns_genStackOffset() but then don't use it further while generating the offset. + // return; + //} } else if (ins == INS_movw || ins == INS_movt) { @@ -3762,8 +3758,12 @@ void emitter::emitIns_S_R(instruction ins, emitAttr attr, regNumber reg1, int va else { regNumber rsvdReg = codeGen->rsGetRsvdReg(); - emitIns_genStackOffset(rsvdReg, varx, offs, /* isFloatUsage */ true); - emitIns_R_R(INS_add, EA_4BYTE, rsvdReg, reg2); // not sure if here too there could be case where we set the regBase inside emitIns_genStackOffset() but then don't use it further while generating the offset. + regNumber baseRegUsed; + emitIns_genStackOffset(rsvdReg, varx, offs, /* isFloatUsage */ true, &baseRegUsed); + + // Make sure to use updated baseReg + assert(baseRegUsed == reg2); + emitIns_R_R(INS_add, EA_4BYTE, rsvdReg, reg2); emitIns_R_R_I(ins, attr, reg1, rsvdReg, 0); return; } From 85feea66d7e9edbac1f5c64ebdf88845bcd84a4c Mon Sep 17 00:00:00 2001 From: Kunal Pathak Date: Mon, 6 Jul 2020 16:43:27 -0700 Subject: [PATCH 09/16] jit-formatting --- src/coreclr/src/jit/emitarm.cpp | 16 +++++++++------- src/coreclr/src/jit/emitarm.h | 2 +- src/coreclr/src/jit/lclvars.cpp | 34 ++++++++++++++++----------------- 3 files changed, 27 insertions(+), 25 deletions(-) diff --git a/src/coreclr/src/jit/emitarm.cpp b/src/coreclr/src/jit/emitarm.cpp index def6ac17ebaa44..c4b309d7cb713f 100644 --- a/src/coreclr/src/jit/emitarm.cpp +++ b/src/coreclr/src/jit/emitarm.cpp @@ -3552,7 +3552,7 @@ void emitter::emitIns_R_S(instruction ins, emitAttr attr, regNumber reg1, int va int disp; unsigned undisp; - //if (offs == 1208) + // if (offs == 1208) //{ // printf("----debug break - this is it 2B!----"); //} @@ -3566,7 +3566,7 @@ void emitter::emitIns_R_S(instruction ins, emitAttr attr, regNumber reg1, int va disp = base + offs; undisp = unsigned_abs(disp); - //if (undisp == 0x1000) + // if (undisp == 0x1000) //{ // printf("----debug break - this is it!----"); //} @@ -3614,17 +3614,18 @@ void emitter::emitIns_R_S(instruction ins, emitAttr attr, regNumber reg1, int va // Load disp into a register regNumber rsvdReg = codeGen->rsGetRsvdReg(); emitIns_genStackOffset(rsvdReg, varx, offs, /* isFloatUsage */ false); - fmt = IF_T2_E0; // not sure if here too there could be case where we set the regBase inside emitIns_genStackOffset() but then don't use it further while generating the offset. + fmt = IF_T2_E0; // not sure if here too there could be case where we set the regBase inside + // emitIns_genStackOffset() but then don't use it further while generating the offset. } } else if (ins == INS_add) // no one calls this case. { assert(!"someone is calling emitIns_R_S(INS_add)"); - //if (isLowRegister(reg1) && (reg2 == REG_SP) && ((disp & 0x03fc) == disp)) + // if (isLowRegister(reg1) && (reg2 == REG_SP) && ((disp & 0x03fc) == disp)) //{ // fmt = IF_T1_J2; //} - //else if (undisp <= 0x0fff) + // else if (undisp <= 0x0fff) //{ // if (disp < 0) // { @@ -3636,12 +3637,13 @@ void emitter::emitIns_R_S(instruction ins, emitAttr attr, regNumber reg1, int va // ins = (ins == INS_add) ? INS_addw : INS_subw; // fmt = IF_T2_M0; //} - //else + // else //{ // // Load disp into a register // regNumber rsvdReg = codeGen->rsGetRsvdReg(); // emitIns_genStackOffset(rsvdReg, varx, offs, /* isFloatUsage */ false); - // emitIns_R_R_R(ins, attr, reg1, reg2, rsvdReg); // not sure if here too there could be case where we set the regBase inside emitIns_genStackOffset() but then don't use it further while generating the offset. + // emitIns_R_R_R(ins, attr, reg1, reg2, rsvdReg); // not sure if here too there could be case where we set + // the regBase inside emitIns_genStackOffset() but then don't use it further while generating the offset. // return; //} } diff --git a/src/coreclr/src/jit/emitarm.h b/src/coreclr/src/jit/emitarm.h index a1eec9a894fe44..eabcc025cba274 100644 --- a/src/coreclr/src/jit/emitarm.h +++ b/src/coreclr/src/jit/emitarm.h @@ -267,7 +267,7 @@ void emitIns_C(instruction ins, emitAttr attr, CORINFO_FIELD_HANDLE fdlHnd, int void emitIns_S(instruction ins, emitAttr attr, int varx, int offs); -void emitIns_genStackOffset(regNumber r, int varx, int offs, bool isFloatUsage, regNumber* baseReg = nullptr); +void emitIns_genStackOffset(regNumber r, int varx, int offs, bool isFloatUsage, regNumber* baseReg = nullptr); void emitIns_S_R(instruction ins, emitAttr attr, regNumber ireg, int varx, int offs); diff --git a/src/coreclr/src/jit/lclvars.cpp b/src/coreclr/src/jit/lclvars.cpp index 184b19ff46531a..820b84215c01bd 100644 --- a/src/coreclr/src/jit/lclvars.cpp +++ b/src/coreclr/src/jit/lclvars.cpp @@ -352,7 +352,7 @@ void Compiler::lvaInitArgs(InitVarDscInfo* varDscInfo) /* If we have a hidden return-buffer parameter, that comes here */ lvaInitRetBuffArg(varDscInfo); - //====================================================================== +//====================================================================== #if USER_ARGS_COME_LAST //@GENERICS: final instantiation-info argument for shared generic methods @@ -559,9 +559,9 @@ void Compiler::lvaInitRetBuffArg(InitVarDscInfo* varDscInfo) /*****************************************************************************/ void Compiler::lvaInitUserArgs(InitVarDscInfo* varDscInfo) { - //------------------------------------------------------------------------- - // Walk the function signature for the explicit arguments - //------------------------------------------------------------------------- +//------------------------------------------------------------------------- +// Walk the function signature for the explicit arguments +//------------------------------------------------------------------------- #if defined(TARGET_X86) // Only (some of) the implicit args are enregistered for varargs @@ -1033,8 +1033,8 @@ void Compiler::lvaInitUserArgs(InitVarDscInfo* varDscInfo) #if defined(TARGET_X86) varDsc->lvStkOffs = compArgSize; #else // !TARGET_X86 - // TODO-CQ: We shouldn't have to go as far as to declare these - // address-exposed -- DoNotEnregister should suffice. + // TODO-CQ: We shouldn't have to go as far as to declare these + // address-exposed -- DoNotEnregister should suffice. lvaSetVarAddrExposed(varDscInfo->varNum); #endif // !TARGET_X86 } @@ -1313,7 +1313,7 @@ void Compiler::lvaInitVarDsc(LclVarDsc* varDsc, varDsc->lvIsImplicitByRef = 0; #endif // defined(TARGET_AMD64) || defined(TARGET_ARM64) - // Set the lvType (before this point it is TYP_UNDEF). +// Set the lvType (before this point it is TYP_UNDEF). #ifdef FEATURE_HFA varDsc->SetHfaType(TYP_UNDEF); @@ -2198,7 +2198,7 @@ void Compiler::StructPromotionHelper::PromoteStructVar(unsigned lclNum) compiler->compFloatingPointUsed = true; } - // Now grab the temp for the field local. +// Now grab the temp for the field local. #ifdef DEBUG char buf[200]; @@ -5002,7 +5002,7 @@ void Compiler::lvaFixVirtualFrameOffsets() #endif ) #endif // !defined(TARGET_AMD64) - ) + ) { doAssignStkOffs = false; // Not on frame or an incomming stack arg } @@ -5311,8 +5311,8 @@ void Compiler::lvaAssignVirtualFrameOffsetsToArgs() // ret address slot, stack frame padding, alloca instructions, etc. // Note: This is the implementation for UNIX_AMD64 System V platforms. // -int Compiler::lvaAssignVirtualFrameOffsetToArg(unsigned lclNum, - unsigned argSize, +int Compiler::lvaAssignVirtualFrameOffsetToArg(unsigned lclNum, + unsigned argSize, int argOffs UNIX_AMD64_ABI_ONLY_ARG(int* callerArgOffset)) { noway_assert(lclNum < info.compArgsCount); @@ -5707,8 +5707,8 @@ int Compiler::lvaAssignVirtualFrameOffsetToArg(unsigned lclNum, if ((varDsc->TypeGet() == TYP_LONG) && varDsc->lvPromoted) { noway_assert(varDsc->lvFieldCnt == 2); - fieldVarNum = varDsc->lvFieldLclStart; - lvaTable[fieldVarNum].lvStkOffs = varDsc->lvStkOffs; + fieldVarNum = varDsc->lvFieldLclStart; + lvaTable[fieldVarNum].lvStkOffs = varDsc->lvStkOffs; lvaTable[fieldVarNum + 1].lvStkOffs = varDsc->lvStkOffs + genTypeSize(TYP_INT); } else @@ -6720,7 +6720,7 @@ void Compiler::lvaAlignFrame() // Ensure that the stack is always 16-byte aligned by grabbing an unused QWORD // if needed. bool regPushedCountAligned = (compCalleeRegsPushed % (16 / REGSIZE_BYTES)) == 0; - bool lclFrameSizeAligned = (compLclFrameSize % 16) == 0; + bool lclFrameSizeAligned = (compLclFrameSize % 16) == 0; // If this isn't the final frame layout, assume we have to push an extra QWORD // Just so the offsets are true upper limits. @@ -6733,7 +6733,7 @@ void Compiler::lvaAlignFrame() // Ensure that stack offsets will be double-aligned by grabbing an unused DWORD if needed. // - bool lclFrameSizeAligned = (compLclFrameSize % sizeof(double)) == 0; + bool lclFrameSizeAligned = (compLclFrameSize % sizeof(double)) == 0; bool regPushedCountAligned = ((compCalleeRegsPushed + genCountBits(codeGen->regSet.rsMaskPreSpillRegs(true))) % (sizeof(double) / TARGET_POINTER_SIZE)) == 0; @@ -6815,7 +6815,7 @@ void Compiler::lvaAssignFrameOffsetsToPromotedStructs() && !varDsc->lvIsParam #endif // !defined(TARGET_ARM) #endif // !UNIX_AMD64_ABI - ) + ) { LclVarDsc* parentvarDsc = &lvaTable[varDsc->lvParentLcl]; lvaPromotionType promotionType = lvaGetPromotionType(parentvarDsc); @@ -6966,7 +6966,7 @@ void Compiler::lvaDumpFrameLocation(unsigned lclNum) offset = lvaFrameAddress(lclNum, compLocallocUsed, &baseReg, 0, /* isFloatUsage */ false); #else bool EBPbased; - offset = lvaFrameAddress(lclNum, &EBPbased); + offset = lvaFrameAddress(lclNum, &EBPbased); baseReg = EBPbased ? REG_FPBASE : REG_SPBASE; #endif From b8ac22b07ee8f66cc513a622b6ab3c9b0993580a Mon Sep 17 00:00:00 2001 From: Kunal Pathak Date: Tue, 7 Jul 2020 00:04:24 -0700 Subject: [PATCH 10/16] add back case of INS_add and some more asserts --- src/coreclr/src/jit/emitarm.cpp | 80 ++++++++++++++++++--------------- 1 file changed, 45 insertions(+), 35 deletions(-) diff --git a/src/coreclr/src/jit/emitarm.cpp b/src/coreclr/src/jit/emitarm.cpp index c4b309d7cb713f..e1812b1e2944d8 100644 --- a/src/coreclr/src/jit/emitarm.cpp +++ b/src/coreclr/src/jit/emitarm.cpp @@ -3523,6 +3523,7 @@ void emitter::emitIns_R_S(instruction ins, emitAttr attr, regNumber reg1, int va switch (ins) { + case INS_add: case INS_ldr: case INS_ldrh: case INS_ldrb: @@ -3611,45 +3612,50 @@ void emitter::emitIns_R_S(instruction ins, emitAttr attr, regNumber reg1, int va } else { + regNumber baseReg; // Load disp into a register regNumber rsvdReg = codeGen->rsGetRsvdReg(); - emitIns_genStackOffset(rsvdReg, varx, offs, /* isFloatUsage */ false); - fmt = IF_T2_E0; // not sure if here too there could be case where we set the regBase inside - // emitIns_genStackOffset() but then don't use it further while generating the offset. + emitIns_genStackOffset(rsvdReg, varx, offs, /* isFloatUsage */ false, &baseReg); + fmt = IF_T2_E0; + + // Ensure the baseReg calculated is correct. + assert(baseReg == reg2); } } - else if (ins == INS_add) // no one calls this case. - { - assert(!"someone is calling emitIns_R_S(INS_add)"); - // if (isLowRegister(reg1) && (reg2 == REG_SP) && ((disp & 0x03fc) == disp)) - //{ - // fmt = IF_T1_J2; - //} - // else if (undisp <= 0x0fff) - //{ - // if (disp < 0) - // { - // ins = INS_sub; - // disp = -disp; - // } - // // add/sub => addw/subw instruction - // // Note that even when using the w prefix the immediate is still only 12 bits? - // ins = (ins == INS_add) ? INS_addw : INS_subw; - // fmt = IF_T2_M0; - //} - // else - //{ - // // Load disp into a register - // regNumber rsvdReg = codeGen->rsGetRsvdReg(); - // emitIns_genStackOffset(rsvdReg, varx, offs, /* isFloatUsage */ false); - // emitIns_R_R_R(ins, attr, reg1, reg2, rsvdReg); // not sure if here too there could be case where we set - // the regBase inside emitIns_genStackOffset() but then don't use it further while generating the offset. - // return; - //} + else if (ins == INS_add) + { + if (isLowRegister(reg1) && (reg2 == REG_SP) && ((disp & 0x03fc) == disp)) + { + fmt = IF_T1_J2; + } + else if (undisp <= 0x0fff) + { + if (disp < 0) + { + ins = INS_sub; + disp = -disp; + } + // add/sub => addw/subw instruction + // Note that even when using the w prefix the immediate is still only 12 bits? + ins = (ins == INS_add) ? INS_addw : INS_subw; + fmt = IF_T2_M0; + } + else + { + regNumber baseReg; + // Load disp into a register + regNumber rsvdReg = codeGen->rsGetRsvdReg(); + emitIns_genStackOffset(rsvdReg, varx, offs, /* isFloatUsage */ false, &baseReg); + + // Ensure the baseReg calculated is correct. + assert(baseReg == reg2); + emitIns_R_R_R(ins, attr, reg1, reg2, rsvdReg); + return; + } } else if (ins == INS_movw || ins == INS_movt) { - fmt = IF_T2_N; // assert (reg2 is not used in next instruction which can be add) + fmt = IF_T2_N; } assert((fmt == IF_T1_J2) || (fmt == IF_T2_E0) || (fmt == IF_T2_H0) || (fmt == IF_T2_K1) || (fmt == IF_T2_L0) || @@ -3759,11 +3765,11 @@ void emitter::emitIns_S_R(instruction ins, emitAttr attr, regNumber reg1, int va } else { - regNumber rsvdReg = codeGen->rsGetRsvdReg(); regNumber baseRegUsed; + regNumber rsvdReg = codeGen->rsGetRsvdReg(); emitIns_genStackOffset(rsvdReg, varx, offs, /* isFloatUsage */ true, &baseRegUsed); - // Make sure to use updated baseReg + // Ensure the baseReg calculated is correct. assert(baseRegUsed == reg2); emitIns_R_R(INS_add, EA_4BYTE, rsvdReg, reg2); emitIns_R_R_I(ins, attr, reg1, rsvdReg, 0); @@ -3784,10 +3790,14 @@ void emitter::emitIns_S_R(instruction ins, emitAttr attr, regNumber reg1, int va } else { + regNumber baseRegUsed; // Load disp into a register regNumber rsvdReg = codeGen->rsGetRsvdReg(); - emitIns_genStackOffset(rsvdReg, varx, offs, /* isFloatUsage */ false); + emitIns_genStackOffset(rsvdReg, varx, offs, /* isFloatUsage */ false, &baseRegUsed); fmt = IF_T2_E0; + + // Ensure the baseReg calculated is correct. + assert(baseRegUsed == reg2); } assert((fmt == IF_T1_J2) || (fmt == IF_T2_E0) || (fmt == IF_T2_H0) || (fmt == IF_T2_VLDST) || (fmt == IF_T2_K1)); assert(sf != INS_FLAGS_DONT_CARE); From f288c4072848e051b8cd4dec0417f06bf0a39e96 Mon Sep 17 00:00:00 2001 From: Kunal Pathak Date: Tue, 7 Jul 2020 09:32:59 -0700 Subject: [PATCH 11/16] reset lclvars.cpp --- src/coreclr/src/jit/lclvars.cpp | 54 ++++++++++++++++----------------- 1 file changed, 27 insertions(+), 27 deletions(-) diff --git a/src/coreclr/src/jit/lclvars.cpp b/src/coreclr/src/jit/lclvars.cpp index 820b84215c01bd..f8757328e85130 100644 --- a/src/coreclr/src/jit/lclvars.cpp +++ b/src/coreclr/src/jit/lclvars.cpp @@ -292,9 +292,9 @@ void Compiler::lvaInitTypeRef() } if ( // If there already exist unsafe buffers, don't mark more structs as unsafe - // as that will cause them to be placed along with the real unsafe buffers, - // unnecessarily exposing them to overruns. This can affect GS tests which - // intentionally do buffer-overruns. + // as that will cause them to be placed along with the real unsafe buffers, + // unnecessarily exposing them to overruns. This can affect GS tests which + // intentionally do buffer-overruns. !getNeedsGSSecurityCookie() && // GS checks require the stack to be re-ordered, which can't be done with EnC !opts.compDbgEnC && compStressCompile(STRESS_UNSAFE_BUFFER_CHECKS, 25)) @@ -3394,8 +3394,8 @@ void Compiler::lvaSortByRefCount() if (varDsc->lvAddrExposed) { varDsc->lvTracked = 0; - assert(varDsc->lvType != TYP_STRUCT || varDsc->lvDoNotEnregister); // For structs, should have set this when - // we set lvAddrExposed. + assert(varDsc->lvType != TYP_STRUCT || + varDsc->lvDoNotEnregister); // For structs, should have set this when we set lvAddrExposed. } else if (varTypeIsStruct(varDsc)) { @@ -3549,8 +3549,8 @@ void LclVarDsc::lvaDisqualifyVar() #endif // ASSERTION_PROP /********************************************************************************** - * Get stack size of the varDsc. - */ +* Get stack size of the varDsc. +*/ size_t LclVarDsc::lvArgStackSize() const { // Make sure this will have a stack size @@ -3591,8 +3591,8 @@ size_t LclVarDsc::lvArgStackSize() const } /********************************************************************************** - * Get type of a variable when passed as an argument. - */ +* Get type of a variable when passed as an argument. +*/ var_types LclVarDsc::lvaArgType() { var_types type = TypeGet(); @@ -4368,11 +4368,11 @@ inline void Compiler::lvaIncrementFrameSize(unsigned size) } /**************************************************************************** - * - * Return true if absolute offsets of temps are larger than vars, or in other - * words, did we allocate temps before of after vars. The /GS buffer overrun - * checks want temps to be at low stack addresses than buffers - */ +* +* Return true if absolute offsets of temps are larger than vars, or in other +* words, did we allocate temps before of after vars. The /GS buffer overrun +* checks want temps to be at low stack addresses than buffers +*/ bool Compiler::lvaTempsHaveLargerOffsetThanVars() { #ifdef TARGET_ARM @@ -4391,10 +4391,10 @@ bool Compiler::lvaTempsHaveLargerOffsetThanVars() } /**************************************************************************** - * - * Return an upper bound estimate for the size of the compiler spill temps - * - */ +* +* Return an upper bound estimate for the size of the compiler spill temps +* +*/ unsigned Compiler::lvaGetMaxSpillTempSize() { unsigned result = 0; @@ -5665,8 +5665,8 @@ int Compiler::lvaAssignVirtualFrameOffsetToArg(unsigned lclNum, (codeGen->regSet.rsMaskPreSpillAlign & genRegMask(REG_ARG_LAST)); noway_assert(cond); - noway_assert(sizeofPreSpillRegArgs <= argOffs + TARGET_POINTER_SIZE); // at most one register of - // alignment + noway_assert(sizeofPreSpillRegArgs <= + argOffs + TARGET_POINTER_SIZE); // at most one register of alignment } argOffs = sizeofPreSpillRegArgs; } @@ -5836,8 +5836,8 @@ void Compiler::lvaAssignVirtualFrameOffsetsToLocals() stkOffs -= initialStkOffs; } - if (codeGen->IsSaveFpLrWithAllCalleeSavedRegisters() || !isFramePointerUsed()) // Note that currently we always have - // a frame pointer + if (codeGen->IsSaveFpLrWithAllCalleeSavedRegisters() || + !isFramePointerUsed()) // Note that currently we always have a frame pointer { stkOffs -= compCalleeRegsPushed * REGSIZE_BYTES; } @@ -6488,8 +6488,8 @@ void Compiler::lvaAssignVirtualFrameOffsetsToLocals() #endif // FEATURE_EH_FUNCLETS && defined(TARGET_AMD64) #ifdef TARGET_ARM64 - if (!codeGen->IsSaveFpLrWithAllCalleeSavedRegisters() && isFramePointerUsed()) // Note that currently we always have - // a frame pointer + if (!codeGen->IsSaveFpLrWithAllCalleeSavedRegisters() && + isFramePointerUsed()) // Note that currently we always have a frame pointer { // Create space for saving FP and LR. stkOffs -= 2 * REGSIZE_BYTES; @@ -7227,9 +7227,9 @@ void Compiler::lvaDumpEntry(unsigned lclNum, FrameLayoutState curState, size_t r } /***************************************************************************** - * - * dump the lvaTable - */ +* +* dump the lvaTable +*/ void Compiler::lvaTableDump(FrameLayoutState curState) { From 9e450f8e9b51772cc051bf91348d68798281e48e Mon Sep 17 00:00:00 2001 From: Kunal Pathak Date: Tue, 7 Jul 2020 09:37:28 -0700 Subject: [PATCH 12/16] delete comments and cleanup code --- src/coreclr/src/jit/emitarm.cpp | 37 +++++++++------------------------ src/coreclr/src/jit/emitarm.h | 2 +- 2 files changed, 11 insertions(+), 28 deletions(-) diff --git a/src/coreclr/src/jit/emitarm.cpp b/src/coreclr/src/jit/emitarm.cpp index e1812b1e2944d8..203bfa83cee010 100644 --- a/src/coreclr/src/jit/emitarm.cpp +++ b/src/coreclr/src/jit/emitarm.cpp @@ -3547,17 +3547,13 @@ void emitter::emitIns_R_S(instruction ins, emitAttr attr, regNumber reg1, int va insFormat fmt = IF_NONE; insFlags sf = INS_FLAGS_NOT_SET; regNumber reg2; + regNumber baseRegUsed; /* Figure out the variable's frame position */ int base; int disp; unsigned undisp; - // if (offs == 1208) - //{ - // printf("----debug break - this is it 2B!----"); - //} - base = emitComp->lvaFrameAddress(varx, emitComp->funCurrentFunc()->funKind != FUNC_ROOT, ®2, offs, CodeGen::instIsFP(ins)); if (baseReg != nullptr) @@ -3567,10 +3563,7 @@ void emitter::emitIns_R_S(instruction ins, emitAttr attr, regNumber reg1, int va disp = base + offs; undisp = unsigned_abs(disp); - // if (undisp == 0x1000) - //{ - // printf("----debug break - this is it!----"); - //} + if (CodeGen::instIsFP(ins)) { // all fp mem ops take 8 bit immediate, multiplied by 4, plus sign @@ -3587,10 +3580,7 @@ void emitter::emitIns_R_S(instruction ins, emitAttr attr, regNumber reg1, int va else { regNumber rsvdReg = codeGen->rsGetRsvdReg(); - regNumber baseRegUsed; emitIns_genStackOffset(rsvdReg, varx, offs, /* isFloatUsage */ true, &baseRegUsed); - - // Make sure to use updated baseReg emitIns_R_R(INS_add, EA_4BYTE, rsvdReg, baseRegUsed); emitIns_R_R_I(ins, attr, reg1, rsvdReg, 0); return; @@ -3612,14 +3602,13 @@ void emitter::emitIns_R_S(instruction ins, emitAttr attr, regNumber reg1, int va } else { - regNumber baseReg; // Load disp into a register regNumber rsvdReg = codeGen->rsGetRsvdReg(); - emitIns_genStackOffset(rsvdReg, varx, offs, /* isFloatUsage */ false, &baseReg); + emitIns_genStackOffset(rsvdReg, varx, offs, /* isFloatUsage */ false, &baseRegUsed); fmt = IF_T2_E0; // Ensure the baseReg calculated is correct. - assert(baseReg == reg2); + assert(baseRegUsed == reg2); } } else if (ins == INS_add) @@ -3642,13 +3631,12 @@ void emitter::emitIns_R_S(instruction ins, emitAttr attr, regNumber reg1, int va } else { - regNumber baseReg; // Load disp into a register regNumber rsvdReg = codeGen->rsGetRsvdReg(); - emitIns_genStackOffset(rsvdReg, varx, offs, /* isFloatUsage */ false, &baseReg); + emitIns_genStackOffset(rsvdReg, varx, offs, /* isFloatUsage */ false, &baseRegUsed); // Ensure the baseReg calculated is correct. - assert(baseReg == reg2); + assert(baseRegUsed == reg2); emitIns_R_R_R(ins, attr, reg1, reg2, rsvdReg); return; } @@ -3686,6 +3674,7 @@ void emitter::emitIns_R_S(instruction ins, emitAttr attr, regNumber reg1, int va } // generate the offset of &varx + offs into a register +// also capture the 'baseReg' used for generating movw/movt void emitter::emitIns_genStackOffset(regNumber r, int varx, int offs, bool isFloatUsage, regNumber* baseReg) { regNumber regBase; @@ -3696,18 +3685,13 @@ void emitter::emitIns_genStackOffset(regNumber r, int varx, int offs, bool isFlo emitComp->lvaFrameAddress(varx, emitComp->funCurrentFunc()->funKind != FUNC_ROOT, ®Base, offs, isFloatUsage); disp = base + offs; - emitIns_R_S(INS_movw, EA_4BYTE, r, varx, offs, ®Base); + emitIns_R_S(INS_movw, EA_4BYTE, r, varx, offs, baseReg); if ((disp & 0xffff) != disp) { regNumber regBaseUsedInMovT; emitIns_R_S(INS_movt, EA_4BYTE, r, varx, offs, ®BaseUsedInMovT); - assert(regBase == regBaseUsedInMovT); - } - - if (baseReg != nullptr) - { - *baseReg = regBase; + assert(*baseReg == regBaseUsedInMovT); } } @@ -3738,6 +3722,7 @@ void emitter::emitIns_S_R(instruction ins, emitAttr attr, regNumber reg1, int va insFormat fmt = IF_NONE; insFlags sf = INS_FLAGS_NOT_SET; regNumber reg2; + regNumber baseRegUsed; /* Figure out the variable's frame position */ int base; @@ -3765,7 +3750,6 @@ void emitter::emitIns_S_R(instruction ins, emitAttr attr, regNumber reg1, int va } else { - regNumber baseRegUsed; regNumber rsvdReg = codeGen->rsGetRsvdReg(); emitIns_genStackOffset(rsvdReg, varx, offs, /* isFloatUsage */ true, &baseRegUsed); @@ -3790,7 +3774,6 @@ void emitter::emitIns_S_R(instruction ins, emitAttr attr, regNumber reg1, int va } else { - regNumber baseRegUsed; // Load disp into a register regNumber rsvdReg = codeGen->rsGetRsvdReg(); emitIns_genStackOffset(rsvdReg, varx, offs, /* isFloatUsage */ false, &baseRegUsed); diff --git a/src/coreclr/src/jit/emitarm.h b/src/coreclr/src/jit/emitarm.h index eabcc025cba274..f30a576f0879d1 100644 --- a/src/coreclr/src/jit/emitarm.h +++ b/src/coreclr/src/jit/emitarm.h @@ -267,7 +267,7 @@ void emitIns_C(instruction ins, emitAttr attr, CORINFO_FIELD_HANDLE fdlHnd, int void emitIns_S(instruction ins, emitAttr attr, int varx, int offs); -void emitIns_genStackOffset(regNumber r, int varx, int offs, bool isFloatUsage, regNumber* baseReg = nullptr); +void emitIns_genStackOffset(regNumber r, int varx, int offs, bool isFloatUsage, regNumber* baseReg); void emitIns_S_R(instruction ins, emitAttr attr, regNumber ireg, int varx, int offs); From 6ab40f28e0b081f338908670e95d5d8c16e08b21 Mon Sep 17 00:00:00 2001 From: Kunal Pathak Date: Tue, 7 Jul 2020 10:10:05 -0700 Subject: [PATCH 13/16] revert changes inside common.il --- src/coreclr/tests/src/JIT/jit64/mcc/common/common.il | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/coreclr/tests/src/JIT/jit64/mcc/common/common.il b/src/coreclr/tests/src/JIT/jit64/mcc/common/common.il index eb951451c915a1..dad1e37692638e 100644 --- a/src/coreclr/tests/src/JIT/jit64/mcc/common/common.il +++ b/src/coreclr/tests/src/JIT/jit64/mcc/common/common.il @@ -96,7 +96,7 @@ // Code size 34 (0x22) .maxstack 8 IL_0000: ldarg.0 - IL_0001: ldstr "FAILED: [field {0}] => actual = {1}, expected = {2}" + IL_0001: ldstr "FAILED: [field {0}] => expected = {1}, actual = {2}" IL_0006: ldarg.1 IL_0007: ldarg.3 IL_0008: box [mscorlib]System.Double From db7e86dd9c6db0c529c36388a073cd1fde3884d4 Mon Sep 17 00:00:00 2001 From: Kunal Pathak Date: Tue, 7 Jul 2020 10:45:47 -0700 Subject: [PATCH 14/16] Revert "Disable failing Windows arm32 tests (#38844)" This reverts commit 311fd810053bcfc4933d1a371323306f8860b3eb. --- src/coreclr/tests/issues.targets | 12 ------------ 1 file changed, 12 deletions(-) diff --git a/src/coreclr/tests/issues.targets b/src/coreclr/tests/issues.targets index ec07afb575c5a9..14a87203905912 100644 --- a/src/coreclr/tests/issues.targets +++ b/src/coreclr/tests/issues.targets @@ -557,18 +557,6 @@ https://github.com/dotnet/runtime/issues/12979 - - https://github.com/dotnet/runtime/issues/35501 - - - https://github.com/dotnet/runtime/issues/35501 - - - https://github.com/dotnet/runtime/issues/35501 - - - https://github.com/dotnet/runtime/issues/35501 - From 943771d2545518bd44b6a6da8e84a74495eaa0c7 Mon Sep 17 00:00:00 2001 From: Kunal Pathak Date: Tue, 7 Jul 2020 13:26:09 -0700 Subject: [PATCH 15/16] review comments --- src/coreclr/src/jit/emitarm.cpp | 22 ++++++++++++++-------- src/coreclr/src/jit/emitarm.h | 4 ++-- 2 files changed, 16 insertions(+), 10 deletions(-) diff --git a/src/coreclr/src/jit/emitarm.cpp b/src/coreclr/src/jit/emitarm.cpp index 203bfa83cee010..d1fb915be155b6 100644 --- a/src/coreclr/src/jit/emitarm.cpp +++ b/src/coreclr/src/jit/emitarm.cpp @@ -3513,8 +3513,10 @@ void emitter::emitIns_S(instruction ins, emitAttr attr, int varx, int offs) /***************************************************************************** * * Add an instruction referencing a register and a stack-based local variable. + * + * Stores the base register used for accessing the offset of stack in "pBaseReg". */ -void emitter::emitIns_R_S(instruction ins, emitAttr attr, regNumber reg1, int varx, int offs, regNumber* baseReg) +void emitter::emitIns_R_S(instruction ins, emitAttr attr, regNumber reg1, int varx, int offs, regNumber* pBaseReg) { if (ins == INS_mov) { @@ -3556,9 +3558,9 @@ void emitter::emitIns_R_S(instruction ins, emitAttr attr, regNumber reg1, int va base = emitComp->lvaFrameAddress(varx, emitComp->funCurrentFunc()->funKind != FUNC_ROOT, ®2, offs, CodeGen::instIsFP(ins)); - if (baseReg != nullptr) + if (pBaseReg != nullptr) { - *baseReg = reg2; + *pBaseReg = reg2; } disp = base + offs; @@ -3673,9 +3675,13 @@ void emitter::emitIns_R_S(instruction ins, emitAttr attr, regNumber reg1, int va appendToCurIG(id); } -// generate the offset of &varx + offs into a register -// also capture the 'baseReg' used for generating movw/movt -void emitter::emitIns_genStackOffset(regNumber r, int varx, int offs, bool isFloatUsage, regNumber* baseReg) +/***************************************************************************** +* +* Generate the offset of &varx + offs into a register +* +* Stores the base register used for accessing the offset of stack in "pBaseReg". +*/ + void emitter::emitIns_genStackOffset(regNumber r, int varx, int offs, bool isFloatUsage, regNumber* pBaseReg) { regNumber regBase; int base; @@ -3685,13 +3691,13 @@ void emitter::emitIns_genStackOffset(regNumber r, int varx, int offs, bool isFlo emitComp->lvaFrameAddress(varx, emitComp->funCurrentFunc()->funKind != FUNC_ROOT, ®Base, offs, isFloatUsage); disp = base + offs; - emitIns_R_S(INS_movw, EA_4BYTE, r, varx, offs, baseReg); + emitIns_R_S(INS_movw, EA_4BYTE, r, varx, offs, pBaseReg); if ((disp & 0xffff) != disp) { regNumber regBaseUsedInMovT; emitIns_R_S(INS_movt, EA_4BYTE, r, varx, offs, ®BaseUsedInMovT); - assert(*baseReg == regBaseUsedInMovT); + assert(*pBaseReg == regBaseUsedInMovT); } } diff --git a/src/coreclr/src/jit/emitarm.h b/src/coreclr/src/jit/emitarm.h index f30a576f0879d1..8c9c77eec98d91 100644 --- a/src/coreclr/src/jit/emitarm.h +++ b/src/coreclr/src/jit/emitarm.h @@ -267,11 +267,11 @@ void emitIns_C(instruction ins, emitAttr attr, CORINFO_FIELD_HANDLE fdlHnd, int void emitIns_S(instruction ins, emitAttr attr, int varx, int offs); -void emitIns_genStackOffset(regNumber r, int varx, int offs, bool isFloatUsage, regNumber* baseReg); +void emitIns_genStackOffset(regNumber r, int varx, int offs, bool isFloatUsage, regNumber* pBaseReg); void emitIns_S_R(instruction ins, emitAttr attr, regNumber ireg, int varx, int offs); -void emitIns_R_S(instruction ins, emitAttr attr, regNumber ireg, int varx, int offs, regNumber* baseReg = nullptr); +void emitIns_R_S(instruction ins, emitAttr attr, regNumber ireg, int varx, int offs, regNumber* pBaseReg = nullptr); void emitIns_S_I(instruction ins, emitAttr attr, int varx, int offs, int val); From 0094682748b14a3f928619b34c7be32fe9414b90 Mon Sep 17 00:00:00 2001 From: Kunal Pathak Date: Tue, 7 Jul 2020 14:45:23 -0700 Subject: [PATCH 16/16] added standard function header --- src/coreclr/src/jit/emitarm.cpp | 46 +++++++++++++++++++++++---------- 1 file changed, 33 insertions(+), 13 deletions(-) diff --git a/src/coreclr/src/jit/emitarm.cpp b/src/coreclr/src/jit/emitarm.cpp index d1fb915be155b6..bf586cb0353060 100644 --- a/src/coreclr/src/jit/emitarm.cpp +++ b/src/coreclr/src/jit/emitarm.cpp @@ -3510,12 +3510,21 @@ void emitter::emitIns_S(instruction ins, emitAttr attr, int varx, int offs) NYI("emitIns_S"); } -/***************************************************************************** - * - * Add an instruction referencing a register and a stack-based local variable. - * - * Stores the base register used for accessing the offset of stack in "pBaseReg". - */ +//------------------------------------------------------------------------------------- +// emitIns_R_S: Add an instruction referencing a register and a stack-based local variable. +// +// Arguments: +// ins - The instruction to add. +// attr - Oeration size. +// varx - The variable to generate offset for. +// offs - The offset of variable or field in stack. +// pBaseReg - The base register that is used while calculating the offset. For example, if the offset +// with "stack pointer" can't be encoded in instruction, "frame pointer" can be used to get +// the offset of the field. In such case, pBaseReg will store the "fp". +// +// Return Value: +// The pBaseReg that holds the base register that was used to calculate the offset. +// void emitter::emitIns_R_S(instruction ins, emitAttr attr, regNumber reg1, int varx, int offs, regNumber* pBaseReg) { if (ins == INS_mov) @@ -3675,13 +3684,24 @@ void emitter::emitIns_R_S(instruction ins, emitAttr attr, regNumber reg1, int va appendToCurIG(id); } -/***************************************************************************** -* -* Generate the offset of &varx + offs into a register -* -* Stores the base register used for accessing the offset of stack in "pBaseReg". -*/ - void emitter::emitIns_genStackOffset(regNumber r, int varx, int offs, bool isFloatUsage, regNumber* pBaseReg) +//------------------------------------------------------------------------------------- +// emitIns_genStackOffset: Generate the offset of &varx + offs into a register +// +// Arguments: +// r - Register in which offset calculation result is stored. +// varx - The variable to generate offset for. +// offs - The offset of variable or field in stack. +// isFloatUsage - True if the instruction being generated is a floating point instruction. This requires using +// floating-point offset restrictions. Note that a variable can be non-float, e.g., struct, but +// accessed as a float local field. +// pBaseReg - The base register that is used while calculating the offset. For example, if the offset with +// "stack pointer" can't be encoded in instruction, "frame pointer" can be used to get the offset +// of the field. In such case, pBaseReg will store the "fp". +// +// Return Value: +// The pBaseReg that holds the base register that was used to calculate the offset. +// +void emitter::emitIns_genStackOffset(regNumber r, int varx, int offs, bool isFloatUsage, regNumber* pBaseReg) { regNumber regBase; int base;