diff mbox

v{extract,insert,broadcast,perm2}{i,f}128

Message ID 20110920133856.GN2687@tyan-ft48-01.lab.bos.redhat.com
State New
Headers show

Commit Message

Jakub Jelinek Sept. 20, 2011, 1:38 p.m. UTC 
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.


	Jakub

Comments

Uros Bizjak Sept. 20, 2011, 1:47 p.m. UTC | #1 
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.
Jakub Jelinek Sept. 20, 2011, 2 p.m. UTC | #2 
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
Uros Bizjak Sept. 20, 2011, 2:07 p.m. UTC | #3 
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.
Uros Bizjak Sept. 20, 2011, 2:10 p.m. UTC | #4 
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.
diff mbox

Patch

--- 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))
 	{