[PATCH] D120592: [X86] Fix handling of Address-Size override prefix

[Craig Topper via Phabricator via llvm-commits]

craig.topper added a comment.

In D120592#3348249 <https://reviews.llvm.org/D120592#3348249>, @skan wrote:

> In D120592#3348246 <https://reviews.llvm.org/D120592#3348246>, @craig.topper wrote:
>
>> In D120592#3348216 <https://reviews.llvm.org/D120592#3348216>, @skan wrote:
>>
>>> I think this patch is totally wrong. We need address size override prefix only when the instruction has a memory operand.The address-size override prefix (67H) allows programs to switch between 16- and 32-bit addressing. Either size can be the default; the prefix selects the non-default size. Using this prefix and/or other undefined opcodes when operands for the instruction do not reside in memory is reserved; such use may cause unpredictable behavior.  Whether a instruction need ASZ should be included in the definition of the instruction in tablgen.
>>
>> I believe example test case is supported by gas and callq does use address size. What syntax should be used instead?
>
> This test case is misleading.  LLVM-MC can pass the test case w/o this patch b/c an expicit prefix is added. However, this callq does not need a ASZ at all.

There is a legimate use for a redundant addr32 on a call instruction. See this code in lld

  // Convert call/jmp instructions.
  if (modRm == 0x15) {
    // ABI says we can convert "call *foo at GOTPCREL(%rip)" to "nop; call foo".
    // Instead we convert to "addr32 call foo" where addr32 is an instruction
    // prefix. That makes result expression to be a single instruction.
    loc[-2] = 0x67; // addr32 prefix
    loc[-1] = 0xe8; // call
    write32le(loc, val);
    return;
  }


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D120592/new/

https://reviews.llvm.org/D120592