New user self-registration is disabled due to spam. For an account please email bugs-admin@lists.llvm.org with your e-mail address and full name.

Bug 7352 - x86/llvm-mc missing instructions
Summary: x86/llvm-mc missing instructions
Status: RESOLVED FIXED
Alias: None
Product: libraries
Classification: Unclassified
Component: Backend: X86 (show other bugs)
Version: 1.0
Hardware: Macintosh All
: P normal
Assignee: Unassigned LLVM Bugs
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2010-06-10 18:34 PDT by Alexander Strange
Modified: 2010-09-08 00:02 PDT (History)
7 users (show)

See Also:
Fixed By Commit(s):


Attachments
testcase (403 bytes, application/octet-stream)
2010-06-10 18:34 PDT, Alexander Strange
Details
WIP Patch (2.16 KB, patch)
2010-06-24 00:04 PDT, Eli Friedman
Details
testcase for movsx/movzx (502 bytes, application/octet-stream)
2010-07-31 19:57 PDT, David Conrad
Details

Note You need to log in before you can comment on or make changes to this bug.
Description Alexander Strange 2010-06-10 18:34:59 PDT
Created attachment 5011 [details]
testcase

> /Xcode4/usr/bin/clang -v           
Apple clang version 2.0 (tags/Apple/clang-107.1)
Target: x86_64-apple-darwin10
Thread model: posix

Attached file has 5 inline asm instructions which compile with standard as but fail to assemble with clang/llvm-mc.

> gcc -c asmtests.c             
> clang -c asmtests.c
<inline asm>:1:2: error: unrecognized instruction
        add $1, 1(%esp) 
        ^
asmtests.c:4:19: note: generated from here
    asm volatile ("add $1, 1(%esp) \n");
                  ^
<inline asm>:1:9: error: unknown token in expression
        lea 1+(%eax), %ebx 
               ^
asmtests.c:10:19: note: generated from here
    asm volatile ("lea 1+(%eax), %ebx \n");
                  ^
<inline asm>:1:2: error: unrecognized instruction
        pavgusb %mm1, %mm0 
        ^
asmtests.c:16:19: note: generated from here
    asm volatile ("pavgusb %mm1, %mm0 \n");
                  ^
<inline asm>:1:2: error: unrecognized instruction
        jmp *1(%esp) 
        ^
asmtests.c:22:19: note: generated from here
    asm volatile ("jmp *1(%esp) \n");
                  ^
<inline asm>:1:2: error: unrecognized instruction
        prefetch (%eax) 
        ^
asmtests.c:28:19: note: generated from here
    asm volatile ("prefetch (%eax) \n");
                  ^
5 errors generated.

These are all required to compile FFmpeg.
Comment 1 Chris Lattner 2010-06-14 15:30:31 PDT
"add $1, 1(%esp) " is invalid.  You have to specify a width when using that form, as immediates and memory don't have an inherent size.  GAS accepts this as a bug.


I'm not familiar with "1+(%eax)" syntax.  That should be written as "1(%eax)".


pavgusb and jmp and prefetch seem like real mc bugs.
Comment 2 Daniel Dunbar 2010-06-14 15:52:24 PDT
With regard to:
  add $1, 1(%esp)
this is not matched by design. llvm-mc tries not to infer the operand width unless it is unambiguous based on a register name; you can use "addl $1, 1(%esp)" if you want to match the Darwin system assembler.

With regard to:
  lea 1+(%eax), %ebx
I have never seen this form before. I am not sure if Darwin 'as' is accepting it intentionally or unintentionally, but at this point we have run MC over lots of asm code and haven't seen this -- it's probably better to just change your code to:
  lea 1(%eax), %ebx
to match the more conventional usage.

Thanks for the reports on the other mismatches!
Comment 3 Daniel Dunbar 2010-06-14 15:52:52 PDT
Oops, Chris types faster than me! :)
Comment 4 Alexander Strange 2010-06-14 17:46:40 PDT
I agree about the add $1, 1(%esp) - I'll fix it upstream. It'd be nice if it had a specific warning, but that's not very important.

1+(%eax) is a form that only appears in inline asm. It appears in something like this:

struct s {
    int a;
    int b;
};

#define B_OFF "4"

void inc_b(struct s *s)
{
    asm ("incl "B_OFF"+%0" : "=m"(*s));
}

Since the compiler generates "(%eax)" instead of "0(%eax)" (and there's no way to force it to) you get 4+(%eax).
Darwin as didn't support this for a long time, so I managed to get people to stop writing it in ffmpeg, but it supports it now because I sent a patch for it. See radr://5828463.

Of course there are workarounds for this, but it ends up being convenient sometimes.
Comment 5 Chris Lattner 2010-06-14 20:39:27 PDT
After trying to implement this, I think that adding support for 4+(%eax) is really terrible.  There are good ways to write this directly, e.g.:

    asm ("incl %0" : "=m"(*((char*)s+ B_OFF));

and this causes the parser to have to disambiguate a case late (because 4+(5)(%eax) is legal.  Given that cctools didn't support it until very recently and this was only somewhat needed by ffmpeg, I don't think we should add it to the mc parser.
Comment 6 Eli Friedman 2010-06-24 00:04:57 PDT
Created attachment 5100 [details]
WIP Patch

Work in progress for 3DNow instructions.  I don't know the proper magic to make assembly and disassembly work correctly.  I believe I have the right encoding for prefetch/prefetchw; the others require some additional changes to express the encoding properly (two 0x0F prefix bytes and the opcode as a suffix).
Comment 7 David Conrad 2010-07-31 19:57:08 PDT
Created attachment 5305 [details]
testcase for movsx/movzx
Comment 8 David Conrad 2010-07-31 19:57:32 PDT
movsx r*,r* and movzx r16,r* are also missing. movzx r8,r* works, but it's not obvious to me why looking at the .td file.

I've attached a test case for those.
Comment 9 Chris Lattner 2010-07-31 21:06:01 PDT
Kevin, can you take a look at movsx/movzx when you get a chance?
Comment 10 Kevin Enderby 2010-08-02 13:37:26 PDT
(In reply to comment #9)
> Kevin, can you take a look at movsx/movzx when you get a chance?

I recall this problem with these instructions, it is also being tracked as rdar://8017633 here at Apple.

An incomplete fix was in r105005 in lib/Target/X86/AsmParser/X86AsmParser.cpp to add an alias:
       .Case("movzx", "movzb")
which is why the .td files don't tell the whole story on these.

But the thinking on this is to get llvm-mc to handle just "movzx" without any suffixes, and to do that right we may have to look at the register operands as the darwin assembler does to know if you have "movzwl" or "movzwq" for example.

The work around is to use the suffixes.  So for the "testcase for movsx/movzx" if it used instructions like this:

% cat z.s
	movzwl	%cx,%ecx
	movzwq	%cx,%rcx
	movsbw	%cl,%cx
	movsbl	%cl,%ecx
	movsbq	%cl,%rcx
	movswl	%cx,%ecx
	movswq	%cx,%rcx

with the explicit suffixes after the "movz" or "movs" to match the register operands, that will assemble correctly.
Comment 11 Chris Lattner 2010-09-08 00:02:03 PDT
Here's an update: the first ambiguous instruction now gets a good diagnostic:


t.s:1:1: error: ambiguous instructions require an explicit suffix (could be 'addb', 'addw', 'addl', or 'addq')
add $1, 1(%esp)
^

The 1+(%eax) example won't be supported.

The third one is movsx/movzx, which is tracked by PR 7459, so I'm closing this as done.
Comment 12 Chris Lattner 2010-09-08 00:02:38 PDT
If there are more missing instructions, please file a new bug with specific instructions that are missing, this bug is already cluttered with too many different things.