From 09260e69333d671c95e95d8caf5f500f76b7203b Mon Sep 17 00:00:00 2001 From: SingleAccretion Date: Fri, 9 Dec 2022 22:10:26 +0300 Subject: [PATCH 1/2] Always set "addlDelta" to zero on x86 The value is used to compensate for the additional instruction bytes, which should only be relevant for RIP-relative addressing, while x86 uses absolute addressing. --- src/coreclr/jit/emitxarch.cpp | 29 +++++++++++------------------ 1 file changed, 11 insertions(+), 18 deletions(-) diff --git a/src/coreclr/jit/emitxarch.cpp b/src/coreclr/jit/emitxarch.cpp index 19c0ea4b480871..57721c4280c7ff 100644 --- a/src/coreclr/jit/emitxarch.cpp +++ b/src/coreclr/jit/emitxarch.cpp @@ -11694,8 +11694,6 @@ BYTE* emitter::emitOutputAM(BYTE* dst, instrDesc* id, code_t code, CnsVal* addc) { if (id->idIsDspReloc()) { - INT32 addlDelta = 0; - // The address is of the form "[disp]" // On x86 - disp is relative to zero // On Amd64 - disp is relative to RIP @@ -11708,20 +11706,17 @@ BYTE* emitter::emitOutputAM(BYTE* dst, instrDesc* id, code_t code, CnsVal* addc) dst += emitOutputWord(dst, code | 0x0500); } + INT32 addlDelta = 0; +#ifdef TARGET_AMD64 if (addc) { - // It is of the form "ins [disp], imm" or "ins reg, [disp], imm" - // For emitting relocation, we also need to take into account of the - // additional bytes of code emitted for immed val. - + // It is of the form "ins [disp], imm" or "ins reg, [disp], imm". Emitting relocation for a + // RIP-relative address means we also need to take into account the additional bytes of code + // generated for the immediate value, since RIP will point at the next instruction. ssize_t cval = addc->cnsVal; -#ifdef TARGET_AMD64 // all these opcodes only take a sign-extended 4-byte immediate noway_assert(opsz < 8 || ((int)cval == cval && !addc->cnsReloc)); -#else // TARGET_X86 - noway_assert(opsz <= 4); -#endif // TARGET_X86 switch (opsz) { @@ -11742,6 +11737,7 @@ BYTE* emitter::emitOutputAM(BYTE* dst, instrDesc* id, code_t code, CnsVal* addc) unreached(); } } +#endif // TARGET_AMD64 #ifdef TARGET_AMD64 // We emit zero on Amd64, to avoid the assert in emitOutputLong() @@ -12993,20 +12989,16 @@ BYTE* emitter::emitOutputCV(BYTE* dst, instrDesc* id, code_t code, CnsVal* addc) { INT32 addlDelta = 0; +#ifdef TARGET_AMD64 if (addc) { - // It is of the form "ins [disp], imm" or "ins reg, [disp], imm" - // For emitting relocation, we also need to take into account of the - // additional bytes of code emitted for immed val. - + // It is of the form "ins [disp], imm" or "ins reg, [disp], imm". Emitting relocation for a + // RIP-relative address means we also need to take into account the additional bytes of code + // generated for the immediate value, since RIP will point at the next instruction. ssize_t cval = addc->cnsVal; -#ifdef TARGET_AMD64 // all these opcodes only take a sign-extended 4-byte immediate noway_assert(opsz < 8 || ((int)cval == cval && !addc->cnsReloc)); -#else // TARGET_X86 - noway_assert(opsz <= 4); -#endif // TARGET_X86 switch (opsz) { @@ -13027,6 +13019,7 @@ BYTE* emitter::emitOutputCV(BYTE* dst, instrDesc* id, code_t code, CnsVal* addc) unreached(); } } +#endif // TARGET_AMD64 #ifdef TARGET_AMD64 // All static field and data section constant accesses should be marked as relocatable From 32a1486b3cdabd58bf54bf51ffca8efaf2bf234b Mon Sep 17 00:00:00 2001 From: SingleAccretion Date: Fri, 9 Dec 2022 22:15:27 +0300 Subject: [PATCH 2/2] Re-enable the test --- src/tests/issues.targets | 3 --- 1 file changed, 3 deletions(-) diff --git a/src/tests/issues.targets b/src/tests/issues.targets index 64c6b4a775e5d3..2e8791bde8abff 100644 --- a/src/tests/issues.targets +++ b/src/tests/issues.targets @@ -277,9 +277,6 @@ https://github.com/dotnet/runtime/issues/57458 - - https://github.com/dotnet/runtime/issues/79170 - https://github.com/dotnet/runtime/issues/78898