[llvm-commits] PATCH: Fix PR8528

[Chris Lattner]

On Nov 6, 2010, at 1:42 AM, İsmail Dönmez wrote:

> 
> 
> On Sat, Nov 6, 2010 at 10:37 AM, İsmail Dönmez <ismail at namtrac.org> wrote:
> Hi Chris;
> 
> On Sat, Nov 6, 2010 at 8:18 AM, Chris Lattner <clattner at apple.com> wrote:
> 
> On Nov 3, 2010, at 3:54 PM, İsmail Dönmez wrote:
> 
>> Hi;
>> 
>> Original patch by pdox on #llvm . Asm parser was not handling fist and fistp instructions correctly. Please apply.
> 
> Hi Ismail,
> 
> This is great detective work!  Just to verify my understanding, the issue is that GAS is treating:
>    fistp (%rax)
> as an alias for:
>   fistps (%rax)
> and we're compiling it to:
>   fistpl (%rax)
> 
> 
> You are correct.
>  
> If this is the case, I think that the MC assembler should be changed to *reject* fisp, not to emulate GAS's behavior.  With a simple mem operand, there is nothing to say that "4 bytes" is the right size of the store, it should be diagnosed as an ambiguous instruction.  Reporting it as a bug in the code is much more friendly than miscompiling it of course. :)
> 
> What do you think?
> 
> I am no assembly expert but this seems to be another vague gas behaviour. I guess rejecting this is the correct behaviour indeed.
> 
> Also there is an existing alias for fstp, should that be removed also?

Yes, you're completely right!  I fixed this in r118346, we now get:

t.s:1:1: error: ambiguous instructions require an explicit suffix (could be 'fstps', 'fstpl', or 'fstpt')
fstp	(%rax)
^
t.s:2:1: error: ambiguous instructions require an explicit suffix (could be 'fistps', or 'fistpl')
fistp (%rax)
^
t.s:3:1: error: ambiguous instructions require an explicit suffix (could be 'fists', or 'fistl')
fist (%rax)
^

Thanks again for tracking this down!

-Chris

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20101106/0b5039e2/attachment.html>