[PATCH] x86/asm: avoid mnemonics without type suffix

1c54d77 (x86: partial unification of asm-x86/bitops.h, 2008-01-30)
changed a bunch of btrl/btsl instructions to btr/bts, with the following
justification:

  The inline assembly for the bit operations has been changed to remove
  explicit sizing hints on the instructions, so the assembler will pick
  the appropriate instruction forms depending on the architecture and
  the context.

Unfortunately, GNU as does no such thing, and the AT&T syntax manual
[1] contains no references to any such inference. As evidenced by the
following experiment, gas always disambiguates btr/bts to btrl/btsl.
Feed the following input to gas:

  btrl $1, 0
  btr $1, 0
  btsl $1, 0
  bts $1, 0

Check that btr matches btrl, and bts matches btsl in both cases:

  $ as --32 -a in.s
  $ as --64 -a in.s

To avoid giving readers the illusion of such an inference, and for
clarity, change btr/bts back to btrl/btsl. Also, llvm-mc refuses to
disambiguate btr/bts automatically.

[1]: http://docs.oracle.com/cd/E19253-01/817-5477/817-5477.pdf

1c54d77 (x86: partial unification of asm-x86/bitops.h, 2008-01-30)
changed a bunch of btrl/btsl instructions to btr/bts, with the following
justification:

  The inline assembly for the bit operations has been changed to remove
  explicit sizing hints on the instructions, so the assembler will pick
  the appropriate instruction forms depending on the architecture and
  the context.

Unfortunately, GNU as does no such thing

Yes it does.

  btrl $1, 0
  btr $1, 0
  btsl $1, 0
  bts $1, 0

What the heck is that supposed to show? It shows nothing at all. With
an argument of '1', *of*course* gas will use "btsl", since that's the
short form. Using the rex-predix and a btsq would be *stupid*.

So gas will pick the appropriate form, exactly as claimed.

Try some actual relevant test instead:

   bt %eax,mem
   bt %rax,mem

and notice how they are actually fundamentally different. Test-case:

int main(int argc, char **argv)
{
  asm("bt %1,%0":"=m" (**argv): "a" (argc));
  asm("bt %1,%0":"=m" (**argv): "a" ((unsigned long)(argc)));
}

and I get

   0f a3 02 bt %eax,(%rdx)
   48 0f a3 02 bt %rax,(%rdx)

exactly as expected and wanted.

Now, there are possible cases where you want to make the size explicit
because you are mixing memory operand sizes and there can be nasty
performance implications of doing a 32-bit write and then doing a
64-bit read of the result. I'm not actually aware of us having ever
worried/cared about it, but it's a possible source of trouble: mixing
bitop instructions with non-bitop instructions can have some subtle
interactions, and you need to be careful, since the size of the
operand affects both the offset *and* the memory access size. The
access size generally is meaningless from a semantic standpoint
(little-endian being the only sane model), but the access size *can*
have performance implications for the write queue forwarding.

                      Linus

Linus Torvalds wrote:

  btrl $1, 0
  btr $1, 0
  btsl $1, 0
  bts $1, 0

What the heck is that supposed to show?

I was trying to show a reduced case where gas doesn't complain, but
llvm-mc does. Try compiling this with llvm-mc, and you'll get:

        .text
        btrl $1, 0
in.s:2:1: error: ambiguous instructions require an explicit suffix
      (could be 'btrw', 'btrl', or 'btrq')
btr $1, 0
^
        btsl $1, 0
in.s:4:1: error: ambiguous instructions require an explicit suffix
      (could be 'btsw', 'btsl', or 'btsq')
bts $1, 0
^

Obviously, I misunderstood something major and screwed up the commit message.

int main(int argc, char **argv)
{
  asm("bt %1,%0":"=m" (**argv): "a" (argc));
  asm("bt %1,%0":"=m" (**argv): "a" ((unsigned long)(argc)));
}

Right, so in:

int main(int argc, char **argv)
{
  asm("bts %1,%0":"=m" (**argv): "r" (argc));
  asm("btsl %1,%0":"=m" (**argv): "r" (argc));
  asm("btr %1,%0":"=m" (**argv): "r" ((unsigned long)(argc)));
  asm("btrq %1,%0":"=m" (**argv): "r" ((unsigned long)(argc)));
}

bts disambiguates to btsl, and btr disambiguates to btrq, as
advertised. Is it dependent on whether I have a 32-bit machine or
64-bit machine, or just on the operand lengths?

Either way, this is not a very enlightening example, because clang
also compiles this fine, and doesn't complain about any ambiguity. To
see the ambiguity I'm talking about, try to compile linux.git with
clang; I'll paste one error:

arch/x86/include/asm/bitops.h:129:15: error: ambiguous instructions
      require an explicit suffix (could be 'btrw', 'btrl', or 'btrq')
        asm volatile("btr %1,%0" : ADDR : "Ir" (nr));
                     ^
<inline asm>:1:2: note: instantiated into assembly here
        btr $0,(%rsi)
        ^

Since nr is an int, and ADDR is *(volatile long *), this should
disambiguate to btrl, right? Any clue why clang is complaining?

Ok. So your commit message and explanation was pure and utter tripe,
and the real reason you want this is that llvm-mc is broken.

Please fix llvm-mc instead, ok? If the intent of llvm is to be
compatible with the gnu compiler tools, then it should do that. Plus
the gas behavior is clearly superior, so why not just improve the llvm
toolchain to match those improved semantics?

                Linus

Hi,

The issue perhaps wasn't explained ideally (and possibly shouldn't
have been CCed directly to you either, so apologies, but now that
there *is* a discussion...)

Try some actual relevant test instead:

   bt %eax,mem
   bt %rax,mem

and notice how they are actually fundamentally different. Test-case:

I'm coming at this from the compiler side, where the register form is
unambiguous and not questioned. The discussion we're having involves
only the immediate form of the instruction. GNU as interprets:

    bt $63, mem

as
    btl $63, mem

which may or may not be what the user intended, but is not the same as
"btq $63, mem".

I'm not an official LLVM spokesperson or anything, but our consensus
seems to be that "bt $imm, whatever" is ambiguous (the %eax and %rax
versions you quoted disambiguate the width) and should be disallowed
by the assembler.

The patch we're replying to implements that as a NOP fix to the kernel
(GNU as always treats "bt" with an immediate as "btl"). I don't
believe there's any situation in which it will produce different code,
but it will allow Clang to compile (this part of) the kernel.

There is, however, a potential optimisation here for someone who knows
their inline asm. Currently "set_bit(63, addr)" will use the "r"
version of the constraint even on amd64 targets, materialising 63 with
a "movl". With sufficiently clever faff, it could use "btsq" instead.

Cheers.

Tim.

Linus Torvalds wrote:

Ok. So your commit message and explanation was pure and utter tripe,
and the real reason you want this is that llvm-mc is broken.

Please fix llvm-mc instead, ok? If the intent of llvm is to be
compatible with the gnu compiler tools, then it should do that. Plus
the gas behavior is clearly superior, so why not just improve the llvm
toolchain to match those improved semantics?

Yep. I started the discussion on LLVMDev, and posted patches [1].

From the discussions on the list, many of the devs are claiming that

LLVM is "correct" and that linux.git needs to be patched. I'm not
taking sides; I just want a solution to the problem.

[1]: The archive is broken, but here are some pieces:
http://lists.cs.uiuc.edu/pipermail/llvm-commits/Week-of-Mon-20130708/180968.html

Umm. The user doesn't care. The user wants the best code without
having to worry about it.

Think of it this way: the whole and ONLY point of an assembler is to
make machine code reasonably easy to write, by not having to worry
about the exact encoding details. We don't want the users specifying
the hex representation of the instructions, do we? Or even details
like "what is the most efficient form of this instruction". For
example, think about branch offsets and immediates. Most architectures
have some limits about how long branch offsets or immediates are, and
a short branch offset may use TOTALLY DIFFERENT instruction encoding
than a long branch offset.

Do you really expect that the user says "jnel" for the long form of
the "jne" instruction? And "jnes" if you want the
smaller/faster/simpler 8-bit version?

No sane person actually wants that, and no modern assembler does that
(although I can remember ones that did - ugh). You write "jne target"
and depend on the assembler doing the right thing. Or you write "add
$5,%eax", and depend on the fact that the assembler will use the much
shorter version of the "add" instruction that just takes a 8-bit
signed value instead of the full 32-bit immediate. Or any number of
details like this ("there are special versions that only work on %eax"
etc rules)

And that is why I think you should just consider "bt $x,y" to be
trivially the same thing and not at all ambiguous. Because there is
ABSOLUTELY ZERO ambiguity when people write

   bt $63, mem

Zero. Nada. None. The semantics are *exactly* the same for btl and btq
in this case, so why would you want the user to specify one or the
other? The user may be knowledgeable about the architecture, and know
that "btl" is one byte shorter than "btq", and use "btl" for that
reason. You seem to argue that that is the "right thing"(tm) to do,
since that's what the instruction encoding will be. But if that's the
case, then you are arguing that "jne target" is "ambiguous" because
there are two different ways to encode that too? Do you seriously
argue that?

So I'm arguing that that is wrong for an assembler to not just do the
right thing, because the user isn't *supposed* to have to know about
things like "one byte shorter encoding format". And there really is no
semantic difference between the two forms. So making the user specify
the size is just going to cause problems (in particular, it might well
make users go "this is an array of 64-bit entities, so I should use
btq", even though that is actually incorrect).

Now, I obviously think that the user should have the choice to
*override* the default thing, so sometimes you might have

   /* We use a 64-bit btsq to encourage the CPU to do it as a 64-bit
read-modify-write, since we will do a 64-bit read of the result later,
and otherwise we'll get a partial write buffer stall */

   btsq $63, mem

and then the assembler had obviously better use the size information
the user gave it. But the thing is, this is basically never a concern
in practice, and when it is, the assembler really cannot know (it
could go either way: maybe the bts is following a 32-bit write, and
you want the 32-bit version - and I suspect that the likelihood of
most users getting this right by hand is quite low too).

(Side note: I'm not even going to guarantee that the actual CPU uses
the operand size for the memory access size. The manuals imply they
do, but since there are no real semantic reasons to enforce that, I
could imagine that some microarchitecture doesn't actually care).

                    Linus

When I originally did those patches, I was careful make sure that we didn’t give implied sizes to operations with only immediate and/or memory operands because - in general - gas can’t infer the operation size from such operands. However, in the case of the bit test/set operations, the memory access size is not really derived from the operation size (the SDM is a bit vague), and even if it were it would be an operation rather than semantic difference. So there’s no real problem with gas choosing ‘l’ as a default size in the absence of any explicit override or constraint. That sounds reasonable for all other operations because it makes a real semantic difference, but overly strict for bit operations.

The SDM entry for BT mentions that the instruction may touch 2 or 4 bytes depending on the operand size, but doesn’t specifically mention that a 64 bit operation size touches 8 bytes - and it doesn’t mention anything at all about operand size and access size in BTR/BTS/BTC (unless it’s implied as part of the discussion about encoding the MSBs of a constant bit offset in the offset of the addressing mode). Is that an oversight? It looks like that if the base address isn’t aligned then neither is the generated access, so you could get a protection fault if it overlaps a page boundary, which is a semantic rather than purely operational difference.

(resent without HTML)

1c54d77 (x86: partial unification of asm-x86/bitops.h, 2008-01-30)
changed a bunch of btrl/btsl instructions to btr/bts, with the following
justification:

  The inline assembly for the bit operations has been changed to remove
  explicit sizing hints on the instructions, so the assembler will pick
  the appropriate instruction forms depending on the architecture and
  the context.

Unfortunately, GNU as does no such thing, and the AT&T syntax manual
[1] contains no references to any such inference. As evidenced by the
following experiment, gas always disambiguates btr/bts to btrl/btsl.
Feed the following input to gas:

  btrl $1, 0
  btr $1, 0
  btsl $1, 0
  bts $1, 0

When I originally did those patches, I was careful make sure that we
didn't give implied sizes to operations with only immediate and/or
memory operands because - in general - gas can't infer the operation
size from such operands. However, in the case of the bit test/set
operations, the memory access size is not really derived from the
operation size (the SDM is a bit vague), and even if it were it would be
an operation rather than semantic difference. So there's no real
problem with gas choosing 'l' as a default size in the absence of any
explicit override or constraint.

Check that btr matches btrl, and bts matches btsl in both cases:

  $ as --32 -a in.s
  $ as --64 -a in.s

To avoid giving readers the illusion of such an inference, and for
clarity, change btr/bts back to btrl/btsl. Also, llvm-mc refuses to
disambiguate btr/bts automatically.

That sounds reasonable for all other operations because it makes a real
semantic difference, but overly strict for bit operations.

    J

(Resent without HTML)

Now, there are possible cases where you want to make the size explicit
because you are mixing memory operand sizes and there can be nasty
performance implications of doing a 32-bit write and then doing a
64-bit read of the result. I'm not actually aware of us having ever
worried/cared about it, but it's a possible source of trouble: mixing
bitop instructions with non-bitop instructions can have some subtle
interactions, and you need to be careful, since the size of the
operand affects both the offset *and* the memory access size.

The SDM entry for BT mentions that the instruction may touch 2 or 4
bytes depending on the operand size, but doesn't specifically mention
that a 64 bit operation size touches 8 bytes - and it doesn't mention
anything at all about operand size and access size in BTR/BTS/BTC
(unless it's implied as part of the discussion about encoding the MSBs
of a constant bit offset in the offset of the addressing mode). Is that
an oversight?

The
access size generally is meaningless from a semantic standpoint
(little-endian being the only sane model), but the access size *can*
have performance implications for the write queue forwarding.

It looks like that if the base address isn't aligned then neither is the
generated access, so you could get a protection fault if it overlaps a
page boundary, which is a semantic rather than purely operational
difference.

    J

You could also get AC fault for the btq if the thing is only long-aligned.

But yes, I checked the Intel manuals too, and the access size is
actually not well-specified (even the 16-bit case says "may", I
think), so both the page-fault and the alignment fault are purely
theoretical. And i'm too lazy to bother trying the (easily testable)
alignment fault case in practice, since (a) nobody cares and (b)
nobody cares.

In the (unlikely) situation that somebody actually cares, that
somebody should obviously then have to specify "btl" vs "btq".
Assuming the hardware cares, which is testable but might be
micro-architecture dependent.

                         Linus

And that is why I think you should just consider "bt $x,y" to be
trivially the same thing and not at all ambiguous. Because there is
ABSOLUTELY ZERO ambiguity when people write

   bt $63, mem

Zero. Nada. None. The semantics are *exactly* the same for btl and btq
in this case, so why would you want the user to specify one or the
other?

I don't think you've actually tested that, have you? (x86-64)

int main() {
  long val = 0xffffffff;
  char res;

  asm("btl $63, %1\n\tsetc %0" : "=r"(res) : "m"(val));
  printf("%d\n", res);

  asm("btq $63, %1\n\tsetc %0" : "=r"(res) : "m"(val));
  printf("%d\n", res);
}

Tim.

And that is why I think you should just consider "bt $x,y" to be
trivially the same thing and not at all ambiguous. Because there is
ABSOLUTELY ZERO ambiguity when people write

   bt $63, mem

Zero. Nada. None. The semantics are *exactly* the same for btl and btq
in this case, so why would you want the user to specify one or the
other?

I don't think you've actually tested that, have you? (x86-64)

int main() {
  long val = 0xffffffff;
  char res;

  asm("btl $63, %1\n\tsetc %0" : "=r"(res) : "m"(val));
  printf("%d\n", res);

  asm("btq $63, %1\n\tsetc %0" : "=r"(res) : "m"(val));
  printf("%d\n", res);
}

Blerk. It doesn't undermine the original point - that gas can
unambiguously choose the right operation size for a constant bit offset
- but yes, the operation size is meaningful in the case of a immediate
bit offset. Its pretty nasty of Intel to hide that detail in Table 3-2,
far from the instructions which use it...

    J

Oh, you're right, for constants > 5 bits you have that other thing
going on. I didn't think about the fact that the constant changed in
the middle of the thread (it started out as 1).

We use the gcc constraint "I" (0-31) in the kernel for this reason.

             Linus

I think best would be to just find some way to implement LOCK prefix
patching using atomic compiler intrinsics and then switch to those

Then all this inline assembler horror could be ifdef'ed away
for old compilers only, and likely the generated code would
be better as the compiler could optimize more.

Or just give up on LOCK patching, as single CPU systems
and VMs are less and less interesting?

-Andi

This is also why the Intel manuals point out that "some assemblers" can
take things like:

  bt[l] $63,(%rsi)

... and turn it into:

  btl $31,4(%rsi)

This is definitely the friendly thing to do toward the human programmer.
Unfortunately gas doesn't, nor does e.g. NASM.

  -hpa

Most likely. I'll check with the people responsible for the SDM here at
Intel.

  -hpa

That will take a long time to make happen now, and then when we change
things we have to wait for gcc to catch up. At some point this always
degenerates into the "do we need a kernel-specific compiler" morass.

  -hpa

To be fair, we *ought to* be able to do something like:

  asm volatile(LOCK_PREFIX "bts%z0 %1,%0"
       : BITOP_ADDR(addr) : "Ir" (nr) : "memory");

... but some older version of gcc are broken and emit "ll" rather than
"q". Furthermore, since that would actually result in *worse* code
emitted overall (unnecessary REX prefixes), I'm not exactly happy on the
idea.

  -hpa