[PATCH v2] X86: disambiguate unqualified btr, bts

[PaX Team]

On 11 Jul 2013 at 17:17, Eli Friedman wrote:

> On Wed, Jul 10, 2013 at 11:47 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, and there are instances of its
> > usage in linux.git.  Unfortunately, LLVM does not support them, and
> > barfs with:
> >
> >   error: ambiguous instructions require an explicit suffix
> >
> > Fix this problem by disambiguating it just like GNU as does, following
> > the example set by 824a907.
> >
> > Cc: Eli Friedman <eli.friedman at gmail.com>
> > Cc: Jim Grosbach <grosbach at apple.com>
> > Cc: Chris Lattner <sabre at nondot.org>
> > Signed-off-by: Ramkumar Ramachandra <artagnon at gmail.com>
> > ---
> >  FWIW, tests pass and clang goes past that point in linux.git.  Feel
> >  free to tweak the justification before applying (I couldn't think of
> >  anything else).
> >
> >  lib/Target/X86/X86InstrInfo.td | 4 ++++
> >  test/MC/X86/x86-64.s           | 6 ++++++
> >  2 files changed, 10 insertions(+)
> >
> > diff --git a/lib/Target/X86/X86InstrInfo.td b/lib/Target/X86/X86InstrInfo.td
> > index f33ae2a..d1b8d15 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", (BTR16mi8 i16mem:$mem, i16i8imm:$imm)>;
> > +def : InstAlias<"bts $imm, $mem", (BTS16mi8 i16mem:$mem, i16i8imm:$imm)>;
> > +
> >  // clr aliases.
> >  def : InstAlias<"clrb $reg", (XOR8rr  GR8 :$reg, GR8 :$reg)>;
> >  def : InstAlias<"clrw $reg", (XOR16rr GR16:$reg, GR16:$reg)>;
> > diff --git a/test/MC/X86/x86-64.s b/test/MC/X86/x86-64.s
> > index 521a077..9f89cda 100644
> > --- a/test/MC/X86/x86-64.s
> > +++ b/test/MC/X86/x86-64.s
> > @@ -698,6 +698,12 @@ movl       0, %eax   // CHECK: movl 0, %eax # encoding: [0x8b,0x04,0x25,0x00,0x00,0x00
> >  btl    $1, 0 // CHECK: btl $1, 0 # encoding: [0x0f,0xba,0x24,0x25,0x00,0x00,0x00,0x00,0x01]
> >  bt     $1, 0 // CHECK: btl $1, 0 # encoding: [0x0f,0xba,0x24,0x25,0x00,0x00,0x00,0x00,0x01]
> >
> > +// Disambiguate btr, bts
> > +btrw   $1, 0 // CHECK: btrw $1, 0 # encoding: [0x66,0x0f,0xba,0x34,0x25,0x00,0x00,0x00,0x00,0x01]
> > +btr    $1, 0 // CHECK: btrw $1, 0 # encoding: [0x66,0x0f,0xba,0x34,0x25,0x00,0x00,0x00,0x00,0x01]
> > +btsw   $1, 0 // CHECK: btsw $1, 0 # encoding: [0x66,0x0f,0xba,0x2c,0x25,0x00,0x00,0x00,0x00,0x01]
> > +bts    $1, 0 // CHECK: btsw $1, 0 # encoding: [0x66,0x0f,0xba,0x2c,0x25,0x00,0x00,0x00,0x00,0x01]
> 
> This doesn't match gas.

this has been discussed in the past already:

http://llvm.org/bugs/show_bug.cgi?id=9362
http://llvm.org/bugs/show_bug.cgi?id=9437

and the conclusion was to change the linux (user) side, not llvm.