[LLVMdev] Bogus X86-64 Patterns

[David Greene]

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