llvm dev - Jul 2013 - [LLVMdev] [PATCH] x86: disambiguate unqualified btr, bts

On Wed, Jul 10, 2013 at 12:29 PM, Ramkumar Ramachandra
<artagnon at gmail.com> wrote:
> The instructions btr and bts are perfectly valid, and have existed since
> Intel 386.  GNU as supports them fine.  Unfortunately, LLVM does not
> support them, and barfs with:
>
>   error: ambiguous instructions require an explicit suffix
>
> Fix this problem by disambiguating it correctly, following the example
> set by 824a907.
>
> Cc: Eli Friedman <eli.friedman at gmail.com>
> Cc: Chris Lattner <sabre at nondot.org>
> Signed-off-by: Ramkumar Ramachandra <artagnon at gmail.com>
> ---
>  I've probably done something stupid; seems to build correctly, but
>  that's all I know.  Also, tests are pending.
>
>  lib/Target/X86/X86InstrInfo.td | 4 ++++
>  1 file changed, 4 insertions(+)
>
> diff --git a/lib/Target/X86/X86InstrInfo.td
b/lib/Target/X86/X86InstrInfo.td
> index f33ae2a..4950674 100644
> --- a/lib/Target/X86/X86InstrInfo.td
> +++ b/lib/Target/X86/X86InstrInfo.td
> @@ -1971,6 +1971,10 @@ def : InstAlias<"aam", (AAM8i8 10)>;
>  // Disambiguate the mem/imm form of bt-without-a-suffix as btl.
>  def : InstAlias<"bt $imm, $mem", (BT32mi8 i32mem:$mem,
i32i8imm:$imm)>;
>
> +// Disambiguate btr and bts, just like GNU as.
> +def : InstAlias<"btr $imm, $mem", (BT16mi8 i16mem:$mem,
i16i8imm:$imm)>;
> +def : InstAlias<"bts $imm, $mem", (BT16mi8 i16mem:$mem,
i16i8imm:$imm)>;
> +
>  // clr aliases.
>  def : InstAlias<"clrb $reg", (XOR8rr  GR8 :$reg, GR8
:$reg)>;
>  def : InstAlias<"clrw $reg", (XOR16rr GR16:$reg,
GR16:$reg)>;
> --
> 1.8.3.2.736.g869de25
>
Please send patches to llvm-commits.  Please include a testcase with
each patch.  Please check that your patch actually works correctly
before sending it to the mailing list for review.  (See
http://llvm.org/docs/DeveloperPolicy.html .)

-Eli

Also, please elaborate on why this is a good change. Because gas accepts it
isn’t sufficient reason in and of itself.

-Jim

On Jul 10, 2013, at 1:18 PM, Eli Friedman <eli.friedman at gmail.com>
wrote:
> On Wed, Jul 10, 2013 at 12:29 PM, Ramkumar Ramachandra
> <artagnon at gmail.com> wrote:
>> The instructions btr and bts are perfectly valid, and have existed
since
>> Intel 386.  GNU as supports them fine.  Unfortunately, LLVM does not
>> support them, and barfs with:
>> 
>>  error: ambiguous instructions require an explicit suffix
>> 
>> Fix this problem by disambiguating it correctly, following the example
>> set by 824a907.
>> 
>> Cc: Eli Friedman <eli.friedman at gmail.com>
>> Cc: Chris Lattner <sabre at nondot.org>
>> Signed-off-by: Ramkumar Ramachandra <artagnon at gmail.com>
>> ---
>> I've probably done something stupid; seems to build correctly, but
>> that's all I know.  Also, tests are pending.
>> 
>> lib/Target/X86/X86InstrInfo.td | 4 ++++
>> 1 file changed, 4 insertions(+)
>> 
>> diff --git a/lib/Target/X86/X86InstrInfo.td
b/lib/Target/X86/X86InstrInfo.td
>> index f33ae2a..4950674 100644
>> --- a/lib/Target/X86/X86InstrInfo.td
>> +++ b/lib/Target/X86/X86InstrInfo.td
>> @@ -1971,6 +1971,10 @@ def : InstAlias<"aam", (AAM8i8
10)>;
>> // Disambiguate the mem/imm form of bt-without-a-suffix as btl.
>> def : InstAlias<"bt $imm, $mem", (BT32mi8 i32mem:$mem,
i32i8imm:$imm)>;
>> 
>> +// Disambiguate btr and bts, just like GNU as.
>> +def : InstAlias<"btr $imm, $mem", (BT16mi8 i16mem:$mem,
i16i8imm:$imm)>;
>> +def : InstAlias<"bts $imm, $mem", (BT16mi8 i16mem:$mem,
i16i8imm:$imm)>;
>> +
>> // clr aliases.
>> def : InstAlias<"clrb $reg", (XOR8rr  GR8 :$reg, GR8
:$reg)>;
>> def : InstAlias<"clrw $reg", (XOR16rr GR16:$reg,
GR16:$reg)>;
>> --
>> 1.8.3.2.736.g869de25
>> 
> 
> Please send patches to llvm-commits.  Please include a testcase with
> each patch.  Please check that your patch actually works correctly
> before sending it to the mailing list for review.  (See
> http://llvm.org/docs/DeveloperPolicy.html .)
> 
> -Eli
> _______________________________________________
> LLVM Developers mailing list
> LLVMdev at cs.uiuc.edu         http://llvm.cs.uiuc.edu
> http://lists.cs.uiuc.edu/mailman/listinfo/llvmdev
-------------- next part --------------
An HTML attachment was scrubbed...
URL:
<http://lists.llvm.org/pipermail/llvm-dev/attachments/20130710/c37694d0/attachment.html>

Eli Friedman wrote:
>>  I've probably done something stupid; seems to build correctly, but
>>  that's all I know.  Also, tests are pending.
>
> Please send patches to llvm-commits.  Please include a testcase with
> each patch.  Please check that your patch actually works correctly
> before sending it to the mailing list for review.
I know.  I was asking for a sanity check before doing another
iteration.  Never mind: I'll submit another iteration soon.

Jim Grosbach wrote:
> Also, please elaborate on why this is a good change. Because gas accepts it
> isn’t sufficient reason in and of itself.
That they're valid instructions isn't sufficient reason?  Should I
additionally say that linux.git uses them?

I wrote:
> The instructions btr and bts are perfectly valid, and have existed since
> Intel 386.