public inbox for gcc-patches@gcc.gnu.org
 help / color / mirror / Atom feed
* [PATCH i386 AVX512] [56/n] Add plus/minus/abs/neg/andnot insn patterns.
@ 2014-09-25 14:12 Kirill Yukhin
  2014-09-25 18:12 ` Uros Bizjak
  0 siblings, 1 reply; 8+ messages in thread
From: Kirill Yukhin @ 2014-09-25 14:12 UTC (permalink / raw)
  To: Uros Bizjak; +Cc: Jakub Jelinek, Richard Henderson, GCC Patches, kirill.yukhin

Hello,
Patch in the bottom extends plus/minus/abs/andnot patterns
to support AVX-512.
I've used questionable hack in the patterns.
Instead of writing dozen similar patterns with masking
I've simply substed them, prohibiting non-mask variant in
the pattern condition. E.g.:
(define_expand "<mask_codefor><plusminus_insn><mode>3<mask_name>"
  [(set (match_operand:VI12_AVX512VL 0 "register_operand")
       (plusminus:VI12_AVX512VL
	  (match_operand:VI12_AVX512VL 1 "nonimmediate_operand")
	  (match_operand:VI12_AVX512VL 2 "nonimmediate_operand")))]
  "TARGET_AVX512BW && <mask_applied>"
  "ix86_fixup_binary_operands_no_copy (<CODE>, <MODE>mode, operands);")

If this is not acceptable, I'll rewrite it to somthing like:
(define_expand "<mask_codefor><plusminus_insn><mode>3<mask_name>"
  [(set (match_operand:VI12_AVX512VL 0 "register_operand")
	(vec_merge: VI12_AVX512VL
	  (plusminus:VI12_AVX512VL
	    (match_operand:VI12_AVX512VL 1 "nonimmediate_operand")
	    (match_operand:VI12_AVX512VL 2 "nonimmediate_operand"))
	  (match_operand:SUBST_V 2 "vector_move_operand" "0C")
	  (match_operand:<avx512fmaskmode> 3 "register_operand" "Yk")))]
  "TARGET_AVX512BW && <mask_applied>"
  "ix86_fixup_binary_operands_no_copy (<CODE>, <MODE>mode, operands);")

Testing is in progress. Is it ok for trunk if pass?

Also we might want to rename VI_AVX2, but I didn't do that
since new (generic) name would be too long. Say: VI_AVX2_AVX512BW_AVX512F.

gcc/
	* config/i386/sse.md (define_mode_iterator VI_AVX2): Extend
	to support AVX-512BW.
	(define_mode_iterator VI124_AVX2_48_AVX512F): Remove.
	(define_expand "<plusminus_insn><mode>3"): Remove masking support.
	(define_insn "*<plusminus_insn><mode>3"): Ditto.
	(define_expand "<mask_codefor><plusminus_insn><VI48_AVX512VL:mode>3<mask_name>"):
	New.
	(define_expand "<mask_codefor><plusminus_insn><VI12_AVX512VL:mode>3<mask_name>"):
	Ditto.
	(define_insn "*<plusminus_insn><VI48_AVX512VL:mode>3<mask_name>"): Ditto.
	(define_insn "*<plusminus_insn><VI12_AVX512VL:mode>3<mask_name>"): Ditto.
	(define_expand "<sse2_avx2>_andnot<mode>3"): Remove masking support.
	(define_insn "*andnot<mode>3"): Ditto.
	(define_expand "<mask_codefor><sse2_avx2>_andnot<VI48_AVX512VL:mode>3<mask_name>"): New.
	(define_expand "<mask_codefor><sse2_avx2>_andnot<VI12_AVX512VL:mode>3<mask_name>"): Ditto.
	(define_insn "*andnot<VI48_AVX512VL:mode>3<mask_name>"): Ditto.
	(define_insn "*andnot<VI12_AVX512VL:mode>3<mask_name>"): Ditto.
	(define_insn "*abs<mode>2"): Remove masking support.
	(define_insn "<mask_codefor>abs<VI48_AVX512VL:mode>2<mask_name>"): New.
	(define_insn "<mask_codefor>abs<VI12_AVX512VL:mode>2<mask_name>"): Ditto.
	(define_expand "abs<mode>2"): Use VI_AVX2 mode iterator.

--
Thanks, K

diff --git a/gcc/config/i386/sse.md b/gcc/config/i386/sse.md
index ffc831f..d6861e5 100644
--- a/gcc/config/i386/sse.md
+++ b/gcc/config/i386/sse.md
@@ -268,8 +268,8 @@
    (V4DI "TARGET_AVX") V2DI])
 
 (define_mode_iterator VI_AVX2
-  [(V32QI "TARGET_AVX2") V16QI
-   (V16HI "TARGET_AVX2") V8HI
+  [(V64QI "TARGET_AVX512BW") (V32QI "TARGET_AVX2") V16QI
+   (V32HI "TARGET_AVX512BW") (V16HI "TARGET_AVX2") V8HI
    (V16SI "TARGET_AVX512F") (V8SI "TARGET_AVX2") V4SI
    (V8DI "TARGET_AVX512F") (V4DI "TARGET_AVX2") V2DI])
 
@@ -359,12 +359,6 @@
   [(V16HI "TARGET_AVX2") V8HI
    (V8SI "TARGET_AVX2") V4SI])
 
-(define_mode_iterator VI124_AVX2_48_AVX512F
-  [(V32QI "TARGET_AVX2") V16QI
-   (V16HI "TARGET_AVX2") V8HI
-   (V16SI "TARGET_AVX512F") (V8SI "TARGET_AVX2") V4SI
-   (V8DI "TARGET_AVX512F")])
-
 (define_mode_iterator VI124_AVX512F
   [(V32QI "TARGET_AVX2") V16QI
    (V32HI "TARGET_AVX512F") (V16HI "TARGET_AVX2") V8HI
@@ -9051,20 +9045,37 @@
   "TARGET_SSE2"
   "operands[2] = force_reg (<MODE>mode, CONST0_RTX (<MODE>mode));")
 
-(define_expand "<plusminus_insn><mode>3<mask_name>"
+(define_expand "<plusminus_insn><mode>3"
   [(set (match_operand:VI_AVX2 0 "register_operand")
 	(plusminus:VI_AVX2
 	  (match_operand:VI_AVX2 1 "nonimmediate_operand")
 	  (match_operand:VI_AVX2 2 "nonimmediate_operand")))]
-  "TARGET_SSE2 && <mask_mode512bit_condition>"
+  "TARGET_SSE2"
   "ix86_fixup_binary_operands_no_copy (<CODE>, <MODE>mode, operands);")
 
-(define_insn "*<plusminus_insn><mode>3<mask_name>"
+(define_expand "<mask_codefor><plusminus_insn><mode>3<mask_name>"
+  [(set (match_operand:VI48_AVX512VL 0 "register_operand")
+	(plusminus:VI48_AVX512VL
+	  (match_operand:VI48_AVX512VL 1 "nonimmediate_operand")
+	  (match_operand:VI48_AVX512VL 2 "nonimmediate_operand")))]
+  "TARGET_AVX512F && <mask_applied>"
+  "ix86_fixup_binary_operands_no_copy (<CODE>, <MODE>mode, operands);")
+
+(define_expand "<mask_codefor><plusminus_insn><mode>3<mask_name>"
+  [(set (match_operand:VI12_AVX512VL 0 "register_operand")
+	(plusminus:VI12_AVX512VL
+	  (match_operand:VI12_AVX512VL 1 "nonimmediate_operand")
+	  (match_operand:VI12_AVX512VL 2 "nonimmediate_operand")))]
+  "TARGET_AVX512BW && <mask_applied>"
+  "ix86_fixup_binary_operands_no_copy (<CODE>, <MODE>mode, operands);")
+
+(define_insn "*<plusminus_insn><mode>3"
   [(set (match_operand:VI_AVX2 0 "register_operand" "=x,v")
 	(plusminus:VI_AVX2
 	  (match_operand:VI_AVX2 1 "nonimmediate_operand" "<comm>0,v")
 	  (match_operand:VI_AVX2 2 "nonimmediate_operand" "xm,vm")))]
-  "TARGET_SSE2 && ix86_binary_operator_ok (<CODE>, <MODE>mode, operands) && <mask_mode512bit_condition>"
+  "TARGET_SSE2
+   && ix86_binary_operator_ok (<CODE>, <MODE>mode, operands)"
   "@
    p<plusminus_mnemonic><ssemodesuffix>\t{%2, %0|%0, %2}
    vp<plusminus_mnemonic><ssemodesuffix>\t{%2, %1, %0<mask_operand3>|%0<mask_operand3>, %1, %2}"
@@ -9074,6 +9085,30 @@
    (set_attr "prefix" "<mask_prefix3>")
    (set_attr "mode" "<sseinsnmode>")])
 
+(define_insn "*<plusminus_insn><mode>3<mask_name>"
+  [(set (match_operand:VI48_AVX512VL 0 "register_operand" "=v")
+	(plusminus:VI48_AVX512VL
+	  (match_operand:VI48_AVX512VL 1 "nonimmediate_operand" "<comm>v")
+	  (match_operand:VI48_AVX512VL 2 "nonimmediate_operand" "vm")))]
+  "TARGET_AVX512F && <mask_applied>
+   && ix86_binary_operator_ok (<CODE>, <MODE>mode, operands)"
+  "vp<plusminus_mnemonic><ssemodesuffix>\t{%2, %1, %0<mask_operand3>|%0<mask_operand3>, %1, %2}"
+  [(set_attr "type" "sseiadd")
+   (set_attr "prefix" "evex")
+   (set_attr "mode" "<sseinsnmode>")])
+
+(define_insn "*<plusminus_insn><mode>3<mask_name>"
+  [(set (match_operand:VI12_AVX512VL 0 "register_operand" "=v")
+	(plusminus:VI12_AVX512VL
+	  (match_operand:VI12_AVX512VL 1 "nonimmediate_operand" "<comm>v")
+	  (match_operand:VI12_AVX512VL 2 "nonimmediate_operand" "vm")))]
+  "TARGET_AVX512BW && <mask_applied>
+   && ix86_binary_operator_ok (<CODE>, <MODE>mode, operands)"
+  "vp<plusminus_mnemonic><ssemodesuffix>\t{%2, %1, %0<mask_operand3>|%0<mask_operand3>, %1, %2}"
+  [(set_attr "type" "sseiadd")
+   (set_attr "prefix" "evex")
+   (set_attr "mode" "<sseinsnmode>")])
+
 (define_expand "<sse2_avx2>_<plusminus_insn><mode>3<mask_name>"
   [(set (match_operand:VI12_AVX2 0 "register_operand")
 	(sat_plusminus:VI12_AVX2
@@ -10489,19 +10524,33 @@
   operands[2] = force_reg (<MODE>mode, gen_rtx_CONST_VECTOR (<MODE>mode, v));
 })
 
-(define_expand "<sse2_avx2>_andnot<mode>3<mask_name>"
+(define_expand "<sse2_avx2>_andnot<mode>3"
   [(set (match_operand:VI_AVX2 0 "register_operand")
 	(and:VI_AVX2
 	  (not:VI_AVX2 (match_operand:VI_AVX2 1 "register_operand"))
 	  (match_operand:VI_AVX2 2 "nonimmediate_operand")))]
-  "TARGET_SSE2 && <mask_mode512bit_condition>")
+  "TARGET_SSE2")
 
-(define_insn "*andnot<mode>3<mask_name>"
+(define_expand "<mask_codefor><sse2_avx2>_andnot<mode>3<mask_name>"
+  [(set (match_operand:VI48_AVX512VL 0 "register_operand")
+	(and:VI48_AVX512VL
+	  (not:VI48_AVX512VL (match_operand:VI48_AVX512VL 1 "register_operand"))
+	  (match_operand:VI48_AVX512VL 2 "nonimmediate_operand")))]
+  "TARGET_AVX512F && <mask_applied>")
+
+(define_expand "<mask_codefor><sse2_avx2>_andnot<mode>3<mask_name>"
+  [(set (match_operand:VI12_AVX512VL 0 "register_operand")
+	(and:VI12_AVX512VL
+	  (not:VI12_AVX512VL (match_operand:VI12_AVX512VL 1 "register_operand"))
+	  (match_operand:VI12_AVX512VL 2 "nonimmediate_operand")))]
+  "TARGET_AVX512BW && <mask_applied>")
+
+(define_insn "*andnot<mode>3"
   [(set (match_operand:VI 0 "register_operand" "=x,v")
 	(and:VI
 	  (not:VI (match_operand:VI 1 "register_operand" "0,v"))
 	  (match_operand:VI 2 "nonimmediate_operand" "xm,vm")))]
-  "TARGET_SSE && <mask_mode512bit_condition>"
+  "TARGET_SSE"
 {
   static char buf[64];
   const char *ops;
@@ -10560,7 +10609,7 @@
 	    (eq_attr "mode" "TI"))
        (const_string "1")
        (const_string "*")))
-   (set_attr "prefix" "<mask_prefix3>")
+   (set_attr "prefix" "orig,vex")
    (set (attr "mode")
 	(cond [(and (match_test "<MODE_SIZE> == 16")
 		    (match_test "TARGET_SSE_PACKED_SINGLE_INSN_OPTIMAL"))
@@ -10578,6 +10627,30 @@
 	      ]
 	      (const_string "<sseinsnmode>")))])
 
+(define_insn "*andnot<mode>3<mask_name>"
+  [(set (match_operand:VI48_AVX512VL 0 "register_operand" "=v")
+	(and:VI48_AVX512VL
+	  (not:VI48_AVX512VL
+	    (match_operand:VI48_AVX512VL 1 "register_operand" "v"))
+	  (match_operand:VI48_AVX512VL 2 "nonimmediate_operand" "vm")))]
+  "TARGET_AVX512F && <mask_applied>"
+  "vpandn<ssemodesuffix>\t{%%2, %%1, %%0<mask_operand3>|%%0<mask_operand3>, %%1, %%2}";
+  [(set_attr "type" "sselog")
+   (set_attr "prefix" "evex")
+   (set_attr "mode" "<sseinsnmode>")])
+
+(define_insn "*andnot<mode>3<mask_name>"
+  [(set (match_operand:VI12_AVX512VL 0 "register_operand" "=v")
+	(and:VI12_AVX512VL
+	  (not:VI12_AVX512VL
+	    (match_operand:VI12_AVX512VL 1 "register_operand" "v"))
+	  (match_operand:VI12_AVX512VL 2 "nonimmediate_operand" "vm")))]
+  "TARGET_AVX512BW && <mask_applied>"
+  "vpandn<ssemodesuffix>\t{%%2, %%1, %%0<mask_operand3>|%%0<mask_operand3>, %%1, %%2}";
+  [(set_attr "type" "sselog")
+   (set_attr "prefix" "evex")
+   (set_attr "mode" "<sseinsnmode>")])
+
 (define_expand "<code><mode>3"
   [(set (match_operand:VI 0 "register_operand")
 	(any_logic:VI
@@ -13361,22 +13434,43 @@
    (set (attr "prefix_rex") (symbol_ref "x86_extended_reg_mentioned_p (insn)"))
    (set_attr "mode" "DI")])
 
-(define_insn "<mask_codefor>abs<mode>2<mask_name>"
-  [(set (match_operand:VI124_AVX2_48_AVX512F 0 "register_operand" "=v")
-	(abs:VI124_AVX2_48_AVX512F
-	  (match_operand:VI124_AVX2_48_AVX512F 1 "nonimmediate_operand" "vm")))]
-  "TARGET_SSSE3 && <mask_mode512bit_condition>"
-  "%vpabs<ssemodesuffix>\t{%1, %0<mask_operand2>|%0<mask_operand2>, %1}"
+(define_insn "*abs<mode>2"
+  [(set (match_operand:VI_AVX2 0 "register_operand" "=v")
+	(abs:VI_AVX2
+	  (match_operand:VI_AVX2 1 "nonimmediate_operand" "vm")))]
+  "TARGET_SSSE3"
+  "%vpabs<ssemodesuffix>\t{%1, %0|%0, %1}"
   [(set_attr "type" "sselog1")
    (set_attr "prefix_data16" "1")
    (set_attr "prefix_extra" "1")
    (set_attr "prefix" "maybe_vex")
    (set_attr "mode" "<sseinsnmode>")])
 
+(define_insn "<mask_codefor>abs<mode>2<mask_name>"
+  [(set (match_operand:VI48_AVX512VL 0 "register_operand" "=v")
+	(abs:VI48_AVX512VL
+	  (match_operand:VI48_AVX512VL 1 "nonimmediate_operand" "vm")))]
+  "TARGET_AVX512F && <mask_applied>"
+  "vpabs<ssemodesuffix>\t{%1, %0<mask_operand2>|%0<mask_operand2>, %1}"
+  [(set_attr "type" "sselog1")
+   (set_attr "prefix" "evex")
+   (set_attr "mode" "<sseinsnmode>")])
+
+(define_insn "<mask_codefor>abs<mode>2<mask_name>"
+  [(set (match_operand:VI12_AVX512VL 0 "register_operand" "=v")
+	(abs:VI12_AVX512VL
+	  (match_operand:VI12_AVX512VL 1 "nonimmediate_operand" "vm")))]
+  "TARGET_AVX512BW && <mask_applied>"
+  "vpabs<ssemodesuffix>\t{%1, %0<mask_operand2>|%0<mask_operand2>, %1}"
+  [(set_attr "type" "sselog1")
+   (set_attr "prefix" "evex")
+   (set_attr "mode" "<sseinsnmode>")])
+
+
 (define_expand "abs<mode>2"
-  [(set (match_operand:VI124_AVX2_48_AVX512F 0 "register_operand")
-	(abs:VI124_AVX2_48_AVX512F
-	  (match_operand:VI124_AVX2_48_AVX512F 1 "nonimmediate_operand")))]
+  [(set (match_operand:VI_AVX2 0 "register_operand")
+	(abs:VI_AVX2
+	  (match_operand:VI_AVX2 1 "nonimmediate_operand")))]
   "TARGET_SSE2"
 {
   if (!TARGET_SSSE3)

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH i386 AVX512] [56/n] Add plus/minus/abs/neg/andnot insn patterns.
  2014-09-25 14:12 [PATCH i386 AVX512] [56/n] Add plus/minus/abs/neg/andnot insn patterns Kirill Yukhin
@ 2014-09-25 18:12 ` Uros Bizjak
  2014-10-14  7:20   ` Kirill Yukhin
  0 siblings, 1 reply; 8+ messages in thread
From: Uros Bizjak @ 2014-09-25 18:12 UTC (permalink / raw)
  To: Kirill Yukhin; +Cc: Jakub Jelinek, Richard Henderson, GCC Patches

On Thu, Sep 25, 2014 at 4:12 PM, Kirill Yukhin <kirill.yukhin@gmail.com> wrote:
> Hello,
> Patch in the bottom extends plus/minus/abs/andnot patterns
> to support AVX-512.
> I've used questionable hack in the patterns.
> Instead of writing dozen similar patterns with masking
> I've simply substed them, prohibiting non-mask variant in
> the pattern condition. E.g.:
> (define_expand "<mask_codefor><plusminus_insn><mode>3<mask_name>"
>   [(set (match_operand:VI12_AVX512VL 0 "register_operand")
>        (plusminus:VI12_AVX512VL
>           (match_operand:VI12_AVX512VL 1 "nonimmediate_operand")
>           (match_operand:VI12_AVX512VL 2 "nonimmediate_operand")))]
>   "TARGET_AVX512BW && <mask_applied>"
>   "ix86_fixup_binary_operands_no_copy (<CODE>, <MODE>mode, operands);")
>
> If this is not acceptable, I'll rewrite it to somthing like:
> (define_expand "<mask_codefor><plusminus_insn><mode>3<mask_name>"
>   [(set (match_operand:VI12_AVX512VL 0 "register_operand")
>         (vec_merge: VI12_AVX512VL
>           (plusminus:VI12_AVX512VL
>             (match_operand:VI12_AVX512VL 1 "nonimmediate_operand")
>             (match_operand:VI12_AVX512VL 2 "nonimmediate_operand"))
>           (match_operand:SUBST_V 2 "vector_move_operand" "0C")
>           (match_operand:<avx512fmaskmode> 3 "register_operand" "Yk")))]
>   "TARGET_AVX512BW && <mask_applied>"
>   "ix86_fixup_binary_operands_no_copy (<CODE>, <MODE>mode, operands);")

I'd rather go with the second approach, it is less confusing from the
maintainer POV. All other patterns with masking use some consistent
template, so I'd suggest using the same approach for everything. If it
is indeed too many patterns, then please split the patch to smaller
pieces.

Uros.

> Testing is in progress. Is it ok for trunk if pass?
>
> Also we might want to rename VI_AVX2, but I didn't do that
> since new (generic) name would be too long. Say: VI_AVX2_AVX512BW_AVX512F.

Iterator names try to follow some logic, but we can live with that.

> gcc/
>         * config/i386/sse.md (define_mode_iterator VI_AVX2): Extend
>         to support AVX-512BW.
>         (define_mode_iterator VI124_AVX2_48_AVX512F): Remove.
>         (define_expand "<plusminus_insn><mode>3"): Remove masking support.
>         (define_insn "*<plusminus_insn><mode>3"): Ditto.
>         (define_expand "<mask_codefor><plusminus_insn><VI48_AVX512VL:mode>3<mask_name>"):
>         New.
>         (define_expand "<mask_codefor><plusminus_insn><VI12_AVX512VL:mode>3<mask_name>"):
>         Ditto.
>         (define_insn "*<plusminus_insn><VI48_AVX512VL:mode>3<mask_name>"): Ditto.
>         (define_insn "*<plusminus_insn><VI12_AVX512VL:mode>3<mask_name>"): Ditto.
>         (define_expand "<sse2_avx2>_andnot<mode>3"): Remove masking support.
>         (define_insn "*andnot<mode>3"): Ditto.
>         (define_expand "<mask_codefor><sse2_avx2>_andnot<VI48_AVX512VL:mode>3<mask_name>"): New.
>         (define_expand "<mask_codefor><sse2_avx2>_andnot<VI12_AVX512VL:mode>3<mask_name>"): Ditto.
>         (define_insn "*andnot<VI48_AVX512VL:mode>3<mask_name>"): Ditto.
>         (define_insn "*andnot<VI12_AVX512VL:mode>3<mask_name>"): Ditto.
>         (define_insn "*abs<mode>2"): Remove masking support.
>         (define_insn "<mask_codefor>abs<VI48_AVX512VL:mode>2<mask_name>"): New.
>         (define_insn "<mask_codefor>abs<VI12_AVX512VL:mode>2<mask_name>"): Ditto.
>         (define_expand "abs<mode>2"): Use VI_AVX2 mode iterator.
>
> --
> Thanks, K
>
> diff --git a/gcc/config/i386/sse.md b/gcc/config/i386/sse.md
> index ffc831f..d6861e5 100644
> --- a/gcc/config/i386/sse.md
> +++ b/gcc/config/i386/sse.md
> @@ -268,8 +268,8 @@
>     (V4DI "TARGET_AVX") V2DI])
>
>  (define_mode_iterator VI_AVX2
> -  [(V32QI "TARGET_AVX2") V16QI
> -   (V16HI "TARGET_AVX2") V8HI
> +  [(V64QI "TARGET_AVX512BW") (V32QI "TARGET_AVX2") V16QI
> +   (V32HI "TARGET_AVX512BW") (V16HI "TARGET_AVX2") V8HI
>     (V16SI "TARGET_AVX512F") (V8SI "TARGET_AVX2") V4SI
>     (V8DI "TARGET_AVX512F") (V4DI "TARGET_AVX2") V2DI])
>
> @@ -359,12 +359,6 @@
>    [(V16HI "TARGET_AVX2") V8HI
>     (V8SI "TARGET_AVX2") V4SI])
>
> -(define_mode_iterator VI124_AVX2_48_AVX512F
> -  [(V32QI "TARGET_AVX2") V16QI
> -   (V16HI "TARGET_AVX2") V8HI
> -   (V16SI "TARGET_AVX512F") (V8SI "TARGET_AVX2") V4SI
> -   (V8DI "TARGET_AVX512F")])
> -
>  (define_mode_iterator VI124_AVX512F
>    [(V32QI "TARGET_AVX2") V16QI
>     (V32HI "TARGET_AVX512F") (V16HI "TARGET_AVX2") V8HI
> @@ -9051,20 +9045,37 @@
>    "TARGET_SSE2"
>    "operands[2] = force_reg (<MODE>mode, CONST0_RTX (<MODE>mode));")
>
> -(define_expand "<plusminus_insn><mode>3<mask_name>"
> +(define_expand "<plusminus_insn><mode>3"
>    [(set (match_operand:VI_AVX2 0 "register_operand")
>         (plusminus:VI_AVX2
>           (match_operand:VI_AVX2 1 "nonimmediate_operand")
>           (match_operand:VI_AVX2 2 "nonimmediate_operand")))]
> -  "TARGET_SSE2 && <mask_mode512bit_condition>"
> +  "TARGET_SSE2"
>    "ix86_fixup_binary_operands_no_copy (<CODE>, <MODE>mode, operands);")
>
> -(define_insn "*<plusminus_insn><mode>3<mask_name>"
> +(define_expand "<mask_codefor><plusminus_insn><mode>3<mask_name>"
> +  [(set (match_operand:VI48_AVX512VL 0 "register_operand")
> +       (plusminus:VI48_AVX512VL
> +         (match_operand:VI48_AVX512VL 1 "nonimmediate_operand")
> +         (match_operand:VI48_AVX512VL 2 "nonimmediate_operand")))]
> +  "TARGET_AVX512F && <mask_applied>"
> +  "ix86_fixup_binary_operands_no_copy (<CODE>, <MODE>mode, operands);")
> +
> +(define_expand "<mask_codefor><plusminus_insn><mode>3<mask_name>"
> +  [(set (match_operand:VI12_AVX512VL 0 "register_operand")
> +       (plusminus:VI12_AVX512VL
> +         (match_operand:VI12_AVX512VL 1 "nonimmediate_operand")
> +         (match_operand:VI12_AVX512VL 2 "nonimmediate_operand")))]
> +  "TARGET_AVX512BW && <mask_applied>"
> +  "ix86_fixup_binary_operands_no_copy (<CODE>, <MODE>mode, operands);")
> +
> +(define_insn "*<plusminus_insn><mode>3"
>    [(set (match_operand:VI_AVX2 0 "register_operand" "=x,v")
>         (plusminus:VI_AVX2
>           (match_operand:VI_AVX2 1 "nonimmediate_operand" "<comm>0,v")
>           (match_operand:VI_AVX2 2 "nonimmediate_operand" "xm,vm")))]
> -  "TARGET_SSE2 && ix86_binary_operator_ok (<CODE>, <MODE>mode, operands) && <mask_mode512bit_condition>"
> +  "TARGET_SSE2
> +   && ix86_binary_operator_ok (<CODE>, <MODE>mode, operands)"
>    "@
>     p<plusminus_mnemonic><ssemodesuffix>\t{%2, %0|%0, %2}
>     vp<plusminus_mnemonic><ssemodesuffix>\t{%2, %1, %0<mask_operand3>|%0<mask_operand3>, %1, %2}"
> @@ -9074,6 +9085,30 @@
>     (set_attr "prefix" "<mask_prefix3>")
>     (set_attr "mode" "<sseinsnmode>")])
>
> +(define_insn "*<plusminus_insn><mode>3<mask_name>"
> +  [(set (match_operand:VI48_AVX512VL 0 "register_operand" "=v")
> +       (plusminus:VI48_AVX512VL
> +         (match_operand:VI48_AVX512VL 1 "nonimmediate_operand" "<comm>v")
> +         (match_operand:VI48_AVX512VL 2 "nonimmediate_operand" "vm")))]
> +  "TARGET_AVX512F && <mask_applied>
> +   && ix86_binary_operator_ok (<CODE>, <MODE>mode, operands)"
> +  "vp<plusminus_mnemonic><ssemodesuffix>\t{%2, %1, %0<mask_operand3>|%0<mask_operand3>, %1, %2}"
> +  [(set_attr "type" "sseiadd")
> +   (set_attr "prefix" "evex")
> +   (set_attr "mode" "<sseinsnmode>")])
> +
> +(define_insn "*<plusminus_insn><mode>3<mask_name>"
> +  [(set (match_operand:VI12_AVX512VL 0 "register_operand" "=v")
> +       (plusminus:VI12_AVX512VL
> +         (match_operand:VI12_AVX512VL 1 "nonimmediate_operand" "<comm>v")
> +         (match_operand:VI12_AVX512VL 2 "nonimmediate_operand" "vm")))]
> +  "TARGET_AVX512BW && <mask_applied>
> +   && ix86_binary_operator_ok (<CODE>, <MODE>mode, operands)"
> +  "vp<plusminus_mnemonic><ssemodesuffix>\t{%2, %1, %0<mask_operand3>|%0<mask_operand3>, %1, %2}"
> +  [(set_attr "type" "sseiadd")
> +   (set_attr "prefix" "evex")
> +   (set_attr "mode" "<sseinsnmode>")])
> +
>  (define_expand "<sse2_avx2>_<plusminus_insn><mode>3<mask_name>"
>    [(set (match_operand:VI12_AVX2 0 "register_operand")
>         (sat_plusminus:VI12_AVX2
> @@ -10489,19 +10524,33 @@
>    operands[2] = force_reg (<MODE>mode, gen_rtx_CONST_VECTOR (<MODE>mode, v));
>  })
>
> -(define_expand "<sse2_avx2>_andnot<mode>3<mask_name>"
> +(define_expand "<sse2_avx2>_andnot<mode>3"
>    [(set (match_operand:VI_AVX2 0 "register_operand")
>         (and:VI_AVX2
>           (not:VI_AVX2 (match_operand:VI_AVX2 1 "register_operand"))
>           (match_operand:VI_AVX2 2 "nonimmediate_operand")))]
> -  "TARGET_SSE2 && <mask_mode512bit_condition>")
> +  "TARGET_SSE2")
>
> -(define_insn "*andnot<mode>3<mask_name>"
> +(define_expand "<mask_codefor><sse2_avx2>_andnot<mode>3<mask_name>"
> +  [(set (match_operand:VI48_AVX512VL 0 "register_operand")
> +       (and:VI48_AVX512VL
> +         (not:VI48_AVX512VL (match_operand:VI48_AVX512VL 1 "register_operand"))
> +         (match_operand:VI48_AVX512VL 2 "nonimmediate_operand")))]
> +  "TARGET_AVX512F && <mask_applied>")
> +
> +(define_expand "<mask_codefor><sse2_avx2>_andnot<mode>3<mask_name>"
> +  [(set (match_operand:VI12_AVX512VL 0 "register_operand")
> +       (and:VI12_AVX512VL
> +         (not:VI12_AVX512VL (match_operand:VI12_AVX512VL 1 "register_operand"))
> +         (match_operand:VI12_AVX512VL 2 "nonimmediate_operand")))]
> +  "TARGET_AVX512BW && <mask_applied>")
> +
> +(define_insn "*andnot<mode>3"
>    [(set (match_operand:VI 0 "register_operand" "=x,v")
>         (and:VI
>           (not:VI (match_operand:VI 1 "register_operand" "0,v"))
>           (match_operand:VI 2 "nonimmediate_operand" "xm,vm")))]
> -  "TARGET_SSE && <mask_mode512bit_condition>"
> +  "TARGET_SSE"
>  {
>    static char buf[64];
>    const char *ops;
> @@ -10560,7 +10609,7 @@
>             (eq_attr "mode" "TI"))
>         (const_string "1")
>         (const_string "*")))
> -   (set_attr "prefix" "<mask_prefix3>")
> +   (set_attr "prefix" "orig,vex")
>     (set (attr "mode")
>         (cond [(and (match_test "<MODE_SIZE> == 16")
>                     (match_test "TARGET_SSE_PACKED_SINGLE_INSN_OPTIMAL"))
> @@ -10578,6 +10627,30 @@
>               ]
>               (const_string "<sseinsnmode>")))])
>
> +(define_insn "*andnot<mode>3<mask_name>"
> +  [(set (match_operand:VI48_AVX512VL 0 "register_operand" "=v")
> +       (and:VI48_AVX512VL
> +         (not:VI48_AVX512VL
> +           (match_operand:VI48_AVX512VL 1 "register_operand" "v"))
> +         (match_operand:VI48_AVX512VL 2 "nonimmediate_operand" "vm")))]
> +  "TARGET_AVX512F && <mask_applied>"
> +  "vpandn<ssemodesuffix>\t{%%2, %%1, %%0<mask_operand3>|%%0<mask_operand3>, %%1, %%2}";
> +  [(set_attr "type" "sselog")
> +   (set_attr "prefix" "evex")
> +   (set_attr "mode" "<sseinsnmode>")])
> +
> +(define_insn "*andnot<mode>3<mask_name>"
> +  [(set (match_operand:VI12_AVX512VL 0 "register_operand" "=v")
> +       (and:VI12_AVX512VL
> +         (not:VI12_AVX512VL
> +           (match_operand:VI12_AVX512VL 1 "register_operand" "v"))
> +         (match_operand:VI12_AVX512VL 2 "nonimmediate_operand" "vm")))]
> +  "TARGET_AVX512BW && <mask_applied>"
> +  "vpandn<ssemodesuffix>\t{%%2, %%1, %%0<mask_operand3>|%%0<mask_operand3>, %%1, %%2}";
> +  [(set_attr "type" "sselog")
> +   (set_attr "prefix" "evex")
> +   (set_attr "mode" "<sseinsnmode>")])
> +
>  (define_expand "<code><mode>3"
>    [(set (match_operand:VI 0 "register_operand")
>         (any_logic:VI
> @@ -13361,22 +13434,43 @@
>     (set (attr "prefix_rex") (symbol_ref "x86_extended_reg_mentioned_p (insn)"))
>     (set_attr "mode" "DI")])
>
> -(define_insn "<mask_codefor>abs<mode>2<mask_name>"
> -  [(set (match_operand:VI124_AVX2_48_AVX512F 0 "register_operand" "=v")
> -       (abs:VI124_AVX2_48_AVX512F
> -         (match_operand:VI124_AVX2_48_AVX512F 1 "nonimmediate_operand" "vm")))]
> -  "TARGET_SSSE3 && <mask_mode512bit_condition>"
> -  "%vpabs<ssemodesuffix>\t{%1, %0<mask_operand2>|%0<mask_operand2>, %1}"
> +(define_insn "*abs<mode>2"
> +  [(set (match_operand:VI_AVX2 0 "register_operand" "=v")
> +       (abs:VI_AVX2
> +         (match_operand:VI_AVX2 1 "nonimmediate_operand" "vm")))]
> +  "TARGET_SSSE3"
> +  "%vpabs<ssemodesuffix>\t{%1, %0|%0, %1}"
>    [(set_attr "type" "sselog1")
>     (set_attr "prefix_data16" "1")
>     (set_attr "prefix_extra" "1")
>     (set_attr "prefix" "maybe_vex")
>     (set_attr "mode" "<sseinsnmode>")])
>
> +(define_insn "<mask_codefor>abs<mode>2<mask_name>"
> +  [(set (match_operand:VI48_AVX512VL 0 "register_operand" "=v")
> +       (abs:VI48_AVX512VL
> +         (match_operand:VI48_AVX512VL 1 "nonimmediate_operand" "vm")))]
> +  "TARGET_AVX512F && <mask_applied>"
> +  "vpabs<ssemodesuffix>\t{%1, %0<mask_operand2>|%0<mask_operand2>, %1}"
> +  [(set_attr "type" "sselog1")
> +   (set_attr "prefix" "evex")
> +   (set_attr "mode" "<sseinsnmode>")])
> +
> +(define_insn "<mask_codefor>abs<mode>2<mask_name>"
> +  [(set (match_operand:VI12_AVX512VL 0 "register_operand" "=v")
> +       (abs:VI12_AVX512VL
> +         (match_operand:VI12_AVX512VL 1 "nonimmediate_operand" "vm")))]
> +  "TARGET_AVX512BW && <mask_applied>"
> +  "vpabs<ssemodesuffix>\t{%1, %0<mask_operand2>|%0<mask_operand2>, %1}"
> +  [(set_attr "type" "sselog1")
> +   (set_attr "prefix" "evex")
> +   (set_attr "mode" "<sseinsnmode>")])
> +
> +
>  (define_expand "abs<mode>2"
> -  [(set (match_operand:VI124_AVX2_48_AVX512F 0 "register_operand")
> -       (abs:VI124_AVX2_48_AVX512F
> -         (match_operand:VI124_AVX2_48_AVX512F 1 "nonimmediate_operand")))]
> +  [(set (match_operand:VI_AVX2 0 "register_operand")
> +       (abs:VI_AVX2
> +         (match_operand:VI_AVX2 1 "nonimmediate_operand")))]
>    "TARGET_SSE2"
>  {
>    if (!TARGET_SSSE3)

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH i386 AVX512] [56/n] Add plus/minus/abs/neg/andnot insn patterns.
  2014-09-25 18:12 ` Uros Bizjak
@ 2014-10-14  7:20   ` Kirill Yukhin
  2014-10-14 18:37     ` Uros Bizjak
  2014-10-20 12:38     ` Jakub Jelinek
  0 siblings, 2 replies; 8+ messages in thread
From: Kirill Yukhin @ 2014-10-14  7:20 UTC (permalink / raw)
  To: Uros Bizjak; +Cc: Jakub Jelinek, Richard Henderson, GCC Patches

Hello Uroš,
It seems like I missed to post uppdated patch.
On 25 Sep 20:11, Uros Bizjak wrote:
> I'd rather go with the second approach, it is less confusing from the
> maintainer POV. All other patterns with masking use some consistent
> template, so I'd suggest using the same approach for everything. If it
> is indeed too many patterns, then please split the patch to smaller
> pieces.
Goal was not to decrease size of the patch, I wanted to make pattern look
simpler by hiding masking stuff beyond `subst'.
Anyway, I've updated the patch.

Here it is (bootstrapped and regtested).

Is it ok for trunk?

gcc/
	* config/i386/sse.md (define_mode_iterator VI_AVX2): Extend
	to support AVX-512BW.
	(define_mode_iterator VI124_AVX2_48_AVX512F): Remove.
	(define_expand "<plusminus_insn><mode>3"): Remove masking support.
	(define_insn "*<plusminus_insn><mode>3"): Ditto.
	(define_expand "<plusminus_insn><VI48_AVX512VL:mode>3_mask"): New.
	(define_expand "<plusminus_insn><VI12_AVX512VL:mode>3_mask"): Ditto.
	(define_insn "*<plusminus_insn><VI48_AVX512VL:mode>3_mask"): Ditto.
	(define_insn "*<plusminus_insn><VI12_AVX512VL:mode>3_mask"): Ditto.
	(define_expand "<sse2_avx2>_andnot<mode>3"): Remove masking support.
	(define_insn "*andnot<mode>3"): Ditto.
	(define_expand "<sse2_avx2>_andnot<VI48_AVX512VL:mode>3_mask"): New.
	(define_expand "<sse2_avx2>_andnot<VI12_AVX512VL:mode>3_mask"): Ditto.
	(define_insn "*andnot<VI48_AVX512VL:mode>3<mask_name>"): Ditto.
	(define_insn "*andnot<VI12_AVX512VL:mode>3<mask_name>"): Ditto.
	(define_insn "*abs<mode>2"): Remove masking support.
	(define_insn "abs<VI48_AVX512VL:mode>2_mask"): New.
	(define_insn "abs<VI12_AVX512VL:mode>2_mask"): Ditto.
	(define_expand "abs<mode>2"): Use VI_AVX2 mode iterator.

--
Thanks, K

diff --git a/gcc/config/i386/sse.md b/gcc/config/i386/sse.md
index ffc831f..9edfebc 100644
--- a/gcc/config/i386/sse.md
+++ b/gcc/config/i386/sse.md
@@ -268,8 +268,8 @@
    (V4DI "TARGET_AVX") V2DI])
 
 (define_mode_iterator VI_AVX2
-  [(V32QI "TARGET_AVX2") V16QI
-   (V16HI "TARGET_AVX2") V8HI
+  [(V64QI "TARGET_AVX512BW") (V32QI "TARGET_AVX2") V16QI
+   (V32HI "TARGET_AVX512BW") (V16HI "TARGET_AVX2") V8HI
    (V16SI "TARGET_AVX512F") (V8SI "TARGET_AVX2") V4SI
    (V8DI "TARGET_AVX512F") (V4DI "TARGET_AVX2") V2DI])
 
@@ -359,12 +359,6 @@
   [(V16HI "TARGET_AVX2") V8HI
    (V8SI "TARGET_AVX2") V4SI])
 
-(define_mode_iterator VI124_AVX2_48_AVX512F
-  [(V32QI "TARGET_AVX2") V16QI
-   (V16HI "TARGET_AVX2") V8HI
-   (V16SI "TARGET_AVX512F") (V8SI "TARGET_AVX2") V4SI
-   (V8DI "TARGET_AVX512F")])
-
 (define_mode_iterator VI124_AVX512F
   [(V32QI "TARGET_AVX2") V16QI
    (V32HI "TARGET_AVX512F") (V16HI "TARGET_AVX2") V8HI
@@ -9051,20 +9045,43 @@
   "TARGET_SSE2"
   "operands[2] = force_reg (<MODE>mode, CONST0_RTX (<MODE>mode));")
 
-(define_expand "<plusminus_insn><mode>3<mask_name>"
+(define_expand "<plusminus_insn><mode>3"
   [(set (match_operand:VI_AVX2 0 "register_operand")
 	(plusminus:VI_AVX2
 	  (match_operand:VI_AVX2 1 "nonimmediate_operand")
 	  (match_operand:VI_AVX2 2 "nonimmediate_operand")))]
-  "TARGET_SSE2 && <mask_mode512bit_condition>"
+  "TARGET_SSE2"
+  "ix86_fixup_binary_operands_no_copy (<CODE>, <MODE>mode, operands);")
+
+(define_expand "<plusminus_insn><mode>3_mask"
+  [(set (match_operand:VI48_AVX512VL 0 "register_operand")
+	(vec_merge:VI48_AVX512VL
+	  (plusminus:VI48_AVX512VL
+	    (match_operand:VI48_AVX512VL 1 "nonimmediate_operand")
+	    (match_operand:VI48_AVX512VL 2 "nonimmediate_operand"))
+	  (match_operand:VI48_AVX512VL 3 "vector_move_operand")
+	  (match_operand:<avx512fmaskmode> 4 "register_operand")))]
+  "TARGET_AVX512F"
+  "ix86_fixup_binary_operands_no_copy (<CODE>, <MODE>mode, operands);")
+
+(define_expand "<plusminus_insn><mode>3_mask"
+  [(set (match_operand:VI12_AVX512VL 0 "register_operand")
+	(vec_merge:VI12_AVX512VL
+	  (plusminus:VI12_AVX512VL
+	    (match_operand:VI12_AVX512VL 1 "nonimmediate_operand")
+	    (match_operand:VI12_AVX512VL 2 "nonimmediate_operand"))
+	  (match_operand:VI12_AVX512VL 3 "vector_move_operand")
+	  (match_operand:<avx512fmaskmode> 4 "register_operand")))]
+  "TARGET_AVX512BW"
   "ix86_fixup_binary_operands_no_copy (<CODE>, <MODE>mode, operands);")
 
-(define_insn "*<plusminus_insn><mode>3<mask_name>"
+(define_insn "*<plusminus_insn><mode>3"
   [(set (match_operand:VI_AVX2 0 "register_operand" "=x,v")
 	(plusminus:VI_AVX2
 	  (match_operand:VI_AVX2 1 "nonimmediate_operand" "<comm>0,v")
 	  (match_operand:VI_AVX2 2 "nonimmediate_operand" "xm,vm")))]
-  "TARGET_SSE2 && ix86_binary_operator_ok (<CODE>, <MODE>mode, operands) && <mask_mode512bit_condition>"
+  "TARGET_SSE2
+   && ix86_binary_operator_ok (<CODE>, <MODE>mode, operands)"
   "@
    p<plusminus_mnemonic><ssemodesuffix>\t{%2, %0|%0, %2}
    vp<plusminus_mnemonic><ssemodesuffix>\t{%2, %1, %0<mask_operand3>|%0<mask_operand3>, %1, %2}"
@@ -9074,6 +9091,35 @@
    (set_attr "prefix" "<mask_prefix3>")
    (set_attr "mode" "<sseinsnmode>")])
 
+(define_insn "*<plusminus_insn><mode>3_mask"
+  [(set (match_operand:VI48_AVX512VL 0 "register_operand" "=v")
+	(vec_merge:VI48_AVX512VL
+	  (plusminus:VI48_AVX512VL
+	    (match_operand:VI48_AVX512VL 1 "nonimmediate_operand" "<comm>v")
+	    (match_operand:VI48_AVX512VL 2 "nonimmediate_operand" "vm"))
+	  (match_operand:VI48_AVX512VL 3 "vector_move_operand" "0C")
+	  (match_operand:<avx512fmaskmode> 4 "register_operand" "Yk")))]
+  "TARGET_AVX512F
+   && ix86_binary_operator_ok (<CODE>, <MODE>mode, operands)"
+  "vp<plusminus_mnemonic><ssemodesuffix>\t{%2, %1, %0%{%4%}%N3|%0%{%4%}%N3, %1, %2}"
+  [(set_attr "type" "sseiadd")
+   (set_attr "prefix" "evex")
+   (set_attr "mode" "<sseinsnmode>")])
+
+(define_insn "*<plusminus_insn><mode>3_mask"
+  [(set (match_operand:VI12_AVX512VL 0 "register_operand" "=v")
+	(vec_merge:VI12_AVX512VL
+	  (plusminus:VI12_AVX512VL
+	    (match_operand:VI12_AVX512VL 1 "nonimmediate_operand" "<comm>v")
+	    (match_operand:VI12_AVX512VL 2 "nonimmediate_operand" "vm"))
+	  (match_operand:VI12_AVX512VL 3 "vector_move_operand" "0C")
+	  (match_operand:<avx512fmaskmode> 4 "register_operand" "Yk")))]
+  "TARGET_AVX512BW && ix86_binary_operator_ok (<CODE>, <MODE>mode, operands)"
+  "vp<plusminus_mnemonic><ssemodesuffix>\t{%2, %1, %0%{%4%}%N3|%0%{%4%}%N3, %1, %2}"
+  [(set_attr "type" "sseiadd")
+   (set_attr "prefix" "evex")
+   (set_attr "mode" "<sseinsnmode>")])
+
 (define_expand "<sse2_avx2>_<plusminus_insn><mode>3<mask_name>"
   [(set (match_operand:VI12_AVX2 0 "register_operand")
 	(sat_plusminus:VI12_AVX2
@@ -10489,19 +10535,41 @@
   operands[2] = force_reg (<MODE>mode, gen_rtx_CONST_VECTOR (<MODE>mode, v));
 })
 
-(define_expand "<sse2_avx2>_andnot<mode>3<mask_name>"
+(define_expand "<sse2_avx2>_andnot<mode>3"
   [(set (match_operand:VI_AVX2 0 "register_operand")
 	(and:VI_AVX2
 	  (not:VI_AVX2 (match_operand:VI_AVX2 1 "register_operand"))
 	  (match_operand:VI_AVX2 2 "nonimmediate_operand")))]
-  "TARGET_SSE2 && <mask_mode512bit_condition>")
+  "TARGET_SSE2")
 
-(define_insn "*andnot<mode>3<mask_name>"
+(define_expand "<sse2_avx2>_andnot<mode>3_mask"
+  [(set (match_operand:VI48_AVX512VL 0 "register_operand")
+	(vec_merge:VI48_AVX512VL
+	  (and:VI48_AVX512VL
+	    (not:VI48_AVX512VL
+	      (match_operand:VI48_AVX512VL 1 "register_operand"))
+	    (match_operand:VI48_AVX512VL 2 "nonimmediate_operand"))
+	  (match_operand:VI48_AVX512VL 3 "vector_move_operand")
+	  (match_operand:<avx512fmaskmode> 4 "register_operand")))]
+  "TARGET_AVX512F")
+
+(define_expand "<sse2_avx2>_andnot<mode>3_mask"
+  [(set (match_operand:VI12_AVX512VL 0 "register_operand")
+	(vec_merge:VI12_AVX512VL
+	  (and:VI12_AVX512VL
+	    (not:VI12_AVX512VL
+	      (match_operand:VI12_AVX512VL 1 "register_operand"))
+	    (match_operand:VI12_AVX512VL 2 "nonimmediate_operand"))
+	  (match_operand:VI12_AVX512VL 3 "vector_move_operand")
+	  (match_operand:<avx512fmaskmode> 4 "register_operand")))]
+  "TARGET_AVX512BW")
+
+(define_insn "*andnot<mode>3"
   [(set (match_operand:VI 0 "register_operand" "=x,v")
 	(and:VI
 	  (not:VI (match_operand:VI 1 "register_operand" "0,v"))
 	  (match_operand:VI 2 "nonimmediate_operand" "xm,vm")))]
-  "TARGET_SSE && <mask_mode512bit_condition>"
+  "TARGET_SSE"
 {
   static char buf[64];
   const char *ops;
@@ -10560,7 +10628,7 @@
 	    (eq_attr "mode" "TI"))
        (const_string "1")
        (const_string "*")))
-   (set_attr "prefix" "<mask_prefix3>")
+   (set_attr "prefix" "orig,vex")
    (set (attr "mode")
 	(cond [(and (match_test "<MODE_SIZE> == 16")
 		    (match_test "TARGET_SSE_PACKED_SINGLE_INSN_OPTIMAL"))
@@ -10578,6 +10646,36 @@
 	      ]
 	      (const_string "<sseinsnmode>")))])
 
+(define_insn "*andnot<mode>3_mask"
+  [(set (match_operand:VI48_AVX512VL 0 "register_operand" "=v")
+	(vec_merge:VI48_AVX512VL
+	  (and:VI48_AVX512VL
+	    (not:VI48_AVX512VL
+	      (match_operand:VI48_AVX512VL 1 "register_operand" "v"))
+	    (match_operand:VI48_AVX512VL 2 "nonimmediate_operand" "vm"))
+	  (match_operand:VI48_AVX512VL 3 "vector_move_operand" "0C")
+	  (match_operand:<avx512fmaskmode> 4 "register_operand" "Yk")))]
+  "TARGET_AVX512F"
+  "vpandn<ssemodesuffix>\t{%2, %1, %0%{%4%}%N3|%0%{%4%}%N3, %1, %2}";
+  [(set_attr "type" "sselog")
+   (set_attr "prefix" "evex")
+   (set_attr "mode" "<sseinsnmode>")])
+
+(define_insn "*andnot<mode>3_mask"
+  [(set (match_operand:VI12_AVX512VL 0 "register_operand" "=v")
+	(vec_merge:VI12_AVX512VL
+	  (and:VI12_AVX512VL
+	    (not:VI12_AVX512VL
+	      (match_operand:VI12_AVX512VL 1 "register_operand" "v"))
+	    (match_operand:VI12_AVX512VL 2 "nonimmediate_operand" "vm"))
+	  (match_operand:VI12_AVX512VL 3 "vector_move_operand" "0C")
+	  (match_operand:<avx512fmaskmode> 4 "register_operand" "Yk")))]
+  "TARGET_AVX512BW"
+  "vpandn<ssemodesuffix>\t{%2, %1, %0%{%4%}%N3|%0%{%4%}%N3, %1, %2}";
+  [(set_attr "type" "sselog")
+   (set_attr "prefix" "evex")
+   (set_attr "mode" "<sseinsnmode>")])
+
 (define_expand "<code><mode>3"
   [(set (match_operand:VI 0 "register_operand")
 	(any_logic:VI
@@ -13361,22 +13459,48 @@
    (set (attr "prefix_rex") (symbol_ref "x86_extended_reg_mentioned_p (insn)"))
    (set_attr "mode" "DI")])
 
-(define_insn "<mask_codefor>abs<mode>2<mask_name>"
-  [(set (match_operand:VI124_AVX2_48_AVX512F 0 "register_operand" "=v")
-	(abs:VI124_AVX2_48_AVX512F
-	  (match_operand:VI124_AVX2_48_AVX512F 1 "nonimmediate_operand" "vm")))]
-  "TARGET_SSSE3 && <mask_mode512bit_condition>"
-  "%vpabs<ssemodesuffix>\t{%1, %0<mask_operand2>|%0<mask_operand2>, %1}"
+(define_insn "*abs<mode>2"
+  [(set (match_operand:VI_AVX2 0 "register_operand" "=v")
+	(abs:VI_AVX2
+	  (match_operand:VI_AVX2 1 "nonimmediate_operand" "vm")))]
+  "TARGET_SSSE3"
+  "%vpabs<ssemodesuffix>\t{%1, %0|%0, %1}"
   [(set_attr "type" "sselog1")
    (set_attr "prefix_data16" "1")
    (set_attr "prefix_extra" "1")
    (set_attr "prefix" "maybe_vex")
    (set_attr "mode" "<sseinsnmode>")])
 
+(define_insn "abs<mode>2_mask"
+  [(set (match_operand:VI48_AVX512VL 0 "register_operand" "=v")
+	(vec_merge:VI48_AVX512VL
+	  (abs:VI48_AVX512VL
+	    (match_operand:VI48_AVX512VL 1 "nonimmediate_operand" "vm"))
+	  (match_operand:VI48_AVX512VL 2 "vector_move_operand" "0C")
+	  (match_operand:<avx512fmaskmode> 3 "register_operand" "Yk")))]
+  "TARGET_AVX512F"
+  "vpabs<ssemodesuffix>\t{%1, %0%{%3%}%N2|%0%{%3%}%N2, %1}"
+  [(set_attr "type" "sselog1")
+   (set_attr "prefix" "evex")
+   (set_attr "mode" "<sseinsnmode>")])
+
+(define_insn "abs<mode>2_mask"
+  [(set (match_operand:VI12_AVX512VL 0 "register_operand" "=v")
+	(vec_merge:VI12_AVX512VL
+	  (abs:VI12_AVX512VL
+	    (match_operand:VI12_AVX512VL 1 "nonimmediate_operand" "vm"))
+	  (match_operand:VI12_AVX512VL 2 "vector_move_operand" "0C")
+	  (match_operand:<avx512fmaskmode> 3 "register_operand" "Yk")))]
+  "TARGET_AVX512BW"
+  "vpabs<ssemodesuffix>\t{%1, %0%{%3%}%N2|%0%{%3%}%N2, %1}"
+  [(set_attr "type" "sselog1")
+   (set_attr "prefix" "evex")
+   (set_attr "mode" "<sseinsnmode>")])
+
 (define_expand "abs<mode>2"
-  [(set (match_operand:VI124_AVX2_48_AVX512F 0 "register_operand")
-	(abs:VI124_AVX2_48_AVX512F
-	  (match_operand:VI124_AVX2_48_AVX512F 1 "nonimmediate_operand")))]
+  [(set (match_operand:VI_AVX2 0 "register_operand")
+	(abs:VI_AVX2
+	  (match_operand:VI_AVX2 1 "nonimmediate_operand")))]
   "TARGET_SSE2"
 {
   if (!TARGET_SSSE3)

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH i386 AVX512] [56/n] Add plus/minus/abs/neg/andnot insn patterns.
  2014-10-14  7:20   ` Kirill Yukhin
@ 2014-10-14 18:37     ` Uros Bizjak
  2014-10-20 12:38     ` Jakub Jelinek
  1 sibling, 0 replies; 8+ messages in thread
From: Uros Bizjak @ 2014-10-14 18:37 UTC (permalink / raw)
  To: Kirill Yukhin; +Cc: Jakub Jelinek, Richard Henderson, GCC Patches

On Tue, Oct 14, 2014 at 9:18 AM, Kirill Yukhin <kirill.yukhin@gmail.com> wrote:
> Hello Uroš,
> It seems like I missed to post uppdated patch.
> On 25 Sep 20:11, Uros Bizjak wrote:
>> I'd rather go with the second approach, it is less confusing from the
>> maintainer POV. All other patterns with masking use some consistent
>> template, so I'd suggest using the same approach for everything. If it
>> is indeed too many patterns, then please split the patch to smaller
>> pieces.
> Goal was not to decrease size of the patch, I wanted to make pattern look
> simpler by hiding masking stuff beyond `subst'.
> Anyway, I've updated the patch.
>
> Here it is (bootstrapped and regtested).
>
> Is it ok for trunk?
>
> gcc/
>         * config/i386/sse.md (define_mode_iterator VI_AVX2): Extend
>         to support AVX-512BW.
>         (define_mode_iterator VI124_AVX2_48_AVX512F): Remove.
>         (define_expand "<plusminus_insn><mode>3"): Remove masking support.
>         (define_insn "*<plusminus_insn><mode>3"): Ditto.
>         (define_expand "<plusminus_insn><VI48_AVX512VL:mode>3_mask"): New.
>         (define_expand "<plusminus_insn><VI12_AVX512VL:mode>3_mask"): Ditto.
>         (define_insn "*<plusminus_insn><VI48_AVX512VL:mode>3_mask"): Ditto.
>         (define_insn "*<plusminus_insn><VI12_AVX512VL:mode>3_mask"): Ditto.
>         (define_expand "<sse2_avx2>_andnot<mode>3"): Remove masking support.
>         (define_insn "*andnot<mode>3"): Ditto.
>         (define_expand "<sse2_avx2>_andnot<VI48_AVX512VL:mode>3_mask"): New.
>         (define_expand "<sse2_avx2>_andnot<VI12_AVX512VL:mode>3_mask"): Ditto.
>         (define_insn "*andnot<VI48_AVX512VL:mode>3<mask_name>"): Ditto.
>         (define_insn "*andnot<VI12_AVX512VL:mode>3<mask_name>"): Ditto.
>         (define_insn "*abs<mode>2"): Remove masking support.
>         (define_insn "abs<VI48_AVX512VL:mode>2_mask"): New.
>         (define_insn "abs<VI12_AVX512VL:mode>2_mask"): Ditto.
>         (define_expand "abs<mode>2"): Use VI_AVX2 mode iterator.

IMO, it seems much more readable this way.

OK for mainline.

Thanks,
Uros.

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH i386 AVX512] [56/n] Add plus/minus/abs/neg/andnot insn patterns.
  2014-10-14  7:20   ` Kirill Yukhin
  2014-10-14 18:37     ` Uros Bizjak
@ 2014-10-20 12:38     ` Jakub Jelinek
  2014-10-20 13:41       ` Kirill Yukhin
  1 sibling, 1 reply; 8+ messages in thread
From: Jakub Jelinek @ 2014-10-20 12:38 UTC (permalink / raw)
  To: Kirill Yukhin; +Cc: Uros Bizjak, Richard Henderson, GCC Patches

On Tue, Oct 14, 2014 at 11:18:28AM +0400, Kirill Yukhin wrote:
> 	* config/i386/sse.md (define_mode_iterator VI_AVX2): Extend
> 	to support AVX-512BW.
> 	(define_mode_iterator VI124_AVX2_48_AVX512F): Remove.
> 	(define_expand "<plusminus_insn><mode>3"): Remove masking support.
> 	(define_insn "*<plusminus_insn><mode>3"): Ditto.
> 	(define_expand "<plusminus_insn><VI48_AVX512VL:mode>3_mask"): New.
> 	(define_expand "<plusminus_insn><VI12_AVX512VL:mode>3_mask"): Ditto.
> 	(define_insn "*<plusminus_insn><VI48_AVX512VL:mode>3_mask"): Ditto.
> 	(define_insn "*<plusminus_insn><VI12_AVX512VL:mode>3_mask"): Ditto.
> 	(define_expand "<sse2_avx2>_andnot<mode>3"): Remove masking support.
> 	(define_insn "*andnot<mode>3"): Ditto.
> 	(define_expand "<sse2_avx2>_andnot<VI48_AVX512VL:mode>3_mask"): New.
> 	(define_expand "<sse2_avx2>_andnot<VI12_AVX512VL:mode>3_mask"): Ditto.
> 	(define_insn "*andnot<VI48_AVX512VL:mode>3<mask_name>"): Ditto.
> 	(define_insn "*andnot<VI12_AVX512VL:mode>3<mask_name>"): Ditto.
> 	(define_insn "*abs<mode>2"): Remove masking support.
> 	(define_insn "abs<VI48_AVX512VL:mode>2_mask"): New.
> 	(define_insn "abs<VI12_AVX512VL:mode>2_mask"): Ditto.
> 	(define_expand "abs<mode>2"): Use VI_AVX2 mode iterator.

Unfortunately this caused PR63600.  The problem is that VI_AVX2
mode iterator includes V2DI and for AVX2 also V4DI, but for pre-ssse3
ix86_expand_sse2_abs doesn't handle V2DI (and can't easily, we don't have
PSRAQ instruction), for ssse3 there is no vpabsq instruction, and for
avx2 neither.
We can handle V2DI/V4DI only for TARGET_AVX512VL, and V8DI for
TARGET_AVX512F.
Thus, IMHO the mode iterator on at least
(define_insn "*abs<mode>2"
and on
(define_expand "abs<mode>2"
is wrong, should not include V2DI/V4DI unless TARGET_AVX512VL
(so new (or ressurrected, was that VI124_AVX2_48_AVX512F?)
specialized mode iterator?).

	Jakub

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH i386 AVX512] [56/n] Add plus/minus/abs/neg/andnot insn patterns.
  2014-10-20 12:38     ` Jakub Jelinek
@ 2014-10-20 13:41       ` Kirill Yukhin
  2014-10-20 13:42         ` Jakub Jelinek
  0 siblings, 1 reply; 8+ messages in thread
From: Kirill Yukhin @ 2014-10-20 13:41 UTC (permalink / raw)
  To: Jakub Jelinek; +Cc: Uros Bizjak, Richard Henderson, GCC Patches

Hello,
On 20 Oct 14:36, Jakub Jelinek wrote:
> On Tue, Oct 14, 2014 at 11:18:28AM +0400, Kirill Yukhin wrote:
> > 	* config/i386/sse.md (define_mode_iterator VI_AVX2): Extend
> > 	to support AVX-512BW.
> > 	(define_mode_iterator VI124_AVX2_48_AVX512F): Remove.
> > 	(define_expand "<plusminus_insn><mode>3"): Remove masking support.
> > 	(define_insn "*<plusminus_insn><mode>3"): Ditto.
> > 	(define_expand "<plusminus_insn><VI48_AVX512VL:mode>3_mask"): New.
> > 	(define_expand "<plusminus_insn><VI12_AVX512VL:mode>3_mask"): Ditto.
> > 	(define_insn "*<plusminus_insn><VI48_AVX512VL:mode>3_mask"): Ditto.
> > 	(define_insn "*<plusminus_insn><VI12_AVX512VL:mode>3_mask"): Ditto.
> > 	(define_expand "<sse2_avx2>_andnot<mode>3"): Remove masking support.
> > 	(define_insn "*andnot<mode>3"): Ditto.
> > 	(define_expand "<sse2_avx2>_andnot<VI48_AVX512VL:mode>3_mask"): New.
> > 	(define_expand "<sse2_avx2>_andnot<VI12_AVX512VL:mode>3_mask"): Ditto.
> > 	(define_insn "*andnot<VI48_AVX512VL:mode>3<mask_name>"): Ditto.
> > 	(define_insn "*andnot<VI12_AVX512VL:mode>3<mask_name>"): Ditto.
> > 	(define_insn "*abs<mode>2"): Remove masking support.
> > 	(define_insn "abs<VI48_AVX512VL:mode>2_mask"): New.
> > 	(define_insn "abs<VI12_AVX512VL:mode>2_mask"): Ditto.
> > 	(define_expand "abs<mode>2"): Use VI_AVX2 mode iterator.
> 
> Unfortunately this caused PR63600.  The problem is that VI_AVX2
> mode iterator includes V2DI and for AVX2 also V4DI, but for pre-ssse3
> ix86_expand_sse2_abs doesn't handle V2DI (and can't easily, we don't have
> PSRAQ instruction), for ssse3 there is no vpabsq instruction, and for
> avx2 neither.
> We can handle V2DI/V4DI only for TARGET_AVX512VL, and V8DI for
> TARGET_AVX512F.
> Thus, IMHO the mode iterator on at least
> (define_insn "*abs<mode>2"
> and on
> (define_expand "abs<mode>2"
> is wrong, should not include V2DI/V4DI unless TARGET_AVX512VL
> (so new (or ressurrected, was that VI124_AVX2_48_AVX512F?)
> specialized mode iterator?).


This patch removes absq insn patterns for non-AVX-512 targets.


gcc/
	* config/i386/sse.md (define_mode_iterator VI_AVX2): Restore to 128-,
	256- bit integer modes only.
	(define_mode_iterator VI_AVX2_AVX512): New.
	(define_expand "neg<mode>2"): Use VI_AVX2_AVX512 mode iterator.
	(define_expand "<plusminus_insn><mode>3"): Ditto.
	(define_insn "*<plusminus_insn><mode>3"): Ditto.
	(define_expand "<sse2_avx2>_andnot<mode>3"): Ditto.
	(define_mode_iterator VI1248_AVX512VL_AVX512BW): New.
	(define_insn "abs<VI1248_AVX512VL_AVX512BW:mode>2"): Ditto.

Bootstrap in progress. AVX-512 tests pass.

Is it ok for trunk?

--
Thanks, K

    AVX-512. Disable absq for non AVX-512 targets.

diff --git a/gcc/config/i386/sse.md b/gcc/config/i386/sse.md
index fd40623..74aca48 100644
--- a/gcc/config/i386/sse.md
+++ b/gcc/config/i386/sse.md
@@ -271,6 +271,11 @@
    (V4DI "TARGET_AVX") V2DI])
 
 (define_mode_iterator VI_AVX2
+  [(V32QI "TARGET_AVX2") V16QI
+   (V16HI "TARGET_AVX2") V8HI
+   (V8SI "TARGET_AVX2") V4SI])
+
+(define_mode_iterator VI_AVX2_AVX512
   [(V64QI "TARGET_AVX512BW") (V32QI "TARGET_AVX2") V16QI
    (V32HI "TARGET_AVX512BW") (V16HI "TARGET_AVX2") V8HI
    (V16SI "TARGET_AVX512F") (V8SI "TARGET_AVX2") V4SI
@@ -9142,18 +9147,18 @@
 ;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;
 
 (define_expand "neg<mode>2"
-  [(set (match_operand:VI_AVX2 0 "register_operand")
-	(minus:VI_AVX2
+  [(set (match_operand:VI_AVX2_AVX512 0 "register_operand")
+	(minus:VI_AVX2_AVX512
 	  (match_dup 2)
-	  (match_operand:VI_AVX2 1 "nonimmediate_operand")))]
+	  (match_operand:VI_AVX2_AVX512 1 "nonimmediate_operand")))]
   "TARGET_SSE2"
   "operands[2] = force_reg (<MODE>mode, CONST0_RTX (<MODE>mode));")
 
 (define_expand "<plusminus_insn><mode>3"
-  [(set (match_operand:VI_AVX2 0 "register_operand")
-	(plusminus:VI_AVX2
-	  (match_operand:VI_AVX2 1 "nonimmediate_operand")
-	  (match_operand:VI_AVX2 2 "nonimmediate_operand")))]
+  [(set (match_operand:VI_AVX2_AVX512 0 "register_operand")
+	(plusminus:VI_AVX2_AVX512
+	  (match_operand:VI_AVX2_AVX512 1 "nonimmediate_operand")
+	  (match_operand:VI_AVX2_AVX512 2 "nonimmediate_operand")))]
   "TARGET_SSE2"
   "ix86_fixup_binary_operands_no_copy (<CODE>, <MODE>mode, operands);")
 
@@ -9180,10 +9185,10 @@
   "ix86_fixup_binary_operands_no_copy (<CODE>, <MODE>mode, operands);")
 
 (define_insn "*<plusminus_insn><mode>3"
-  [(set (match_operand:VI_AVX2 0 "register_operand" "=x,v")
-	(plusminus:VI_AVX2
-	  (match_operand:VI_AVX2 1 "nonimmediate_operand" "<comm>0,v")
-	  (match_operand:VI_AVX2 2 "nonimmediate_operand" "xm,vm")))]
+  [(set (match_operand:VI_AVX2_AVX512 0 "register_operand" "=x,v")
+	(plusminus:VI_AVX2_AVX512
+	  (match_operand:VI_AVX2_AVX512 1 "nonimmediate_operand" "<comm>0,v")
+	  (match_operand:VI_AVX2_AVX512 2 "nonimmediate_operand" "xm,vm")))]
   "TARGET_SSE2
    && ix86_binary_operator_ok (<CODE>, <MODE>mode, operands)"
   "@
@@ -10715,10 +10720,10 @@
 })
 
 (define_expand "<sse2_avx2>_andnot<mode>3"
-  [(set (match_operand:VI_AVX2 0 "register_operand")
-	(and:VI_AVX2
-	  (not:VI_AVX2 (match_operand:VI_AVX2 1 "register_operand"))
-	  (match_operand:VI_AVX2 2 "nonimmediate_operand")))]
+  [(set (match_operand:VI_AVX2_AVX512 0 "register_operand")
+	(and:VI_AVX2_AVX512
+	  (not:VI_AVX2_AVX512 (match_operand:VI_AVX2_AVX512 1 "register_operand"))
+	  (match_operand:VI_AVX2_AVX512 2 "nonimmediate_operand")))]
   "TARGET_SSE2")
 
 (define_expand "<sse2_avx2>_andnot<mode>3_mask"
@@ -13836,6 +13841,20 @@
     }
 })
 
+(define_mode_iterator VI1248_AVX512VL_AVX512BW
+   [V8DI (V4DI "TARGET_AVX512VL") (V2DI "TARGET_AVX512VL")
+    V16SI (V64QI "TARGET_AVX512BW") (V32HI "TARGET_AVX512BW")])
+
+(define_insn "abs<mode>2"
+  [(set (match_operand:VI1248_AVX512VL_AVX512BW 0 "register_operand" "=v")
+	(abs:VI1248_AVX512VL_AVX512BW
+	  (match_operand:VI1248_AVX512VL_AVX512BW 1 "nonimmediate_operand" "vm")))]
+  "TARGET_AVX512F"
+  "vpabs<ssemodesuffix>\t{%1, %0|%0, %1}"
+  [(set_attr "type" "sselog1")
+   (set_attr "prefix" "evex")
+   (set_attr "mode" "<sseinsnmode>")])
+
 (define_insn "abs<mode>2"
   [(set (match_operand:MMXMODEI 0 "register_operand" "=y")
 	(abs:MMXMODEI

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH i386 AVX512] [56/n] Add plus/minus/abs/neg/andnot insn patterns.
  2014-10-20 13:41       ` Kirill Yukhin
@ 2014-10-20 13:42         ` Jakub Jelinek
  2014-10-20 15:05           ` Uros Bizjak
  0 siblings, 1 reply; 8+ messages in thread
From: Jakub Jelinek @ 2014-10-20 13:42 UTC (permalink / raw)
  To: Kirill Yukhin; +Cc: Uros Bizjak, Richard Henderson, GCC Patches

On Mon, Oct 20, 2014 at 05:30:36PM +0400, Kirill Yukhin wrote:
> > Unfortunately this caused PR63600.  The problem is that VI_AVX2
> > mode iterator includes V2DI and for AVX2 also V4DI, but for pre-ssse3
> > ix86_expand_sse2_abs doesn't handle V2DI (and can't easily, we don't have
> > PSRAQ instruction), for ssse3 there is no vpabsq instruction, and for
> > avx2 neither.
> > We can handle V2DI/V4DI only for TARGET_AVX512VL, and V8DI for
> > TARGET_AVX512F.
> > Thus, IMHO the mode iterator on at least
> > (define_insn "*abs<mode>2"
> > and on
> > (define_expand "abs<mode>2"
> > is wrong, should not include V2DI/V4DI unless TARGET_AVX512VL
> > (so new (or ressurrected, was that VI124_AVX2_48_AVX512F?)
> > specialized mode iterator?).
> 
> 
> This patch removes absq insn patterns for non-AVX-512 targets.
> 
> 
> gcc/
> 	* config/i386/sse.md (define_mode_iterator VI_AVX2): Restore to 128-,
> 	256- bit integer modes only.
> 	(define_mode_iterator VI_AVX2_AVX512): New.
> 	(define_expand "neg<mode>2"): Use VI_AVX2_AVX512 mode iterator.
> 	(define_expand "<plusminus_insn><mode>3"): Ditto.
> 	(define_insn "*<plusminus_insn><mode>3"): Ditto.
> 	(define_expand "<sse2_avx2>_andnot<mode>3"): Ditto.
> 	(define_mode_iterator VI1248_AVX512VL_AVX512BW): New.
> 	(define_insn "abs<VI1248_AVX512VL_AVX512BW:mode>2"): Ditto.
> 
> Bootstrap in progress. AVX-512 tests pass.
> 
> Is it ok for trunk?

I'll certainly leave the review to Uros, whatever he prefers.
That said, I was expecting you'd keep VI_AVX2 as is (because from the patch
clearly that is what is used most commonly, the V?DI modes are for most
insns normal integral vector modes, VI* uses the same modes and VI_AVX2
used to be just like VI, just with TARGET_AVX conditions replaced with
TARGET_AVX2), and just add a new mode iterator for the two abs patterns
(*abs<mode>2 and abs<mode>2), it can be specialized mode iterator just
for the abs with ABS in names or something.

	Jakub

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH i386 AVX512] [56/n] Add plus/minus/abs/neg/andnot insn patterns.
  2014-10-20 13:42         ` Jakub Jelinek
@ 2014-10-20 15:05           ` Uros Bizjak
  0 siblings, 0 replies; 8+ messages in thread
From: Uros Bizjak @ 2014-10-20 15:05 UTC (permalink / raw)
  To: Jakub Jelinek; +Cc: Kirill Yukhin, Richard Henderson, GCC Patches

On Mon, Oct 20, 2014 at 3:41 PM, Jakub Jelinek <jakub@redhat.com> wrote:
> On Mon, Oct 20, 2014 at 05:30:36PM +0400, Kirill Yukhin wrote:
>> > Unfortunately this caused PR63600.  The problem is that VI_AVX2
>> > mode iterator includes V2DI and for AVX2 also V4DI, but for pre-ssse3
>> > ix86_expand_sse2_abs doesn't handle V2DI (and can't easily, we don't have
>> > PSRAQ instruction), for ssse3 there is no vpabsq instruction, and for
>> > avx2 neither.
>> > We can handle V2DI/V4DI only for TARGET_AVX512VL, and V8DI for
>> > TARGET_AVX512F.
>> > Thus, IMHO the mode iterator on at least
>> > (define_insn "*abs<mode>2"
>> > and on
>> > (define_expand "abs<mode>2"
>> > is wrong, should not include V2DI/V4DI unless TARGET_AVX512VL
>> > (so new (or ressurrected, was that VI124_AVX2_48_AVX512F?)
>> > specialized mode iterator?).
>>
>>
>> This patch removes absq insn patterns for non-AVX-512 targets.
>>
>>
>> gcc/
>>       * config/i386/sse.md (define_mode_iterator VI_AVX2): Restore to 128-,
>>       256- bit integer modes only.
>>       (define_mode_iterator VI_AVX2_AVX512): New.
>>       (define_expand "neg<mode>2"): Use VI_AVX2_AVX512 mode iterator.
>>       (define_expand "<plusminus_insn><mode>3"): Ditto.
>>       (define_insn "*<plusminus_insn><mode>3"): Ditto.
>>       (define_expand "<sse2_avx2>_andnot<mode>3"): Ditto.
>>       (define_mode_iterator VI1248_AVX512VL_AVX512BW): New.
>>       (define_insn "abs<VI1248_AVX512VL_AVX512BW:mode>2"): Ditto.
>>
>> Bootstrap in progress. AVX-512 tests pass.
>>
>> Is it ok for trunk?
>
> I'll certainly leave the review to Uros, whatever he prefers.
> That said, I was expecting you'd keep VI_AVX2 as is (because from the patch
> clearly that is what is used most commonly, the V?DI modes are for most
> insns normal integral vector modes, VI* uses the same modes and VI_AVX2
> used to be just like VI, just with TARGET_AVX conditions replaced with
> TARGET_AVX2), and just add a new mode iterator for the two abs patterns
> (*abs<mode>2 and abs<mode>2), it can be specialized mode iterator just
> for the abs with ABS in names or something.

Yes, I like this idea, too.

Just add IV1248_AVX512VL_AVX512BW and use it in abs patterns.

The changed patch is pre-approved, but please still make full
bootstrap and regtest cycle.

Thanks,
Uros.

^ permalink raw reply	[flat|nested] 8+ messages in thread

end of thread, other threads:[~2014-10-20 14:58 UTC | newest]

Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2014-09-25 14:12 [PATCH i386 AVX512] [56/n] Add plus/minus/abs/neg/andnot insn patterns Kirill Yukhin
2014-09-25 18:12 ` Uros Bizjak
2014-10-14  7:20   ` Kirill Yukhin
2014-10-14 18:37     ` Uros Bizjak
2014-10-20 12:38     ` Jakub Jelinek
2014-10-20 13:41       ` Kirill Yukhin
2014-10-20 13:42         ` Jakub Jelinek
2014-10-20 15:05           ` Uros Bizjak

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).