<div class="gmail_quote">On Thu, Oct 6, 2011 at 11:26 PM, Eli Friedman <span dir="ltr"><<a href="mailto:eli.friedman@gmail.com">eli.friedman@gmail.com</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex;">
On Thu, Oct 6, 2011 at 10:35 PM, Craig Topper <<a href="mailto:craig.topper@gmail.com">craig.topper@gmail.com</a>> wrote:<br>
> Author: ctopper<br>
> Date: Fri Oct  7 00:35:38 2011<br>
> New Revision: 141353<br>
><br>
> URL: <a href="http://llvm.org/viewvc/llvm-project?rev=141353&view=rev" target="_blank">http://llvm.org/viewvc/llvm-project?rev=141353&view=rev</a><br>
> Log:<br>
> Revert part of r141274. Only need to change encoding for xchg %eax, %eax in 64-bit mode. This is because in 64-bit mode xchg %eax, %eax implies zeroing the upper 32-bits of RAX which makes it not a NOP. In 32-bit mode using NOP encoding is fine.<br>

><br>
> Modified:<br>
>    llvm/trunk/lib/Target/X86/X86InstrInfo.td<br>
>    llvm/trunk/lib/Target/X86/X86RegisterInfo.td<br>
>    llvm/trunk/test/MC/X86/x86-32.s<br>
>    llvm/trunk/test/MC/X86/x86-64.s<br>
>    llvm/trunk/utils/TableGen/X86DisassemblerTables.cpp<br>
><br>
> Modified: llvm/trunk/lib/Target/X86/X86InstrInfo.td<br>
> URL: <a href="http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Target/X86/X86InstrInfo.td?rev=141353&r1=141352&r2=141353&view=diff" target="_blank">http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Target/X86/X86InstrInfo.td?rev=141353&r1=141352&r2=141353&view=diff</a><br>

> ==============================================================================<br>
> --- llvm/trunk/lib/Target/X86/X86InstrInfo.td (original)<br>
> +++ llvm/trunk/lib/Target/X86/X86InstrInfo.td Fri Oct  7 00:35:38 2011<br>
> @@ -1154,11 +1154,15 @@<br>
>                   "xchg{q}\t{$val, $src|$src, $val}", []>;<br>
>  }<br>
><br>
> -def XCHG16ar : I<0x90, AddRegFrm, (outs), (ins GR16_NOAX:$src),<br>
> +def XCHG16ar : I<0x90, AddRegFrm, (outs), (ins GR16:$src),<br>
>                   "xchg{w}\t{$src, %ax|AX, $src}", []>, OpSize;<br>
> -def XCHG32ar : I<0x90, AddRegFrm, (outs), (ins GR32_NOAX:$src),<br>
> -                  "xchg{l}\t{$src, %eax|EAX, $src}", []>;<br>
> -def XCHG64ar : RI<0x90, AddRegFrm, (outs), (ins GR64_NOAX:$src),<br>
> +def XCHG32ar : I<0x90, AddRegFrm, (outs), (ins GR32:$src),<br>
> +                  "xchg{l}\t{$src, %eax|EAX, $src}", []>, Requires<[In32BitMode]>;<br>
> +// Uses GR32_NOAX in 64-bit mode to prevent encoding using the 0x90 NOP encoding.<br>
> +// xchg %eax, %eax needs to clear upper 32-bits of RAX so is not a NOP.<br>
> +def XCHG32ar64 : I<0x90, AddRegFrm, (outs), (ins GR32_NOAX:$src),<br>
> +                   "xchg{l}\t{$src, %eax|EAX, $src}", []>, Requires<[In64BitMode]>;<br>
> +def XCHG64ar : RI<0x90, AddRegFrm, (outs), (ins GR64:$src),<br>
>                   "xchg{q}\t{$src, %rax|RAX, $src}", []>;<br>
><br>
><br>
> @@ -1716,6 +1720,7 @@<br>
>  def : InstAlias<"xchgq $mem, $val", (XCHG64rm GR64:$val, i64mem:$mem)>;<br>
><br>
>  // xchg: We accept "xchgX <reg>, %eax" and "xchgX %eax, <reg>" as synonyms.<br>
> -def : InstAlias<"xchgw %ax, $src", (XCHG16ar GR16_NOAX:$src)>;<br>
> -def : InstAlias<"xchgl %eax, $src", (XCHG32ar GR32_NOAX:$src)>;<br>
> -def : InstAlias<"xchgq %rax, $src", (XCHG64ar GR64_NOAX:$src)>;<br>
> +def : InstAlias<"xchgw %ax, $src", (XCHG16ar GR16:$src)>;<br>
> +def : InstAlias<"xchgl %eax, $src", (XCHG32ar GR32:$src)>, Requires<[In32BitMode]>;<br>
> +def : InstAlias<"xchgl %eax, $src", (XCHG32ar64 GR32_NOAX:$src)>, Requires<[In64BitMode]>;<br>
> +def : InstAlias<"xchgq %rax, $src", (XCHG64ar GR64:$src)>;<br>
><br>
> Modified: llvm/trunk/lib/Target/X86/X86RegisterInfo.td<br>
> URL: <a href="http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Target/X86/X86RegisterInfo.td?rev=141353&r1=141352&r2=141353&view=diff" target="_blank">http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Target/X86/X86RegisterInfo.td?rev=141353&r1=141352&r2=141353&view=diff</a><br>

> ==============================================================================<br>
> --- llvm/trunk/lib/Target/X86/X86RegisterInfo.td (original)<br>
> +++ llvm/trunk/lib/Target/X86/X86RegisterInfo.td Fri Oct  7 00:35:38 2011<br>
> @@ -390,21 +390,11 @@<br>
>                        (GR32_NOREX sub_32bit)];<br>
>  }<br>
><br>
> -// GR16_NOAX - GR16 registers except AX.<br>
> -def GR16_NOAX : RegisterClass<"X86", [i16], 16, (sub GR16, AX)> {<br>
> -  let SubRegClasses = [(GR8 sub_8bit, sub_8bit_hi)];<br>
> -}<br>
> -<br>
> -// GR32_NOAX - GR32 registers except EAX.<br>
> +// GR32_NOAX - GR32 registers except EAX. Used by AddRegFrm of XCHG32 in 64-bit<br>
> +// mode to prevent encoding using the 0x90 NOP encoding. xchg %eax, %eax needs<br>
> +// to clear upper 32-bits of RAX so is not a NOP.<br>
>  def GR32_NOAX : RegisterClass<"X86", [i32], 32, (sub GR32, EAX)> {<br>
> -  let SubRegClasses = [(GR8 sub_8bit, sub_8bit_hi), (GR16_NOAX sub_16bit)];<br>
> -}<br>
> -<br>
> -// GR64_NOAX - GR64 registers except RAX.<br>
> -def GR64_NOAX : RegisterClass<"X86", [i64], 64, (sub GR64, RAX)> {<br>
> -  let SubRegClasses = [(GR8 sub_8bit, sub_8bit_hi),<br>
> -                       (GR16_NOAX sub_16bit),<br>
> -                       (GR32_NOAX sub_32bit)];<br>
> +  let SubRegClasses = [(GR8 sub_8bit, sub_8bit_hi), (GR16 sub_16bit)];<br>
>  }<br>
><br>
>  // GR32_NOSP - GR32 registers except ESP.<br>
><br>
> Modified: llvm/trunk/test/MC/X86/x86-32.s<br>
> URL: <a href="http://llvm.org/viewvc/llvm-project/llvm/trunk/test/MC/X86/x86-32.s?rev=141353&r1=141352&r2=141353&view=diff" target="_blank">http://llvm.org/viewvc/llvm-project/llvm/trunk/test/MC/X86/x86-32.s?rev=141353&r1=141352&r2=141353&view=diff</a><br>

> ==============================================================================<br>
> --- llvm/trunk/test/MC/X86/x86-32.s (original)<br>
> +++ llvm/trunk/test/MC/X86/x86-32.s Fri Oct  7 00:35:38 2011<br>
> @@ -948,11 +948,11 @@<br>
><br>
>  // PR10345<br>
>  // CHECK: xchgl %eax, %eax<br>
> -// CHECK: encoding: [0x87,0xc0]<br>
> +// CHECK: encoding: [0x90]<br>
>  xchgl %eax, %eax<br>
><br>
>  // CHECK: xchgw %ax, %ax<br>
> -// CHECK: encoding: [0x66,0x87,0xc0]<br>
> +// CHECK: encoding: [0x66,0x90]<br>
>  xchgw %ax, %ax<br>
><br>
>  // CHECK: xchgl %ecx, %eax<br>
><br>
> Modified: llvm/trunk/test/MC/X86/x86-64.s<br>
> URL: <a href="http://llvm.org/viewvc/llvm-project/llvm/trunk/test/MC/X86/x86-64.s?rev=141353&r1=141352&r2=141353&view=diff" target="_blank">http://llvm.org/viewvc/llvm-project/llvm/trunk/test/MC/X86/x86-64.s?rev=141353&r1=141352&r2=141353&view=diff</a><br>

> ==============================================================================<br>
> --- llvm/trunk/test/MC/X86/x86-64.s (original)<br>
> +++ llvm/trunk/test/MC/X86/x86-64.s Fri Oct  7 00:35:38 2011<br>
> @@ -1173,7 +1173,7 @@<br>
><br>
>  // PR10345<br>
>  // CHECK: xchgq %rax, %rax<br>
> -// CHECK: encoding: [0x48,0x87,0xc0]<br>
> +// CHECK: encoding: [0x48,0x90]<br>
>  xchgq %rax, %rax<br>
><br>
>  // CHECK: xchgl %eax, %eax<br>
> @@ -1181,7 +1181,7 @@<br>
>  xchgl %eax, %eax<br>
><br>
>  // CHECK: xchgw %ax, %ax<br>
> -// CHECK: encoding: [0x66,0x87,0xc0]<br>
> +// CHECK: encoding: [0x66,0x90]<br>
>  xchgw %ax, %ax<br>
<br>
Close, but not quite: 0x66 0x90 is a NOP, not xchgw on x86-64.<br>
<font color="#888888"><br>
-Eli<br></font></blockquote><div><br> I'm not sure I'm following. Are you saying it should be 0x66 0x87 0xc0 or just that the string for the CHECK is wrong? </div></div><br>-- <br>~Craig<br>