Re: PATCH: Support -mmovbe

[Uros Bizjak <ubizjak at gmail dot com>]

H.J. Lu wrote:

> > > > +(define_insn "*movbedi2"
> > > > + ï[(set (match_operand:DI 0 "nonimmediate_operand" "=r,m")
> > > > + ï ï ï (bswap:DI (match_operand:DI 1 "nonimmediate_operand" "m,r")))]
> > > > + ï"TARGET_MOVBE && TARGET_64BIT"
> > > > + ï"movbe\t{%1, %0|%0, %1}"
> > > > + ï[(set_attr "type" "imov")
> > > > + ï (set_attr "modrm" "1")
> > > > + ï (set_attr "prefix_0f" "1")
> > > > + ï (set_attr "prefix_extra" "1")
> > > > + ï (set_attr "mode" "DI")])
> > > >         
> > > Also the pattern above; it should be integrated with existing bswapdi2.
> > >       
> > movbe doesn't support "movbe reg, reg".
> >     

Sure, but attached patch automatically emits bswap in this case.

> It is much cleaner to have a separate pattern since expander is
>   

The constraints on expanders are actually not that important. The 
constraints on the insns pattern operands are what matters, and combine 
does a great job to merge memory operands whenever it can.
> (define_expand "bswapsi2"
>   [(set (match_operand:SI 0 "register_operand" "")
>         (bswap:SI (match_operand:SI 1 "register_operand" "")))]
>
> The new movbe pattern will be used optimizer.
>   

Please see attached i386.md part of the patch. The patch assumes that 
TARGET_MOVBE also supports bswap insn, so following part is not needed 
anymore:

+  /* Enable bswap if movbe is enabled.  */
+  if (TARGET_MOVBE)
+    TARGET_BSWAP = 1;
+


BTW: the patch uses shadowing to select between two patterns.

Uros.

Index: i386.md
===================================================================
--- i386.md	(revision 147770)
+++ i386.md	(working copy)
@@ -16105,7 +16105,7 @@
 	(bswap:SI (match_operand:SI 1 "register_operand" "")))]
   ""
 {
-  if (!TARGET_BSWAP)
+  if (!(TARGET_BSWAP || TARGET_MOVBE))
     {
       rtx x = operands[0];
 
@@ -16117,6 +16117,21 @@
     }
 })
 
+(define_insn "*bswapsi_movbe"
+  [(set (match_operand:SI 0 "nonimmediate_operand" "=r,r,m")
+	(bswap:SI (match_operand:SI 1 "nonimmediate_operand" "0,m,r")))]
+  "TARGET_MOVBE && !(MEM_P (operands[0]) && MEM_P (operands[1]))"
+  "@
+    bswap\t%0
+    movbe\t{%1, %0|%0, %1}
+    movbe\t{%1, %0|%0, %1}"
+  [(set_attr "type" "*,imov,imov")
+   (set_attr "modrm" "*,1,1")
+   (set_attr "prefix_0f" "1")
+   (set_attr "prefix_extra" "*,1,1")
+   (set_attr "length" "2,*,*")
+   (set_attr "mode" "SI")])
+
 (define_insn "*bswapsi_1"
   [(set (match_operand:SI 0 "register_operand" "=r")
 	(bswap:SI (match_operand:SI 1 "register_operand" "0")))]
@@ -16145,10 +16160,32 @@
   [(set_attr "length" "4")
    (set_attr "mode" "HI")])
 
-(define_insn "bswapdi2"
+(define_expand "bswapdi2"
+  [(set (match_operand:DI 0 "register_operand" "")
+	(bswap:DI (match_operand:DI 1 "register_operand" "")))]
+  "TARGET_64BIT && (TARGET_BSWAP || TARGET_MOVBE)"
+  "")
+
+(define_insn "*bswapdi_movbe"
+  [(set (match_operand:DI 0 "nonimmediate_operand" "=r,r,m")
+	(bswap:DI (match_operand:DI 1 "nonimmediate_operand" "0,m,r")))]
+  "TARGET_64BIT && TARGET_MOVBE
+   && !(MEM_P (operands[0]) && MEM_P (operands[1]))"
+  "@
+    bswap\t%0
+    movbe\t{%1, %0|%0, %1}
+    movbe\t{%1, %0|%0, %1}"
+  [(set_attr "type" "*,imov,imov")
+   (set_attr "modrm" "*,1,1")
+   (set_attr "prefix_0f" "1")
+   (set_attr "prefix_extra" "*,1,1")
+   (set_attr "length" "3,*,*")
+   (set_attr "mode" "DI")])
+
+(define_insn "*bswapdi_1"
   [(set (match_operand:DI 0 "register_operand" "=r")
 	(bswap:DI (match_operand:DI 1 "register_operand" "0")))]
-  "TARGET_64BIT"
+  "TARGET_64BIT && TARGET_BSWAP"
   "bswap\t%0"
   [(set_attr "prefix_0f" "1")
    (set_attr "length" "3")])