Bogus X86-64 Patterns

Previous Topic Next Topic
 
classic Classic list List threaded Threaded
7 messages Options
Reply | Threaded
Open this post in threaded view
|

Bogus X86-64 Patterns

dag-7
Tracking down a problem with one of our benchmark codes, we've discovered that
some of the patterns in X86InstrX86-64.td are wrong.  Specifically:

def MOV64toPQIrm : RPDI<0x6E, MRMSrcMem, (outs VR128:$dst), (ins i64mem:$src),
                        "mov{d|q}\t{$src, $dst|$dst, $src}",
                        [(set VR128:$dst,
                          (v2i64 (scalar_to_vector (loadi64 addr:$src))))]>;

def MOVPQIto64mr  : RPDI<0x7E, MRMDestMem, (outs), (ins i64mem:$dst, VR128:
$src),
                         "mov{d|q}\t{$src, $dst|$dst, $src}",
                         [(store (i64 (vector_extract (v2i64 VR128:$src),
                                       (iPTR 0))), addr:$dst)]>;

These say that for an AT&T-style assembler, output movd and for an Intel
assembler, output movq.

The problem is that such movs to and from memory must either use movq
or put a rex prefix before the movd.  No such rex prefix appears in these
patterns, meaning the instructions will move 32 bits rather than 64 bits.

Bad things happen.

A little sleuthing uncovered this:

$ svn log -r32609
~/svn/test/official.llvm/llvm/lib/Target/X86/X86InstrX86-64.td
------------------------------------------------------------------------
r32609 | evancheng | 2006-12-15 13:58:58 -0600 (Fri, 15 Dec 2006) | 2 lines

Some AT&T syntax assembler (e.g. Mac OS X) does not recognize the movq alias
for i64 <-> XMM moves.

------------------------------------------------------------------------
$ svn diff -r32608:32609
~/svn/test/official.llvm/llvm/lib/Target/X86/X86InstrX86-64.td
Index: /usr/people/djg/svn/test/official.llvm/llvm/lib/Target/X86/X86InstrX86-64.td
===================================================================
--- /usr/people/djg/svn/test/official.llvm/llvm/lib/Target/X86/X86InstrX86-64.td        
(revision 32608)
+++ /usr/people/djg/svn/test/official.llvm/llvm/lib/Target/X86/X86InstrX86-64.td        
(revision 32609)
@@ -1110,33 +1110,33 @@
 // Move instructions...
 
 def MOV64toPQIrr : RPDI<0x6E, MRMSrcReg, (ops VR128:$dst, GR64:$src),
-                        "movq {$src, $dst|$dst, $src}",
+                        "mov{d|q} {$src, $dst|$dst, $src}",
                         [(set VR128:$dst,
                           (v2i64 (scalar_to_vector GR64:$src)))]>;
 def MOV64toPQIrm : RPDI<0x6E, MRMSrcMem, (ops VR128:$dst, i64mem:$src),
-                        "movq {$src, $dst|$dst, $src}",
+                        "mov{d|q} {$src, $dst|$dst, $src}",
                         [(set VR128:$dst,
                           (v2i64 (scalar_to_vector (loadi64 addr:$src))))]>;
 
 def MOVPQIto64rr  : RPDI<0x7E, MRMDestReg, (ops GR64:$dst, VR128:$src),
-                         "movq {$src, $dst|$dst, $src}",
+                         "mov{d|q} {$src, $dst|$dst, $src}",
                          [(set GR64:$dst, (vector_extract (v2i64 VR128:$src),
                                            (iPTR 0)))]>;
 def MOVPQIto64mr  : RPDI<0x7E, MRMDestMem, (ops i64mem:$dst, VR128:$src),
-                         "movq {$src, $dst|$dst, $src}",
+                         "mov{d|q} {$src, $dst|$dst, $src}",
                          [(store (i64 (vector_extract (v2i64 VR128:$src),
                                        (iPTR 0))), addr:$dst)]>;
 
 def MOV64toSDrr : RPDI<0x6E, MRMSrcReg, (ops FR64:$dst, GR64:$src),
-                       "movq {$src, $dst|$dst, $src}",
+                       "mov{d|q} {$src, $dst|$dst, $src}",
                        [(set FR64:$dst, (bitconvert GR64:$src))]>;
 def MOV64toSDrm : RPDI<0x6E, MRMSrcMem, (ops FR64:$dst, i64mem:$src),
-                       "movq {$src, $dst|$dst, $src}",
+                       "mov{d|q} {$src, $dst|$dst, $src}",
                        [(set FR64:$dst, (bitconvert (loadi64 addr:$src)))]>;
 
 def MOVSDto64rr  : RPDI<0x7E, MRMDestReg, (ops GR64:$dst, FR64:$src),
-                        "movq {$src, $dst|$dst, $src}",
+                        "mov{d|q} {$src, $dst|$dst, $src}",
                         [(set GR64:$dst, (bitconvert FR64:$src))]>;
 def MOVSDto64mr  : RPDI<0x7E, MRMDestMem, (ops i64mem:$dst, FR64:$src),
-                        "movq {$src, $dst|$dst, $src}",
+                        "mov{d|q} {$src, $dst|$dst, $src}",
                         [(store (i64 (bitconvert FR64:$src)), addr:$dst)]>;

For the rr variants it is ok because the assembler sees the r register use and
adds the necessary rex prefix.  It cannot do so for memory moves.

These patterns are just wrong.  So either the MaxOS assembler adds a rex
(which is wrong if you really wanted 32 bits) or these patterns are broken on
MaxOS as well and the MacOS assembler really needs to be fixed.

We are changing these to movq in our copy of llvm.  Shall I send the changes
upstream?

                                               -Dave

_______________________________________________
LLVM Developers mailing list
[hidden email]         http://llvm.cs.uiuc.edu
http://lists.cs.uiuc.edu/mailman/listinfo/llvmdev
Reply | Threaded
Open this post in threaded view
|

Re: Bogus X86-64 Patterns

Evan Cheng-2

On Dec 12, 2007, at 2:10 PM, David Greene wrote:

> Tracking down a problem with one of our benchmark codes, we've  
> discovered that
> some of the patterns in X86InstrX86-64.td are wrong.  Specifically:
>
> def MOV64toPQIrm : RPDI<0x6E, MRMSrcMem, (outs VR128:$dst), (ins  
> i64mem:$src),
>                         "mov{d|q}\t{$src, $dst|$dst, $src}",
>                         [(set VR128:$dst,
>                           (v2i64 (scalar_to_vector (loadi64 addr:
> $src))))]>;
>
> def MOVPQIto64mr  : RPDI<0x7E, MRMDestMem, (outs), (ins i64mem:
> $dst, VR128:
> $src),
>                          "mov{d|q}\t{$src, $dst|$dst, $src}",
>                          [(store (i64 (vector_extract (v2i64 VR128:
> $src),
>                                        (iPTR 0))), addr:$dst)]>;
>
> These say that for an AT&T-style assembler, output movd and for an  
> Intel
> assembler, output movq.

Right.

>
> The problem is that such movs to and from memory must either use movq
> or put a rex prefix before the movd.  No such rex prefix appears in  
> these
> patterns, meaning the instructions will move 32 bits rather than 64  
> bits.

You mean there should be a "rex" in the assembly string? That is,

rex movd %rax, %xmm0

The Mac OS X assembly does not take rex prefix. So this is what's  
expected:
movd %rax, %xmm0

$ otool64 -t a.o
a.o:
(__TEXT,__text) section
00000000 6e0f4866 000000c0

>
> For the rr variants it is ok because the assembler sees the r  
> register use and
> adds the necessary rex prefix.  It cannot do so for memory moves.
>
> These patterns are just wrong.  So either the MaxOS assembler adds  
> a rex
> (which is wrong if you really wanted 32 bits) or these patterns are  
> broken on
> MaxOS as well and the MacOS assembler really needs to be fixed.

Hrm. Good point. Looks like the Mac OS X assembler is always adding  
rex.w:

movd %xmm0, (%eax)
$ otool64 -t a.o
a.o:
(__TEXT,__text) section
00000000 7e0f6667 00000000

I'll ping our assembler guy.

>
> We are changing these to movq in our copy of llvm.  Shall I send  
> the changes
> upstream?

Hold on, changing to movq will break it on Mac OS X. I'll need to  
find a proper solution.

Thanks,

Evan
>
>                                                -Dave
>
> _______________________________________________
> LLVM Developers mailing list
> [hidden email]         http://llvm.cs.uiuc.edu
> http://lists.cs.uiuc.edu/mailman/listinfo/llvmdev

_______________________________________________
LLVM Developers mailing list
[hidden email]         http://llvm.cs.uiuc.edu
http://lists.cs.uiuc.edu/mailman/listinfo/llvmdev
Reply | Threaded
Open this post in threaded view
|

Re: Bogus X86-64 Patterns

greened
On Wednesday 12 December 2007 06:16:25 pm Evan Cheng wrote:

> Hold on, changing to movq will break it on Mac OS X. I'll need to
> find a proper solution.
>
> Thanks,

The proper solution is to fix the assembler, but in the meantime,
the mr and rm variants at least should be changed to movq.  They
are broken on MacOS X right now anyway.  From what you've said,
one can't do a 32-bit memory mov to/from an xmm on MacOS X.
You still won't be able to do a 32-bit memory operation but at least
systems that use the latest binutils will work properly.

                                          -Dave
_______________________________________________
LLVM Developers mailing list
[hidden email]         http://llvm.cs.uiuc.edu
http://lists.cs.uiuc.edu/mailman/listinfo/llvmdev
Reply | Threaded
Open this post in threaded view
|

Re: Bogus X86-64 Patterns

Evan Cheng-2

On Dec 12, 2007, at 7:42 PM, David A. Greene wrote:

> On Wednesday 12 December 2007 06:16:25 pm Evan Cheng wrote:
>
>> Hold on, changing to movq will break it on Mac OS X. I'll need to
>> find a proper solution.
>>
>> Thanks,
>
> The proper solution is to fix the assembler, but in the meantime,
> the mr and rm variants at least should be changed to movq.  They
> are broken on MacOS X right now anyway.  From what you've said,
> one can't do a 32-bit memory mov to/from an xmm on MacOS X.
> You still won't be able to do a 32-bit memory operation but at least
> systems that use the latest binutils will work properly.

I should get an answer today. The right workaround now is to disable  
this for Darwin.

Thanks!

Evan

>
>                                           -Dave
> _______________________________________________
> LLVM Developers mailing list
> [hidden email]         http://llvm.cs.uiuc.edu
> http://lists.cs.uiuc.edu/mailman/listinfo/llvmdev

_______________________________________________
LLVM Developers mailing list
[hidden email]         http://llvm.cs.uiuc.edu
http://lists.cs.uiuc.edu/mailman/listinfo/llvmdev
Reply | Threaded
Open this post in threaded view
|

Re: Bogus X86-64 Patterns

Evan Cheng-2
Actually, movq is recognized by the assembler on Leopard. I've fixed  
this:
http://lists.cs.uiuc.edu/pipermail/llvm-commits/Week-of- 
Mon-20071210/056334.html

Please verify. Thanks.

Evan

On Dec 14, 2007, at 10:12 AM, Evan Cheng wrote:

>
> On Dec 12, 2007, at 7:42 PM, David A. Greene wrote:
>
>> On Wednesday 12 December 2007 06:16:25 pm Evan Cheng wrote:
>>
>>> Hold on, changing to movq will break it on Mac OS X. I'll need to
>>> find a proper solution.
>>>
>>> Thanks,
>>
>> The proper solution is to fix the assembler, but in the meantime,
>> the mr and rm variants at least should be changed to movq.  They
>> are broken on MacOS X right now anyway.  From what you've said,
>> one can't do a 32-bit memory mov to/from an xmm on MacOS X.
>> You still won't be able to do a 32-bit memory operation but at least
>> systems that use the latest binutils will work properly.
>
> I should get an answer today. The right workaround now is to disable
> this for Darwin.
>
> Thanks!
>
> Evan
>
>>
>>                                           -Dave
>> _______________________________________________
>> LLVM Developers mailing list
>> [hidden email]         http://llvm.cs.uiuc.edu
>> http://lists.cs.uiuc.edu/mailman/listinfo/llvmdev
>
> _______________________________________________
> LLVM Developers mailing list
> [hidden email]         http://llvm.cs.uiuc.edu
> http://lists.cs.uiuc.edu/mailman/listinfo/llvmdev

_______________________________________________
LLVM Developers mailing list
[hidden email]         http://llvm.cs.uiuc.edu
http://lists.cs.uiuc.edu/mailman/listinfo/llvmdev
Reply | Threaded
Open this post in threaded view
|

Re: Bogus X86-64 Patterns

dag-7
On Friday 14 December 2007 13:55, Evan Cheng wrote:
> Actually, movq is recognized by the assembler on Leopard. I've fixed
> this:
> http://lists.cs.uiuc.edu/pipermail/llvm-commits/Week-of-
> Mon-20071210/056334.html
>
> Please verify. Thanks.

The patch is similar to what I did.  I'll have to create an llvm testcase to
verify it as this is a Fortran code.  It would also be good to have a
regression test for this anyway.

Does the Leopard assembler support movq for rr instructions?  The
rep64 movd sequence is ugly and esoteric.  It's much more readable
to say movq.

                                           -Dave
_______________________________________________
LLVM Developers mailing list
[hidden email]         http://llvm.cs.uiuc.edu
http://lists.cs.uiuc.edu/mailman/listinfo/llvmdev
Reply | Threaded
Open this post in threaded view
|

Re: Bogus X86-64 Patterns

Evan Cheng-2

On Dec 17, 2007, at 10:04 AM, David Greene wrote:

> On Friday 14 December 2007 13:55, Evan Cheng wrote:
>> Actually, movq is recognized by the assembler on Leopard. I've fixed
>> this:
>> http://lists.cs.uiuc.edu/pipermail/llvm-commits/Week-of-
>> Mon-20071210/056334.html
>>
>> Please verify. Thanks.
>
> The patch is similar to what I did.  I'll have to create an llvm  
> testcase to
> verify it as this is a Fortran code.  It would also be good to have a
> regression test for this anyway.
>
> Does the Leopard assembler support movq for rr instructions?  The
> rep64 movd sequence is ugly and esoteric.  It's much more readable
> to say movq.

I agree. But unfortunately the assembler still does not recognize movq  
for rr instructions. If this really bug you I'd suggest adding a  
comment to the emitted assembly string.

Evan

>
>
>                                           -Dave
> _______________________________________________
> LLVM Developers mailing list
> [hidden email]         http://llvm.cs.uiuc.edu
> http://lists.cs.uiuc.edu/mailman/listinfo/llvmdev

_______________________________________________
LLVM Developers mailing list
[hidden email]         http://llvm.cs.uiuc.edu
http://lists.cs.uiuc.edu/mailman/listinfo/llvmdev