Skip to content

Commit

Permalink
Always set addlDelta to zero on x86 (#79467)
Browse files Browse the repository at this point in the history
* 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.

* Re-enable the test
  • Loading branch information
SingleAccretion authored Dec 14, 2022
1 parent 582e522 commit ef903fd
Show file tree
Hide file tree
Showing 2 changed files with 11 additions and 21 deletions.
29 changes: 11 additions & 18 deletions src/coreclr/jit/emitxarch.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -11691,8 +11691,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
Expand All @@ -11705,20 +11703,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)
{
Expand All @@ -11739,6 +11734,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()
Expand Down Expand Up @@ -12990,20 +12986,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)
{
Expand All @@ -13024,6 +13016,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
Expand Down
3 changes: 0 additions & 3 deletions src/tests/issues.targets
Original file line number Diff line number Diff line change
Expand Up @@ -277,9 +277,6 @@
<ExcludeList Include="$(XunitTestBinBase)/Regressions/coreclr/GitHub_34094/Test34094/*">
<Issue>https://github.com/dotnet/runtime/issues/57458</Issue>
</ExcludeList>
<ExcludeList Include="$(XunitTestBinBase)/JIT/Regression/JitBlue/Runtime_31615/Runtime_31615/*">
<Issue>https://github.com/dotnet/runtime/issues/79170</Issue>
</ExcludeList>
</ItemGroup>

<!-- Windows arm32 specific excludes -->
Expand Down

0 comments on commit ef903fd

Please sign in to comment.