public inbox for gcc-patches@gcc.gnu.org
 help / color / mirror / Atom feed
* v{extract,insert,broadcast,perm2}{i,f}128
@ 2011-09-19 20:16 Jakub Jelinek
  2011-09-20  8:03 ` v{extract,insert,broadcast,perm2}{i,f}128 Uros Bizjak
  0 siblings, 1 reply; 9+ messages in thread
From: Jakub Jelinek @ 2011-09-19 20:16 UTC (permalink / raw)
  To: Uros Bizjak; +Cc: gcc-patches

Hi!

AVX2 prefers v*i128 instructions for vector integer
modes, while AVX only supports v*f128.
In one of my recent patches I've changed vextract* so that
it emits v*i128 instead of v*f128 for AVX2 on integer modes,
but I wonder what is the prefered way to change the rest
of the insn.

E.g. we have:
(define_insn "avx_vbroadcastf128_<mode>"
  [(set (match_operand:V_256 0 "register_operand" "=x,x,x")
        (vec_concat:V_256
          (match_operand:<ssehalfvecmode> 1 "nonimmediate_operand" "m,0,?x")
          (match_dup 1)))]
  "TARGET_AVX"
  "@
   vbroadcastf128\t{%1, %0|%0, %1}
   vinsertf128\t{$1, %1, %0, %0|%0, %0, %1, 1}
   vperm2f128\t{$0, %t1, %t1, %0|%0, %t1, %t1, 0}"
  [(set_attr "type" "ssemov,sselog1,sselog1")
   (set_attr "prefix_extra" "1")
   (set_attr "length_immediate" "0,1,1")
   (set_attr "prefix" "vex")
   (set_attr "mode" "V4SF,V8SF,V8SF")])

One option is change the mode iterator, so that it
iterates on all 256-bit vectors for !TARGET_AVX2, and only
for V8SF and V4DF for TARGET_AVX2, then for each such an insn also
add similar insn that iterates on the 256-bit integer vectors,
is guarded by TARGET_AVX2 and uses *i128 and OI mode attribute instead of
V8SF or V4DF.

Or, e.g. we could have operand modifier that would print "i128"
resp. "i" for TARGET_AVX2 and integer vector modes, and would print
"f128" resp. "f" otherwise.  Still, something would need to be done
about the mode attribute.

Or something else?

	Jakub

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

* Re: v{extract,insert,broadcast,perm2}{i,f}128
  2011-09-19 20:16 v{extract,insert,broadcast,perm2}{i,f}128 Jakub Jelinek
@ 2011-09-20  8:03 ` Uros Bizjak
  2011-09-20 13:54   ` v{extract,insert,broadcast,perm2}{i,f}128 Jakub Jelinek
  0 siblings, 1 reply; 9+ messages in thread
From: Uros Bizjak @ 2011-09-20  8:03 UTC (permalink / raw)
  To: Jakub Jelinek; +Cc: gcc-patches

On Mon, Sep 19, 2011 at 9:08 PM, Jakub Jelinek <jakub@redhat.com> wrote:

> AVX2 prefers v*i128 instructions for vector integer
> modes, while AVX only supports v*f128.
> In one of my recent patches I've changed vextract* so that
> it emits v*i128 instead of v*f128 for AVX2 on integer modes,
> but I wonder what is the prefered way to change the rest
> of the insn.
>
> E.g. we have:
> (define_insn "avx_vbroadcastf128_<mode>"
>  [(set (match_operand:V_256 0 "register_operand" "=x,x,x")
>        (vec_concat:V_256
>          (match_operand:<ssehalfvecmode> 1 "nonimmediate_operand" "m,0,?x")
>          (match_dup 1)))]
>  "TARGET_AVX"
>  "@
>   vbroadcastf128\t{%1, %0|%0, %1}
>   vinsertf128\t{$1, %1, %0, %0|%0, %0, %1, 1}
>   vperm2f128\t{$0, %t1, %t1, %0|%0, %t1, %t1, 0}"
>  [(set_attr "type" "ssemov,sselog1,sselog1")
>   (set_attr "prefix_extra" "1")
>   (set_attr "length_immediate" "0,1,1")
>   (set_attr "prefix" "vex")
>   (set_attr "mode" "V4SF,V8SF,V8SF")])
>
> One option is change the mode iterator, so that it
> iterates on all 256-bit vectors for !TARGET_AVX2, and only
> for V8SF and V4DF for TARGET_AVX2, then for each such an insn also
> add similar insn that iterates on the 256-bit integer vectors,
> is guarded by TARGET_AVX2 and uses *i128 and OI mode attribute instead of
> V8SF or V4DF.
>
> Or, e.g. we could have operand modifier that would print "i128"
> resp. "i" for TARGET_AVX2 and integer vector modes, and would print
> "f128" resp. "f" otherwise.  Still, something would need to be done
> about the mode attribute.
>
> Or something else?

Perhaps use mode attribute that defines "f" for FP modes and "%i" for
integer modes. "%i" is further processed in ix86_output_operand  (or
perhaps ASM_OUTPUT_OPCODE, similar to %v ?) to output "i" for
TARGET_AVX2 or "f" otherwise.

Uros.

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

* Re: v{extract,insert,broadcast,perm2}{i,f}128
  2011-09-20  8:03 ` v{extract,insert,broadcast,perm2}{i,f}128 Uros Bizjak
@ 2011-09-20 13:54   ` Jakub Jelinek
  2011-09-20 14:00     ` v{extract,insert,broadcast,perm2}{i,f}128 Uros Bizjak
  0 siblings, 1 reply; 9+ messages in thread
From: Jakub Jelinek @ 2011-09-20 13:54 UTC (permalink / raw)
  To: Uros Bizjak; +Cc: gcc-patches

On Tue, Sep 20, 2011 at 07:15:42AM +0200, Uros Bizjak wrote:
> Perhaps use mode attribute that defines "f" for FP modes and "%i" for
> integer modes. "%i" is further processed in ix86_output_operand  (or
> perhaps ASM_OUTPUT_OPCODE, similar to %v ?) to output "i" for
> TARGET_AVX2 or "f" otherwise.
> 

We can do something like following for the patterns (it can't be
like %v, because it is not at the beginning of the insn, so it either
could be some punctuation character without operand (but no idea
where would it take the mode from), or as this patch, which has
an operand number from which to take the mode - but as the code
uses strtoul to parse the number, we can't have %I0128, so it can't
expand just to i/f, but to i128/f128.

But still no idea how to do it nicely for the "mode" attribute,
define_mode_attr can't be conditional.  Using if_then_else
for "mode" attribute is of course possible, but ugly.

--- gcc/config/i386/i386.c.jj	2011-09-18 21:20:04.000000000 +0200
+++ gcc/config/i386/i386.c	2011-09-20 15:20:41.000000000 +0200
@@ -13511,6 +13511,8 @@ get_some_local_dynamic_name (void)
    & -- print some in-use local-dynamic symbol name.
    H -- print a memory address offset by 8; used for sse high-parts
    Y -- print condition for XOP pcom* instruction.
+   I -- if !TARGET_AVX2 or non-integer vector mode, expand to "f128",
+        otherwise expand to "i128".
    + -- print a branch hint as 'cs' or 'ds' prefix
    ; -- print a semicolon (after prefixes due to bug in older gas).
    @ -- print a segment register of thread base pointer load
@@ -14006,6 +14008,15 @@ ix86_print_operand (FILE *file, rtx x, i
 	    fputs ("gs", file);
 	  return;
 
+	case 'I':
+	  /* %I can be used to print i128 for AVX2 and integral modes,
+	     and f128 otherwise.  */
+	  if (TARGET_AVX2 && GET_MODE_CLASS (GET_MODE (x)) == MODE_VECTOR_INT)
+	    fputs ("i128", file);
+	  else
+	    fputs ("f128", file);
+	  return;
+
 	default:
 	    output_operand_lossage ("invalid operand code '%c'", code);
 	}
--- gcc/config/i386/sse.md.jj	2011-09-19 17:43:35.000000000 +0200
+++ gcc/config/i386/sse.md	2011-09-20 15:28:31.000000000 +0200
@@ -3846,12 +3846,7 @@ (define_insn "vec_extract_hi_<mode>"
 	  (match_operand:VI8F_256 1 "register_operand" "x,x")
 	  (parallel [(const_int 2) (const_int 3)])))]
   "TARGET_AVX"
-{
-  if (get_attr_mode (insn) == MODE_OI)
-    return "vextracti128\t{$0x1, %1, %0|%0, %1, 0x1}";
-  else
-    return "vextractf128\t{$0x1, %1, %0|%0, %1, 0x1}";
-}
+  "vextract%I1\t{$0x1, %1, %0|%0, %1, 0x1}"
   [(set_attr "type" "sselog")
    (set_attr "prefix_extra" "1")
    (set_attr "length_immediate" "1")
@@ -3891,12 +3886,7 @@ (define_insn "vec_extract_hi_<mode>"
 	  (parallel [(const_int 4) (const_int 5)
 		     (const_int 6) (const_int 7)])))]
   "TARGET_AVX"
-{
-  if (get_attr_mode (insn) == MODE_OI)
-    return "vextracti128\t{$0x1, %1, %0|%0, %1, 0x1}";
-  else
-    return "vextractf128\t{$0x1, %1, %0|%0, %1, 0x1}";
-}
+  "vextract%I1\t{$0x1, %1, %0|%0, %1, 0x1}"
   [(set_attr "type" "sselog")
    (set_attr "prefix_extra" "1")
    (set_attr "length_immediate" "1")
@@ -3940,12 +3930,7 @@ (define_insn "vec_extract_hi_v16hi"
 		     (const_int 12) (const_int 13)
 		     (const_int 14) (const_int 15)])))]
   "TARGET_AVX"
-{
-  if (get_attr_mode (insn) == MODE_OI)
-    return "vextracti128\t{$0x1, %1, %0|%0, %1, 0x1}";
-  else
-    return "vextractf128\t{$0x1, %1, %0|%0, %1, 0x1}";
-}
+  "vextract%I1\t{$0x1, %1, %0|%0, %1, 0x1}"
   [(set_attr "type" "sselog")
    (set_attr "prefix_extra" "1")
    (set_attr "length_immediate" "1")
@@ -3995,12 +3980,7 @@ (define_insn "vec_extract_hi_v32qi"
 		     (const_int 28) (const_int 29)
 		     (const_int 30) (const_int 31)])))]
   "TARGET_AVX"
-{
-  if (get_attr_mode (insn) == MODE_OI)
-    return "vextracti128\t{$0x1, %1, %0|%0, %1, 0x1}";
-  else
-    return "vextractf128\t{$0x1, %1, %0|%0, %1, 0x1}";
-}
+  "vextract%I1\t{$0x1, %1, %0|%0, %1, 0x1}"
   [(set_attr "type" "sselog")
    (set_attr "prefix_extra" "1")
    (set_attr "length_immediate" "1")
@@ -11672,9 +11652,9 @@ (define_insn "avx_vbroadcastf128_<mode>"
 	  (match_dup 1)))]
   "TARGET_AVX"
   "@
-   vbroadcastf128\t{%1, %0|%0, %1}
-   vinsertf128\t{$1, %1, %0, %0|%0, %0, %1, 1}
-   vperm2f128\t{$0, %t1, %t1, %0|%0, %t1, %t1, 0}"
+   vbroadcast%I0\t{%1, %0|%0, %1}
+   vinsert%I0\t{$1, %1, %0, %0|%0, %0, %1, 1}
+   vperm2%I0\t{$0, %t1, %t1, %0|%0, %t1, %t1, 0}"
   [(set_attr "type" "ssemov,sselog1,sselog1")
    (set_attr "prefix_extra" "1")
    (set_attr "length_immediate" "0,1,1")
@@ -11873,7 +11853,7 @@ (define_insn "*avx_vperm2f128<mode>_full
 	   (match_operand:SI 3 "const_0_to_255_operand" "n")]
 	  UNSPEC_VPERMIL2F128))]
   "TARGET_AVX"
-  "vperm2f128\t{%3, %2, %1, %0|%0, %1, %2, %3}"
+  "vperm2%I0\t{%3, %2, %1, %0|%0, %1, %2, %3}"
   [(set_attr "type" "sselog")
    (set_attr "prefix_extra" "1")
    (set_attr "length_immediate" "1")
@@ -11893,7 +11873,7 @@ (define_insn "*avx_vperm2f128<mode>_noze
 {
   int mask = avx_vperm2f128_parallel (operands[3], <MODE>mode) - 1;
   operands[3] = GEN_INT (mask);
-  return "vperm2f128\t{%3, %2, %1, %0|%0, %1, %2, %3}";
+  return "vperm2%I0\t{%3, %2, %1, %0|%0, %1, %2, %3}";
 }
   [(set_attr "type" "sselog")
    (set_attr "prefix_extra" "1")
@@ -11964,7 +11944,7 @@ (define_insn "vec_set_lo_<mode>"
 	    (match_operand:VI8F_256 1 "register_operand" "x")
 	    (parallel [(const_int 2) (const_int 3)]))))]
   "TARGET_AVX"
-  "vinsertf128\t{$0x0, %2, %1, %0|%0, %1, %2, 0x0}"
+  "vinsert%I0\t{$0x0, %2, %1, %0|%0, %1, %2, 0x0}"
   [(set_attr "type" "sselog")
    (set_attr "prefix_extra" "1")
    (set_attr "length_immediate" "1")
@@ -11979,7 +11959,7 @@ (define_insn "vec_set_hi_<mode>"
 	    (parallel [(const_int 0) (const_int 1)]))
 	  (match_operand:<ssehalfvecmode> 2 "nonimmediate_operand" "xm")))]
   "TARGET_AVX"
-  "vinsertf128\t{$0x1, %2, %1, %0|%0, %1, %2, 0x1}"
+  "vinsert%I0\t{$0x1, %2, %1, %0|%0, %1, %2, 0x1}"
   [(set_attr "type" "sselog")
    (set_attr "prefix_extra" "1")
    (set_attr "length_immediate" "1")
@@ -11995,7 +11975,7 @@ (define_insn "vec_set_lo_<mode>"
 	    (parallel [(const_int 4) (const_int 5)
 		       (const_int 6) (const_int 7)]))))]
   "TARGET_AVX"
-  "vinsertf128\t{$0x0, %2, %1, %0|%0, %1, %2, 0x0}"
+  "vinsert%I0\t{$0x0, %2, %1, %0|%0, %1, %2, 0x0}"
   [(set_attr "type" "sselog")
    (set_attr "prefix_extra" "1")
    (set_attr "length_immediate" "1")
@@ -12011,7 +11991,7 @@ (define_insn "vec_set_hi_<mode>"
 		       (const_int 2) (const_int 3)]))
 	  (match_operand:<ssehalfvecmode> 2 "nonimmediate_operand" "xm")))]
   "TARGET_AVX"
-  "vinsertf128\t{$0x1, %2, %1, %0|%0, %1, %2, 0x1}"
+  "vinsert%I0\t{$0x1, %2, %1, %0|%0, %1, %2, 0x1}"
   [(set_attr "type" "sselog")
    (set_attr "prefix_extra" "1")
    (set_attr "length_immediate" "1")
@@ -12029,7 +12009,7 @@ (define_insn "vec_set_lo_v16hi"
 		       (const_int 12) (const_int 13)
 		       (const_int 14) (const_int 15)]))))]
   "TARGET_AVX"
-  "vinsertf128\t{$0x0, %2, %1, %0|%0, %1, %2, 0x0}"
+  "vinsert%I0\t{$0x0, %2, %1, %0|%0, %1, %2, 0x0}"
   [(set_attr "type" "sselog")
    (set_attr "prefix_extra" "1")
    (set_attr "length_immediate" "1")
@@ -12047,7 +12027,7 @@ (define_insn "vec_set_hi_v16hi"
 		       (const_int 6) (const_int 7)]))
 	  (match_operand:V8HI 2 "nonimmediate_operand" "xm")))]
   "TARGET_AVX"
-  "vinsertf128\t{$0x1, %2, %1, %0|%0, %1, %2, 0x1}"
+  "vinsert%I0\t{$0x1, %2, %1, %0|%0, %1, %2, 0x1}"
   [(set_attr "type" "sselog")
    (set_attr "prefix_extra" "1")
    (set_attr "length_immediate" "1")
@@ -12069,7 +12049,7 @@ (define_insn "vec_set_lo_v32qi"
 		       (const_int 28) (const_int 29)
 		       (const_int 30) (const_int 31)]))))]
   "TARGET_AVX"
-  "vinsertf128\t{$0x0, %2, %1, %0|%0, %1, %2, 0x0}"
+  "vinsert%I0\t{$0x0, %2, %1, %0|%0, %1, %2, 0x0}"
   [(set_attr "type" "sselog")
    (set_attr "prefix_extra" "1")
    (set_attr "length_immediate" "1")
@@ -12091,7 +12071,7 @@ (define_insn "vec_set_hi_v32qi"
 		       (const_int 14) (const_int 15)]))
 	  (match_operand:V16QI 2 "nonimmediate_operand" "xm")))]
   "TARGET_AVX"
-  "vinsertf128\t{$0x1, %2, %1, %0|%0, %1, %2, 0x1}"
+  "vinsert%I0\t{$0x1, %2, %1, %0|%0, %1, %2, 0x1}"
   [(set_attr "type" "sselog")
    (set_attr "prefix_extra" "1")
    (set_attr "length_immediate" "1")
@@ -12477,7 +12457,7 @@ (define_insn "*vec_concat<mode>_avx"
   switch (which_alternative)
     {
     case 0:
-      return "vinsertf128\t{$0x1, %2, %t1, %0|%0, %t1, %2, 0x1}";
+      return "vinsert%I0\t{$0x1, %2, %t1, %0|%0, %t1, %2, 0x1}";
     case 1:
       switch (get_attr_mode (insn))
 	{

	Jakub

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

* Re: v{extract,insert,broadcast,perm2}{i,f}128
  2011-09-20 13:54   ` v{extract,insert,broadcast,perm2}{i,f}128 Jakub Jelinek
@ 2011-09-20 14:00     ` Uros Bizjak
  2011-09-20 14:11       ` v{extract,insert,broadcast,perm2}{i,f}128 Jakub Jelinek
  0 siblings, 1 reply; 9+ messages in thread
From: Uros Bizjak @ 2011-09-20 14:00 UTC (permalink / raw)
  To: Jakub Jelinek; +Cc: gcc-patches

On Tue, Sep 20, 2011 at 3:38 PM, Jakub Jelinek <jakub@redhat.com> wrote:
> On Tue, Sep 20, 2011 at 07:15:42AM +0200, Uros Bizjak wrote:
>> Perhaps use mode attribute that defines "f" for FP modes and "%i" for
>> integer modes. "%i" is further processed in ix86_output_operand  (or
>> perhaps ASM_OUTPUT_OPCODE, similar to %v ?) to output "i" for
>> TARGET_AVX2 or "f" otherwise.
>>
>
> We can do something like following for the patterns (it can't be
> like %v, because it is not at the beginning of the insn, so it either
> could be some punctuation character without operand (but no idea
> where would it take the mode from), or as this patch, which has
> an operand number from which to take the mode - but as the code
> uses strtoul to parse the number, we can't have %I0128, so it can't
> expand just to i/f, but to i128/f128.
>
> But still no idea how to do it nicely for the "mode" attribute,
> define_mode_attr can't be conditional.  Using if_then_else
> for "mode" attribute is of course possible, but ugly.

No, you define mode attribute like

(define_mode_attr somesuffix [(V2DI "%i") (V2DF "f") ...])

where integer modes map to %i and FP modes to "f".

Then, just return "vextract<somesuffix>128\t{$0x1, %1, %0|%0, %1, 0x1}"

(You can use punctuation char in place of %i, it doesn't depend on
operands at all.)

Uros.

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

* Re: v{extract,insert,broadcast,perm2}{i,f}128
  2011-09-20 14:00     ` v{extract,insert,broadcast,perm2}{i,f}128 Uros Bizjak
@ 2011-09-20 14:11       ` Jakub Jelinek
  2011-09-20 14:20         ` v{extract,insert,broadcast,perm2}{i,f}128 Uros Bizjak
  0 siblings, 1 reply; 9+ messages in thread
From: Jakub Jelinek @ 2011-09-20 14:11 UTC (permalink / raw)
  To: Uros Bizjak; +Cc: gcc-patches

On Tue, Sep 20, 2011 at 03:47:56PM +0200, Uros Bizjak wrote:
> No, you define mode attribute like
> 
> (define_mode_attr somesuffix [(V2DI "%i") (V2DF "f") ...])
> 
> where integer modes map to %i and FP modes to "f".
> 
> Then, just return "vextract<somesuffix>128\t{$0x1, %1, %0|%0, %1, 0x1}"

I can surely do that (or e.g.
(define_mode_attr i128 [(V4DI "%~128") (V4DF "f128") ...])
and
"vextract<i128>\t{$0x1, %1, %0|%0, %1, 0x1}"
)
if you prefer it that way, but that is a functional alternative
to the patch I've just posted.  What I don't know how to express
right now is that:
   (set (attr "mode")
     (if_then_else
       (and (match_test "TARGET_AVX2")
            (eq (const_string "<MODE>mode") (const_string "V4DImode")))
     (const_string "OI")
     (const_string "V4DF"))
I've used on some insns, i.e. for either all alternatives (or
just the ones that use <i128> insns), use MODE_OI if TARGET_AVX2
and MODE_VECTOR_INT, or MODE_V4DF resp. MODE_V8SF (what has been used
until now).  No idea whether something uses get_mode_attr for anything
scheduling related (or length etc.), or whether just lying that it is
MODE_V4DF or MODE_V8SF even for vector integer modes with TARGET_AVX2
is ok.

	Jakub

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

* Re: v{extract,insert,broadcast,perm2}{i,f}128
  2011-09-20 14:11       ` v{extract,insert,broadcast,perm2}{i,f}128 Jakub Jelinek
@ 2011-09-20 14:20         ` Uros Bizjak
  2011-09-20 15:03           ` v{extract,insert,broadcast,perm2}{i,f}128 Uros Bizjak
  0 siblings, 1 reply; 9+ messages in thread
From: Uros Bizjak @ 2011-09-20 14:20 UTC (permalink / raw)
  To: Jakub Jelinek; +Cc: gcc-patches

On Tue, Sep 20, 2011 at 4:00 PM, Jakub Jelinek <jakub@redhat.com> wrote:
> On Tue, Sep 20, 2011 at 03:47:56PM +0200, Uros Bizjak wrote:
>> No, you define mode attribute like
>>
>> (define_mode_attr somesuffix [(V2DI "%i") (V2DF "f") ...])
>>
>> where integer modes map to %i and FP modes to "f".
>>
>> Then, just return "vextract<somesuffix>128\t{$0x1, %1, %0|%0, %1, 0x1}"
>
> I can surely do that (or e.g.
> (define_mode_attr i128 [(V4DI "%~128") (V4DF "f128") ...])
> and
> "vextract<i128>\t{$0x1, %1, %0|%0, %1, 0x1}"
> )
> if you prefer it that way, but that is a functional alternative
> to the patch I've just posted.  What I don't know how to express
> right now is that:
>   (set (attr "mode")
>     (if_then_else
>       (and (match_test "TARGET_AVX2")
>            (eq (const_string "<MODE>mode") (const_string "V4DImode")))
>     (const_string "OI")
>     (const_string "V4DF"))
> I've used on some insns, i.e. for either all alternatives (or
> just the ones that use <i128> insns), use MODE_OI if TARGET_AVX2
> and MODE_VECTOR_INT, or MODE_V4DF resp. MODE_V8SF (what has been used
> until now).  No idea whether something uses get_mode_attr for anything
> scheduling related (or length etc.), or whether just lying that it is
> MODE_V4DF or MODE_V8SF even for vector integer modes with TARGET_AVX2
> is ok.

mode attribute us used only to calculate various prefixes (and that
should be reviewed anyway). But for modes you are referring to, it
doesn't matter that much.

Uros.

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

* Re: v{extract,insert,broadcast,perm2}{i,f}128
  2011-09-20 14:20         ` v{extract,insert,broadcast,perm2}{i,f}128 Uros Bizjak
@ 2011-09-20 15:03           ` Uros Bizjak
  2011-09-23 15:45             ` [PATCH] v{extract,insert,broadcast,perm2}{i,f}128 Jakub Jelinek
  0 siblings, 1 reply; 9+ messages in thread
From: Uros Bizjak @ 2011-09-20 15:03 UTC (permalink / raw)
  To: Jakub Jelinek; +Cc: gcc-patches

On Tue, Sep 20, 2011 at 4:07 PM, Uros Bizjak <ubizjak@gmail.com> wrote:

>>> No, you define mode attribute like
>>>
>>> (define_mode_attr somesuffix [(V2DI "%i") (V2DF "f") ...])
>>>
>>> where integer modes map to %i and FP modes to "f".
>>>
>>> Then, just return "vextract<somesuffix>128\t{$0x1, %1, %0|%0, %1, 0x1}"
>>
>> I can surely do that (or e.g.
>> (define_mode_attr i128 [(V4DI "%~128") (V4DF "f128") ...])
>> and
>> "vextract<i128>\t{$0x1, %1, %0|%0, %1, 0x1}"
>> )
>> if you prefer it that way, but that is a functional alternative
>> to the patch I've just posted.  What I don't know how to express
>> right now is that:
>>   (set (attr "mode")
>>     (if_then_else
>>       (and (match_test "TARGET_AVX2")
>>            (eq (const_string "<MODE>mode") (const_string "V4DImode")))
>>     (const_string "OI")
>>     (const_string "V4DF"))
>> I've used on some insns, i.e. for either all alternatives (or
>> just the ones that use <i128> insns), use MODE_OI if TARGET_AVX2
>> and MODE_VECTOR_INT, or MODE_V4DF resp. MODE_V8SF (what has been used
>> until now).  No idea whether something uses get_mode_attr for anything
>> scheduling related (or length etc.), or whether just lying that it is
>> MODE_V4DF or MODE_V8SF even for vector integer modes with TARGET_AVX2
>> is ok.
>
> mode attribute us used only to calculate various prefixes (and that
> should be reviewed anyway). But for modes you are referring to, it
> doesn't matter that much.

Oh, and you can use <sseinsnmode> to set mode attribute statically.

Uros.

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

* [PATCH] v{extract,insert,broadcast,perm2}{i,f}128
  2011-09-20 15:03           ` v{extract,insert,broadcast,perm2}{i,f}128 Uros Bizjak
@ 2011-09-23 15:45             ` Jakub Jelinek
  2011-09-23 17:07               ` Uros Bizjak
  0 siblings, 1 reply; 9+ messages in thread
From: Jakub Jelinek @ 2011-09-23 15:45 UTC (permalink / raw)
  To: Uros Bizjak; +Cc: gcc-patches

On Tue, Sep 20, 2011 at 04:10:56PM +0200, Uros Bizjak wrote:
> >> I can surely do that (or e.g.
> >> (define_mode_attr i128 [(V4DI "%~128") (V4DF "f128") ...])
> >> and
> >> "vextract<i128>\t{$0x1, %1, %0|%0, %1, 0x1}"
> >> )
> >> if you prefer it that way, but that is a functional alternative
> >> to the patch I've just posted.  What I don't know how to express

> Oh, and you can use <sseinsnmode> to set mode attribute statically.

This patch implements that, bootstrapped/regtested on x86_64-linux
and i686-linux, tested on SandyBridge additionally, eyeballed some
-mavx2 assembly files.  Ok for trunk?

2011-09-23  Jakub Jelinek  <jakub@redhat.com>

	* config/i386/i386.c (ix86_print_operand): Handle %~.
	(ix86_print_operand_punct_valid_p): Return true also for '~'.
	* config/i386/sse.md (i128): New mode_attr.
	(vec_extract_hi_<mode>, vec_extract_hi_<mode>,
	avx_vbroadcastf128_<mode>, *avx_vperm2f128<mode>_full,
	*avx_vperm2f128<mode>_nozero, vec_set_lo_<mode>, 
	vec_set_hi_<mode>, *vec_concat<mode>_avx): Use <i128> in the
	patterns, use "<sseinsnmode>" for "mode" attribute.
	(vec_extract_hi_v16hi, vec_extract_hi_v32qi, vec_set_lo_v16hi,
	vec_set_hi_v16hi, vec_set_lo_v32qi, vec_set_hi_v32qi): Use
	%~128 in the patterns, use "OI" for "mode" attribute.

--- gcc/config/i386/i386.c.jj	2011-09-23 10:04:35.000000000 +0200
+++ gcc/config/i386/i386.c	2011-09-23 13:12:11.000000000 +0200
@@ -13513,6 +13513,7 @@ get_some_local_dynamic_name (void)
    Y -- print condition for XOP pcom* instruction.
    + -- print a branch hint as 'cs' or 'ds' prefix
    ; -- print a semicolon (after prefixes due to bug in older gas).
+   ~ -- print "i" if TARGET_AVX2, "f" otherwise.
    @ -- print a segment register of thread base pointer load
  */
 
@@ -14006,6 +14007,10 @@ ix86_print_operand (FILE *file, rtx x, i
 	    fputs ("gs", file);
 	  return;
 
+	case '~':
+	  putc (TARGET_AVX2 ? 'i' : 'f', file);
+	  return;
+
 	default:
 	    output_operand_lossage ("invalid operand code '%c'", code);
 	}
@@ -14141,7 +14146,7 @@ static bool
 ix86_print_operand_punct_valid_p (unsigned char code)
 {
   return (code == '@' || code == '*' || code == '+'
-	  || code == '&' || code == ';');
+	  || code == '&' || code == ';' || code == '~');
 }
 \f
 /* Print a memory operand whose address is ADDR.  */
--- gcc/config/i386/sse.md.jj	2011-09-22 15:40:48.000000000 +0200
+++ gcc/config/i386/sse.md	2011-09-23 12:59:12.000000000 +0200
@@ -297,6 +297,11 @@ (define_mode_attr castmode [(V8SI "si") 
 ;; Instruction suffix for sign and zero extensions.
 (define_code_attr extsuffix [(sign_extend "sx") (zero_extend "zx")])
 
+;; i128 for integer vectors and TARGET_AVX2, f128 otherwise.
+(define_mode_attr i128
+  [(V8SF "f128") (V4DF "f128") (V32QI "%~128") (V16HI "%~128")
+   (V8SI "%~128") (V4DI "%~128")])
+
 ;; Mix-n-match
 (define_mode_iterator AVX256MODE2P [V8SI V8SF V4DF])
 
@@ -3872,23 +3877,13 @@ (define_insn "vec_extract_hi_<mode>"
 	  (match_operand:VI8F_256 1 "register_operand" "x,x")
 	  (parallel [(const_int 2) (const_int 3)])))]
   "TARGET_AVX"
-{
-  if (get_attr_mode (insn) == MODE_OI)
-    return "vextracti128\t{$0x1, %1, %0|%0, %1, 0x1}";
-  else
-    return "vextractf128\t{$0x1, %1, %0|%0, %1, 0x1}";
-}
+  "vextract<i128>\t{$0x1, %1, %0|%0, %1, 0x1}"
   [(set_attr "type" "sselog")
    (set_attr "prefix_extra" "1")
    (set_attr "length_immediate" "1")
    (set_attr "memory" "none,store")
    (set_attr "prefix" "vex")
-   (set (attr "mode")
-     (if_then_else
-       (and (match_test "TARGET_AVX2")
-	    (eq (const_string "<MODE>mode") (const_string "V4DImode")))
-     (const_string "OI")
-     (const_string "V4DF")))])
+   (set_attr "mode" "<sseinsnmode>")])
 
 (define_insn_and_split "vec_extract_lo_<mode>"
   [(set (match_operand:<ssehalfvecmode> 0 "nonimmediate_operand" "=x,m")
@@ -3917,23 +3912,13 @@ (define_insn "vec_extract_hi_<mode>"
 	  (parallel [(const_int 4) (const_int 5)
 		     (const_int 6) (const_int 7)])))]
   "TARGET_AVX"
-{
-  if (get_attr_mode (insn) == MODE_OI)
-    return "vextracti128\t{$0x1, %1, %0|%0, %1, 0x1}";
-  else
-    return "vextractf128\t{$0x1, %1, %0|%0, %1, 0x1}";
-}
+  "vextract<i128>\t{$0x1, %1, %0|%0, %1, 0x1}"
   [(set_attr "type" "sselog")
    (set_attr "prefix_extra" "1")
    (set_attr "length_immediate" "1")
    (set_attr "memory" "none,store")
    (set_attr "prefix" "vex")
-   (set (attr "mode")
-     (if_then_else
-       (and (match_test "TARGET_AVX2")
-	    (eq (const_string "<MODE>mode") (const_string "V8SImode")))
-     (const_string "OI")
-     (const_string "V8SF")))])
+   (set_attr "mode" "<sseinsnmode>")])
 
 (define_insn_and_split "vec_extract_lo_v16hi"
   [(set (match_operand:V8HI 0 "nonimmediate_operand" "=x,m")
@@ -3966,21 +3951,13 @@ (define_insn "vec_extract_hi_v16hi"
 		     (const_int 12) (const_int 13)
 		     (const_int 14) (const_int 15)])))]
   "TARGET_AVX"
-{
-  if (get_attr_mode (insn) == MODE_OI)
-    return "vextracti128\t{$0x1, %1, %0|%0, %1, 0x1}";
-  else
-    return "vextractf128\t{$0x1, %1, %0|%0, %1, 0x1}";
-}
+  "vextract%~128\t{$0x1, %1, %0|%0, %1, 0x1}"
   [(set_attr "type" "sselog")
    (set_attr "prefix_extra" "1")
    (set_attr "length_immediate" "1")
    (set_attr "memory" "none,store")
    (set_attr "prefix" "vex")
-   (set (attr "mode")
-     (if_then_else (match_test "TARGET_AVX2")
-   (const_string "OI")
-   (const_string "V8SF")))])
+   (set_attr "mode" "OI")])
 
 (define_insn_and_split "vec_extract_lo_v32qi"
   [(set (match_operand:V16QI 0 "nonimmediate_operand" "=x,m")
@@ -4021,21 +3998,13 @@ (define_insn "vec_extract_hi_v32qi"
 		     (const_int 28) (const_int 29)
 		     (const_int 30) (const_int 31)])))]
   "TARGET_AVX"
-{
-  if (get_attr_mode (insn) == MODE_OI)
-    return "vextracti128\t{$0x1, %1, %0|%0, %1, 0x1}";
-  else
-    return "vextractf128\t{$0x1, %1, %0|%0, %1, 0x1}";
-}
+  "vextract%~128\t{$0x1, %1, %0|%0, %1, 0x1}"
   [(set_attr "type" "sselog")
    (set_attr "prefix_extra" "1")
    (set_attr "length_immediate" "1")
    (set_attr "memory" "none,store")
    (set_attr "prefix" "vex")
-   (set (attr "mode")
-     (if_then_else (match_test "TARGET_AVX2")
-   (const_string "OI")
-   (const_string "V8SF")))])
+   (set_attr "mode" "OI")])
 
 (define_insn_and_split "*sse4_1_extractps"
   [(set (match_operand:SF 0 "nonimmediate_operand" "=rm,x,x")
@@ -11663,14 +11632,14 @@ (define_insn "avx_vbroadcastf128_<mode>"
 	  (match_dup 1)))]
   "TARGET_AVX"
   "@
-   vbroadcastf128\t{%1, %0|%0, %1}
-   vinsertf128\t{$1, %1, %0, %0|%0, %0, %1, 1}
-   vperm2f128\t{$0, %t1, %t1, %0|%0, %t1, %t1, 0}"
+   vbroadcast<i128>\t{%1, %0|%0, %1}
+   vinsert<i128>\t{$1, %1, %0, %0|%0, %0, %1, 1}
+   vperm2<i128>\t{$0, %t1, %t1, %0|%0, %t1, %t1, 0}"
   [(set_attr "type" "ssemov,sselog1,sselog1")
    (set_attr "prefix_extra" "1")
    (set_attr "length_immediate" "0,1,1")
    (set_attr "prefix" "vex")
-   (set_attr "mode" "V4SF,V8SF,V8SF")])
+   (set_attr "mode" "<sseinsnmode>")])
 
 ;; Recognize broadcast as a vec_select as produced by builtin_vec_perm.
 ;; If it so happens that the input is in memory, use vbroadcast.
@@ -11864,12 +11833,12 @@ (define_insn "*avx_vperm2f128<mode>_full
 	   (match_operand:SI 3 "const_0_to_255_operand" "n")]
 	  UNSPEC_VPERMIL2F128))]
   "TARGET_AVX"
-  "vperm2f128\t{%3, %2, %1, %0|%0, %1, %2, %3}"
+  "vperm2<i128>\t{%3, %2, %1, %0|%0, %1, %2, %3}"
   [(set_attr "type" "sselog")
    (set_attr "prefix_extra" "1")
    (set_attr "length_immediate" "1")
    (set_attr "prefix" "vex")
-   (set_attr "mode" "V8SF")])
+   (set_attr "mode" "<sseinsnmode>")])
 
 (define_insn "*avx_vperm2f128<mode>_nozero"
   [(set (match_operand:AVX256MODE2P 0 "register_operand" "=x")
@@ -11884,13 +11853,13 @@ (define_insn "*avx_vperm2f128<mode>_noze
 {
   int mask = avx_vperm2f128_parallel (operands[3], <MODE>mode) - 1;
   operands[3] = GEN_INT (mask);
-  return "vperm2f128\t{%3, %2, %1, %0|%0, %1, %2, %3}";
+  return "vperm2<i128>\t{%3, %2, %1, %0|%0, %1, %2, %3}";
 }
   [(set_attr "type" "sselog")
    (set_attr "prefix_extra" "1")
    (set_attr "length_immediate" "1")
    (set_attr "prefix" "vex")
-   (set_attr "mode" "V8SF")])
+   (set_attr "mode" "<sseinsnmode>")])
 
 (define_expand "avx_vinsertf128<mode>"
   [(match_operand:V_256 0 "register_operand" "")
@@ -11955,12 +11924,12 @@ (define_insn "vec_set_lo_<mode>"
 	    (match_operand:VI8F_256 1 "register_operand" "x")
 	    (parallel [(const_int 2) (const_int 3)]))))]
   "TARGET_AVX"
-  "vinsertf128\t{$0x0, %2, %1, %0|%0, %1, %2, 0x0}"
+  "vinsert<i128>\t{$0x0, %2, %1, %0|%0, %1, %2, 0x0}"
   [(set_attr "type" "sselog")
    (set_attr "prefix_extra" "1")
    (set_attr "length_immediate" "1")
    (set_attr "prefix" "vex")
-   (set_attr "mode" "V4DF")])
+   (set_attr "mode" "<sseinsnmode>")])
 
 (define_insn "vec_set_hi_<mode>"
   [(set (match_operand:VI8F_256 0 "register_operand" "=x")
@@ -11970,12 +11939,12 @@ (define_insn "vec_set_hi_<mode>"
 	    (parallel [(const_int 0) (const_int 1)]))
 	  (match_operand:<ssehalfvecmode> 2 "nonimmediate_operand" "xm")))]
   "TARGET_AVX"
-  "vinsertf128\t{$0x1, %2, %1, %0|%0, %1, %2, 0x1}"
+  "vinsert<i128>\t{$0x1, %2, %1, %0|%0, %1, %2, 0x1}"
   [(set_attr "type" "sselog")
    (set_attr "prefix_extra" "1")
    (set_attr "length_immediate" "1")
    (set_attr "prefix" "vex")
-   (set_attr "mode" "V4DF")])
+   (set_attr "mode" "<sseinsnmode>")])
 
 (define_insn "vec_set_lo_<mode>"
   [(set (match_operand:VI4F_256 0 "register_operand" "=x")
@@ -11986,12 +11955,12 @@ (define_insn "vec_set_lo_<mode>"
 	    (parallel [(const_int 4) (const_int 5)
 		       (const_int 6) (const_int 7)]))))]
   "TARGET_AVX"
-  "vinsertf128\t{$0x0, %2, %1, %0|%0, %1, %2, 0x0}"
+  "vinsert<i128>\t{$0x0, %2, %1, %0|%0, %1, %2, 0x0}"
   [(set_attr "type" "sselog")
    (set_attr "prefix_extra" "1")
    (set_attr "length_immediate" "1")
    (set_attr "prefix" "vex")
-   (set_attr "mode" "V8SF")])
+   (set_attr "mode" "<sseinsnmode>")])
 
 (define_insn "vec_set_hi_<mode>"
   [(set (match_operand:VI4F_256 0 "register_operand" "=x")
@@ -12002,12 +11971,12 @@ (define_insn "vec_set_hi_<mode>"
 		       (const_int 2) (const_int 3)]))
 	  (match_operand:<ssehalfvecmode> 2 "nonimmediate_operand" "xm")))]
   "TARGET_AVX"
-  "vinsertf128\t{$0x1, %2, %1, %0|%0, %1, %2, 0x1}"
+  "vinsert<i128>\t{$0x1, %2, %1, %0|%0, %1, %2, 0x1}"
   [(set_attr "type" "sselog")
    (set_attr "prefix_extra" "1")
    (set_attr "length_immediate" "1")
    (set_attr "prefix" "vex")
-   (set_attr "mode" "V8SF")])
+   (set_attr "mode" "<sseinsnmode>")])
 
 (define_insn "vec_set_lo_v16hi"
   [(set (match_operand:V16HI 0 "register_operand" "=x")
@@ -12020,12 +11989,12 @@ (define_insn "vec_set_lo_v16hi"
 		       (const_int 12) (const_int 13)
 		       (const_int 14) (const_int 15)]))))]
   "TARGET_AVX"
-  "vinsertf128\t{$0x0, %2, %1, %0|%0, %1, %2, 0x0}"
+  "vinsert%~128\t{$0x0, %2, %1, %0|%0, %1, %2, 0x0}"
   [(set_attr "type" "sselog")
    (set_attr "prefix_extra" "1")
    (set_attr "length_immediate" "1")
    (set_attr "prefix" "vex")
-   (set_attr "mode" "V8SF")])
+   (set_attr "mode" "OI")])
 
 (define_insn "vec_set_hi_v16hi"
   [(set (match_operand:V16HI 0 "register_operand" "=x")
@@ -12038,12 +12007,12 @@ (define_insn "vec_set_hi_v16hi"
 		       (const_int 6) (const_int 7)]))
 	  (match_operand:V8HI 2 "nonimmediate_operand" "xm")))]
   "TARGET_AVX"
-  "vinsertf128\t{$0x1, %2, %1, %0|%0, %1, %2, 0x1}"
+  "vinsert%~128\t{$0x1, %2, %1, %0|%0, %1, %2, 0x1}"
   [(set_attr "type" "sselog")
    (set_attr "prefix_extra" "1")
    (set_attr "length_immediate" "1")
    (set_attr "prefix" "vex")
-   (set_attr "mode" "V8SF")])
+   (set_attr "mode" "OI")])
 
 (define_insn "vec_set_lo_v32qi"
   [(set (match_operand:V32QI 0 "register_operand" "=x")
@@ -12060,12 +12029,12 @@ (define_insn "vec_set_lo_v32qi"
 		       (const_int 28) (const_int 29)
 		       (const_int 30) (const_int 31)]))))]
   "TARGET_AVX"
-  "vinsertf128\t{$0x0, %2, %1, %0|%0, %1, %2, 0x0}"
+  "vinsert%~128\t{$0x0, %2, %1, %0|%0, %1, %2, 0x0}"
   [(set_attr "type" "sselog")
    (set_attr "prefix_extra" "1")
    (set_attr "length_immediate" "1")
    (set_attr "prefix" "vex")
-   (set_attr "mode" "V8SF")])
+   (set_attr "mode" "OI")])
 
 (define_insn "vec_set_hi_v32qi"
   [(set (match_operand:V32QI 0 "register_operand" "=x")
@@ -12082,12 +12051,12 @@ (define_insn "vec_set_hi_v32qi"
 		       (const_int 14) (const_int 15)]))
 	  (match_operand:V16QI 2 "nonimmediate_operand" "xm")))]
   "TARGET_AVX"
-  "vinsertf128\t{$0x1, %2, %1, %0|%0, %1, %2, 0x1}"
+  "vinsert%~128\t{$0x1, %2, %1, %0|%0, %1, %2, 0x1}"
   [(set_attr "type" "sselog")
    (set_attr "prefix_extra" "1")
    (set_attr "length_immediate" "1")
    (set_attr "prefix" "vex")
-   (set_attr "mode" "V8SF")])
+   (set_attr "mode" "OI")])
 
 (define_expand "<avx_avx2>_maskload<ssemodesuffix><avxsizesuffix>"
   [(set (match_operand:V48_AVX2 0 "register_operand" "")
@@ -12468,7 +12437,7 @@ (define_insn "*vec_concat<mode>_avx"
   switch (which_alternative)
     {
     case 0:
-      return "vinsertf128\t{$0x1, %2, %t1, %0|%0, %t1, %2, 0x1}";
+      return "vinsert<i128>\t{$0x1, %2, %t1, %0|%0, %t1, %2, 0x1}";
     case 1:
       switch (get_attr_mode (insn))
 	{


	Jakub

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

* Re: [PATCH] v{extract,insert,broadcast,perm2}{i,f}128
  2011-09-23 15:45             ` [PATCH] v{extract,insert,broadcast,perm2}{i,f}128 Jakub Jelinek
@ 2011-09-23 17:07               ` Uros Bizjak
  0 siblings, 0 replies; 9+ messages in thread
From: Uros Bizjak @ 2011-09-23 17:07 UTC (permalink / raw)
  To: Jakub Jelinek; +Cc: gcc-patches

On Fri, Sep 23, 2011 at 4:47 PM, Jakub Jelinek <jakub@redhat.com> wrote:
>> >> I can surely do that (or e.g.
>> >> (define_mode_attr i128 [(V4DI "%~128") (V4DF "f128") ...])
>> >> and
>> >> "vextract<i128>\t{$0x1, %1, %0|%0, %1, 0x1}"
>> >> )
>> >> if you prefer it that way, but that is a functional alternative
>> >> to the patch I've just posted.  What I don't know how to express
>
>> Oh, and you can use <sseinsnmode> to set mode attribute statically.
>
> This patch implements that, bootstrapped/regtested on x86_64-linux
> and i686-linux, tested on SandyBridge additionally, eyeballed some
> -mavx2 assembly files.  Ok for trunk?
>
> 2011-09-23  Jakub Jelinek  <jakub@redhat.com>
>
>        * config/i386/i386.c (ix86_print_operand): Handle %~.
>        (ix86_print_operand_punct_valid_p): Return true also for '~'.
>        * config/i386/sse.md (i128): New mode_attr.
>        (vec_extract_hi_<mode>, vec_extract_hi_<mode>,
>        avx_vbroadcastf128_<mode>, *avx_vperm2f128<mode>_full,
>        *avx_vperm2f128<mode>_nozero, vec_set_lo_<mode>,
>        vec_set_hi_<mode>, *vec_concat<mode>_avx): Use <i128> in the
>        patterns, use "<sseinsnmode>" for "mode" attribute.
>        (vec_extract_hi_v16hi, vec_extract_hi_v32qi, vec_set_lo_v16hi,
>        vec_set_hi_v16hi, vec_set_lo_v32qi, vec_set_hi_v32qi): Use
>        %~128 in the patterns, use "OI" for "mode" attribute.

OK.

Thanks,
Uros.

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

end of thread, other threads:[~2011-09-23 15:46 UTC | newest]

Thread overview: 9+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2011-09-19 20:16 v{extract,insert,broadcast,perm2}{i,f}128 Jakub Jelinek
2011-09-20  8:03 ` v{extract,insert,broadcast,perm2}{i,f}128 Uros Bizjak
2011-09-20 13:54   ` v{extract,insert,broadcast,perm2}{i,f}128 Jakub Jelinek
2011-09-20 14:00     ` v{extract,insert,broadcast,perm2}{i,f}128 Uros Bizjak
2011-09-20 14:11       ` v{extract,insert,broadcast,perm2}{i,f}128 Jakub Jelinek
2011-09-20 14:20         ` v{extract,insert,broadcast,perm2}{i,f}128 Uros Bizjak
2011-09-20 15:03           ` v{extract,insert,broadcast,perm2}{i,f}128 Uros Bizjak
2011-09-23 15:45             ` [PATCH] v{extract,insert,broadcast,perm2}{i,f}128 Jakub Jelinek
2011-09-23 17:07               ` 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).