diff mbox series

[v2] x86: {,v}psadbw have commutative source operands

Message ID f24c974c-e25b-30d0-dbf7-59a5ac60b361@suse.com
State New
Headers show
Series [v2] x86: {,v}psadbw have commutative source operands | expand

Commit Message

Jan Beulich June 2, 2022, 3 p.m. UTC
Like noticed for gas as well (binutils-gdb commit c8cad9d389b7), the
"absolute difference" aspect of the insns makes their source operands
commutative.

gcc/

	* config/i386/mmx.md (mmx_psadbw): Convert to expander.
	(*mmx_psadbw): New. Mark as commutative.
	* config/i386/sse.md (<sse2_avx2>_psadbw): Convert to expander.
	(*<sse2_avx2>_psadbw): New. Mark as commutative.
---
v2: Introduce expanders.

Comments

Uros Bizjak June 2, 2022, 8:46 p.m. UTC | #1
On Thu, Jun 2, 2022 at 5:00 PM Jan Beulich <jbeulich@suse.com> wrote:
>
> Like noticed for gas as well (binutils-gdb commit c8cad9d389b7), the
> "absolute difference" aspect of the insns makes their source operands
> commutative.
>
> gcc/
>
>         * config/i386/mmx.md (mmx_psadbw): Convert to expander.
>         (*mmx_psadbw): New. Mark as commutative.
>         * config/i386/sse.md (<sse2_avx2>_psadbw): Convert to expander.
>         (*<sse2_avx2>_psadbw): New. Mark as commutative.

OK with a nit below.

Thanks,
Uros.

> ---
> v2: Introduce expanders.
>
> --- a/gcc/config/i386/mmx.md
> +++ b/gcc/config/i386/mmx.md
> @@ -4405,13 +4405,21 @@
>     (set_attr "type" "sseiadd")
>     (set_attr "mode" "TI")])
>
> -(define_insn "mmx_psadbw"
> +(define_expand "mmx_psadbw"
> +  [(set (match_operand:V1DI 0 "register_operand")
> +       (unspec:V1DI [(match_operand:V8QI 1 "register_mmxmem_operand")
> +                     (match_operand:V8QI 2 "register_mmxmem_operand")]
> +                    UNSPEC_PSADBW))]
> +  "(TARGET_MMX || TARGET_MMX_WITH_SSE) && (TARGET_SSE || TARGET_3DNOW_A)"
> +  "ix86_fixup_binary_operands_no_copy (PLUS, V8QImode, operands);")
> +
> +(define_insn "*mmx_psadbw"
>    [(set (match_operand:V1DI 0 "register_operand" "=y,x,Yw")
> -        (unspec:V1DI [(match_operand:V8QI 1 "register_operand" "0,0,Yw")
> +       (unspec:V1DI [(match_operand:V8QI 1 "register_mmxmem_operand" "%0,0,Yw")
>                       (match_operand:V8QI 2 "register_mmxmem_operand" "ym,x,Yw")]
>                      UNSPEC_PSADBW))]
> -  "(TARGET_MMX || TARGET_MMX_WITH_SSE)
> -   && (TARGET_SSE || TARGET_3DNOW_A)"
> +  "(TARGET_MMX || TARGET_MMX_WITH_SSE) && (TARGET_SSE || TARGET_3DNOW_A)
> +   && ix86_binary_operator_ok (PLUS, V8QImode, operands)"
>    "@
>     psadbw\t{%2, %0|%0, %2}
>     psadbw\t{%2, %0|%0, %2}
> --- a/gcc/config/i386/sse.md
> +++ b/gcc/config/i386/sse.md
> @@ -19981,13 +19981,22 @@
>
>  ;; The correct representation for this is absolutely enormous, and
>  ;; surely not generally useful.
> -(define_insn "<sse2_avx2>_psadbw"
> +(define_expand "<sse2_avx2>_psadbw"
> +  [(set (match_operand:VI8_AVX2_AVX512BW 0 "register_operand")
> +       (unspec:VI8_AVX2_AVX512BW
> +         [(match_operand:<ssebytemode> 1 "vector_operand")
> +          (match_operand:<ssebytemode> 2 "vector_operand")]
> +         UNSPEC_PSADBW))]
> +  "TARGET_SSE2"
> +  "ix86_fixup_binary_operands_no_copy (PLUS, <ssebytemode>mode, operands);")
> +
> +(define_insn "*<sse2_avx2>_psadbw"
>    [(set (match_operand:VI8_AVX2_AVX512BW 0 "register_operand" "=x,YW")
>         (unspec:VI8_AVX2_AVX512BW
> -         [(match_operand:<ssebytemode> 1 "register_operand" "0,YW")
> +         [(match_operand:<ssebytemode> 1 "vector_operand" "%0,YW")
>            (match_operand:<ssebytemode> 2 "vector_operand" "xBm,YWm")]
>           UNSPEC_PSADBW))]
> -  "TARGET_SSE2"
> +  "TARGET_SSE2 && ix86_binary_operator_ok (PLUS, <ssebytemode>mode, operands)"

Please put && ix86_binary_operator_ok to a separate line.

>    "@
>     psadbw\t{%2, %0|%0, %2}
>     vpsadbw\t{%2, %1, %0|%0, %1, %2}"
>
diff mbox series

Patch

--- a/gcc/config/i386/mmx.md
+++ b/gcc/config/i386/mmx.md
@@ -4405,13 +4405,21 @@ 
    (set_attr "type" "sseiadd")
    (set_attr "mode" "TI")])
 
-(define_insn "mmx_psadbw"
+(define_expand "mmx_psadbw"
+  [(set (match_operand:V1DI 0 "register_operand")
+	(unspec:V1DI [(match_operand:V8QI 1 "register_mmxmem_operand")
+		      (match_operand:V8QI 2 "register_mmxmem_operand")]
+		     UNSPEC_PSADBW))]
+  "(TARGET_MMX || TARGET_MMX_WITH_SSE) && (TARGET_SSE || TARGET_3DNOW_A)"
+  "ix86_fixup_binary_operands_no_copy (PLUS, V8QImode, operands);")
+
+(define_insn "*mmx_psadbw"
   [(set (match_operand:V1DI 0 "register_operand" "=y,x,Yw")
-        (unspec:V1DI [(match_operand:V8QI 1 "register_operand" "0,0,Yw")
+	(unspec:V1DI [(match_operand:V8QI 1 "register_mmxmem_operand" "%0,0,Yw")
 		      (match_operand:V8QI 2 "register_mmxmem_operand" "ym,x,Yw")]
 		     UNSPEC_PSADBW))]
-  "(TARGET_MMX || TARGET_MMX_WITH_SSE)
-   && (TARGET_SSE || TARGET_3DNOW_A)"
+  "(TARGET_MMX || TARGET_MMX_WITH_SSE) && (TARGET_SSE || TARGET_3DNOW_A)
+   && ix86_binary_operator_ok (PLUS, V8QImode, operands)"
   "@
    psadbw\t{%2, %0|%0, %2}
    psadbw\t{%2, %0|%0, %2}
--- a/gcc/config/i386/sse.md
+++ b/gcc/config/i386/sse.md
@@ -19981,13 +19981,22 @@ 
 
 ;; The correct representation for this is absolutely enormous, and
 ;; surely not generally useful.
-(define_insn "<sse2_avx2>_psadbw"
+(define_expand "<sse2_avx2>_psadbw"
+  [(set (match_operand:VI8_AVX2_AVX512BW 0 "register_operand")
+	(unspec:VI8_AVX2_AVX512BW
+	  [(match_operand:<ssebytemode> 1 "vector_operand")
+	   (match_operand:<ssebytemode> 2 "vector_operand")]
+	  UNSPEC_PSADBW))]
+  "TARGET_SSE2"
+  "ix86_fixup_binary_operands_no_copy (PLUS, <ssebytemode>mode, operands);")
+
+(define_insn "*<sse2_avx2>_psadbw"
   [(set (match_operand:VI8_AVX2_AVX512BW 0 "register_operand" "=x,YW")
 	(unspec:VI8_AVX2_AVX512BW
-	  [(match_operand:<ssebytemode> 1 "register_operand" "0,YW")
+	  [(match_operand:<ssebytemode> 1 "vector_operand" "%0,YW")
 	   (match_operand:<ssebytemode> 2 "vector_operand" "xBm,YWm")]
 	  UNSPEC_PSADBW))]
-  "TARGET_SSE2"
+  "TARGET_SSE2 && ix86_binary_operator_ok (PLUS, <ssebytemode>mode, operands)"
   "@
    psadbw\t{%2, %0|%0, %2}
    vpsadbw\t{%2, %1, %0|%0, %1, %2}"