From 978117541b0ec7f566c292d3fc9511acb996a973 Mon Sep 17 00:00:00 2001 From: Sergey Andreenko Date: Mon, 15 May 2017 16:42:18 -0700 Subject: [PATCH] Use full move for byte registers stores in jumps. (dotnet/coreclr#11570) * Use the 4-byte move for jump spilling. The other types of spilling already use the proper move: 1) for lsra spilling it always use 4-byte move because we allocate 4-byte slots; 2) for other types it sets needsByteReg and lsra chooses correct register; We do not apply the second approach to fixing this issue because jmp doesn't have real uses, that can keep this requirement on. Also, it creates more strict restrictions, that we need. Commit migrated from https://github.com/dotnet/coreclr/commit/81baf0a53fdc3cae247dd80ebc1e8f54e7ad54fa --- src/coreclr/src/jit/codegenxarch.cpp | 5 +++-- src/coreclr/src/jit/emitxarch.cpp | 6 ++++++ src/coreclr/src/jit/lclvars.cpp | 5 ++++- 3 files changed, 13 insertions(+), 3 deletions(-) diff --git a/src/coreclr/src/jit/codegenxarch.cpp b/src/coreclr/src/jit/codegenxarch.cpp index 42f041d..252f004 100644 --- a/src/coreclr/src/jit/codegenxarch.cpp +++ b/src/coreclr/src/jit/codegenxarch.cpp @@ -5406,8 +5406,9 @@ void CodeGen::genJmpMethod(GenTreePtr jmp) // assert should hold. assert(varDsc->lvRegNum != REG_STK); - var_types loadType = varDsc->lvaArgType(); - getEmitter()->emitIns_S_R(ins_Store(loadType), emitTypeSize(loadType), varDsc->lvRegNum, varNum, 0); + assert(!varDsc->lvIsStructField || (compiler->lvaTable[varDsc->lvParentLcl].lvFieldCnt == 1)); + var_types storeType = genActualType(varDsc->lvaArgType()); // We own the memory and can use the full move. + getEmitter()->emitIns_S_R(ins_Store(storeType), emitTypeSize(storeType), varDsc->lvRegNum, varNum, 0); // Update lvRegNum life and GC info to indicate lvRegNum is dead and varDsc stack slot is going live. // Note that we cannot modify varDsc->lvRegNum here because another basic block may not be expecting it. diff --git a/src/coreclr/src/jit/emitxarch.cpp b/src/coreclr/src/jit/emitxarch.cpp index 7608130..8614069 100644 --- a/src/coreclr/src/jit/emitxarch.cpp +++ b/src/coreclr/src/jit/emitxarch.cpp @@ -4821,6 +4821,12 @@ void emitter::emitIns_S_R(instruction ins, emitAttr attr, regNumber ireg, int va UNATIVE_OFFSET sz = emitInsSizeSV(insCodeMR(ins), varx, offs); insFormat fmt = emitInsModeFormat(ins, IF_SRD_RRD); +#ifdef _TARGET_X86_ + if (attr == EA_1BYTE) + { + assert(isByteReg(ireg)); + } +#endif // 16-bit operand instructions will need a prefix if (EA_SIZE(attr) == EA_2BYTE) { diff --git a/src/coreclr/src/jit/lclvars.cpp b/src/coreclr/src/jit/lclvars.cpp index e64b5a1..4770a1d 100644 --- a/src/coreclr/src/jit/lclvars.cpp +++ b/src/coreclr/src/jit/lclvars.cpp @@ -1833,7 +1833,10 @@ bool Compiler::lvaShouldPromoteStructVar(unsigned lclNum, lvaStructPromotionInfo // TODO-PERF - Implement struct promotion for incoming multireg structs // Currently it hits assert(lvFieldCnt==1) in lclvar.cpp line 4417 - + // Also the implementation of jmp uses the 4 byte move to store + // byte parameters to the stack, so that if we have a byte field + // with something else occupying the same 4-byte slot, it will + // overwrite other fields. if (structPromotionInfo->fieldCnt != 1) { JITDUMP("Not promoting promotable struct local V%02u, because lvIsParam is true and #fields = " -- 2.7.4