diff mbox series

[3/3,V2,rs6000] vector conversion RTL pattern update for diff unit size

Message ID c8e8f628-2c21-34e3-101d-56677fc31467@linux.ibm.com
State New
Headers show
Series None | expand

Commit Message

Kewen.Lin Oct. 31, 2019, 9:35 a.m. UTC 
Hi Segher,

Thanks a lot for the comments.

on 2019/10/31 上午2:49, Segher Boessenkool wrote:
> Hi!
> 
> On Wed, Oct 23, 2019 at 05:42:45PM +0800, Kewen.Lin wrote:
>> Following the previous one 2/3, this patch is to update the
>> vector conversions between fixed point and floating point
>> with different element unit sizes, such as: SP <-> DI, DP <-> SI.
> 
>> 	(vsx_xvcvdp[su]xws): New define_expand, old one split to...
> 
> You mean <su> here, please fix (never use wildcards like [su] in changelogs:
> people grep for things in changelogs, which misses entries with wildcards).
> 

OK, will fix it.

>> +/* Half VMX/VSX vector (for select)  */
>> +VECTOR_MODE (FLOAT, SF, 2);   /*                 V2SF  */
>> +VECTOR_MODE (INT, SI, 2);     /*                 V2SI  */
> 
> Or "for internal use", in general.  What happens if a user tries to create
> something of such a mode?  I hope we don't ICE :-/
> 

I did some testings, it failed (ICE) if we constructed one insn with these 
modes artificially.  But I also checked the existing V8SF/V8SI/V4DF/... etc.,
they have same issues.  It looks more like a new issue to avoid that.

>> +;; Convert vector of 64-bit floating point numbers to vector of
>> +;; 32-bit signed/unsigned integers.
>> +(define_insn "vsx_xvcvdp<su>xws_be"
>>    [(set (match_operand:V4SI 0 "vsx_register_operand" "=v,?wa")
>> -	(unspec:V4SI [(match_operand:V2DF 1 "vsx_register_operand" "wa,wa")]
>> -		     UNSPEC_VSX_CVDPSXWS))]
>> -  "VECTOR_UNIT_VSX_P (V2DFmode)"
>> -  "xvcvdpsxws %x0,%x1"
>> +     (any_fix:V4SI
>> +       (vec_concat:V4DF (match_operand:V2DF 1 "vsx_register_operand" "wa,wa")
>> +	 (vec_select:V2DF (match_dup 1)
>> +	   (parallel [(const_int 1) (const_int 0)])))))]
>> +  "VECTOR_UNIT_VSX_P (V2DFmode) && BYTES_BIG_ENDIAN"
>> +  "xvcvdp<su>xws %x0,%x1"
>>    [(set_attr "type" "vecdouble")])
> 
> This doesn't work, I think: the insns actually leaves words 1 and 3
> undefined, but this pattern gives them a meaning.
> 
> I don't think we can do better than unspecs for such insns.  Or change
> the pattern to only describe the defined parts (this works for e.g. mulhw
> that describes its result as SImode: its DImode result has the high half
> undefined).
> 

Good point, thanks!  I agree, the current implementation for 64bit -> 32bit
RTL pattern has different semantics to what the instructions represent.
I was trying to find something represent undefined or random register value
and with twice vec_concat, but failed to.  I'll recover the change for the 
64bit -> 32bit part.

Updated patch attached, new regression testing just launched.


BR,
Kewen

-----------------

gcc/ChangeLog

2019-10-31  Kewen Lin  <linkw@gcc.gnu.org>

	* config/rs6000/rs6000-modes.def (V2SF, V2SI): New modes.
	* config/rs6000/vsx.md (UNSPEC_VSX_CVSPSXDS, UNSPEC_VSX_CVSPUXDS): Remove.
	(vsx_xvcvspdp): New define_expand, old define_insn split to...
	(vsx_xvcvspdp_be): ... this.  New.  And...
	(vsx_xvcvspdp_le): ... this.  New.
	(vsx_xvcv<su>xwdp): New define_expand, old define_insn split to...
	(vsx_xvcv<su>xwdp_be): ... this.  New.  And...
	(vsx_xvcv<su>xwdp_le): ... this.  New.
	(vsx_xvcvsp<su>xds): New define_expand, old define_insn split to...
	(vsx_xvcvsp<su>xds_be): ... this.  New.  And...
	(vsx_xvcvsp<su>xds_le): ... this.  New.

Comments

Segher Boessenkool Oct. 31, 2019, 6:49 p.m. UTC | #1 
Hi!

On Thu, Oct 31, 2019 at 05:35:22PM +0800, Kewen.Lin wrote:
> >> +/* Half VMX/VSX vector (for select)  */
> >> +VECTOR_MODE (FLOAT, SF, 2);   /*                 V2SF  */
> >> +VECTOR_MODE (INT, SI, 2);     /*                 V2SI  */
> > 
> > Or "for internal use", in general.  What happens if a user tries to create
> > something of such a mode?  I hope we don't ICE :-/
> 
> I did some testings, it failed (ICE) if we constructed one insn with these 
> modes artificially.  But I also checked the existing V8SF/V8SI/V4DF/... etc.,
> they have same issues.  It looks more like a new issue to avoid that.

What does "artificially" mean?  If you had to change the compiler for your
test, that doesn't count; otherwise, please file a PR.

> 	* config/rs6000/vsx.md (UNSPEC_VSX_CVSPSXDS, UNSPEC_VSX_CVSPUXDS): Remove.

(line too long)

Okay for trunk.  Thanks!


Segher
Kewen.Lin Nov. 1, 2019, 2:21 a.m. UTC | #2 
Hi Segher,

on 2019/11/1 上午2:49, Segher Boessenkool wrote:
> Hi!
> 
> On Thu, Oct 31, 2019 at 05:35:22PM +0800, Kewen.Lin wrote:
>>>> +/* Half VMX/VSX vector (for select)  */
>>>> +VECTOR_MODE (FLOAT, SF, 2);   /*                 V2SF  */
>>>> +VECTOR_MODE (INT, SI, 2);     /*                 V2SI  */
>>>
>>> Or "for internal use", in general.  What happens if a user tries to create
>>> something of such a mode?  I hope we don't ICE :-/
>>
>> I did some testings, it failed (ICE) if we constructed one insn with these 
>> modes artificially.  But I also checked the existing V8SF/V8SI/V4DF/... etc.,
>> they have same issues.  It looks more like a new issue to avoid that.
> 
> What does "artificially" mean?  If you had to change the compiler for your
> test, that doesn't count; otherwise, please file a PR.
> 

Yes, I hacked the compiler to emit it directly.  OK, it's fine then.  :)

>> 	* config/rs6000/vsx.md (UNSPEC_VSX_CVSPSXDS, UNSPEC_VSX_CVSPUXDS): Remove.
> 
> (line too long)

Will fix it.

> 
> Okay for trunk.  Thanks!
> 

Thanks for your time!


BR,
Kewen
diff mbox series

Patch

diff --git a/gcc/config/rs6000/rs6000-modes.def b/gcc/config/rs6000/rs6000-modes.def
index 677062c..2051358 100644
--- a/gcc/config/rs6000/rs6000-modes.def
+++ b/gcc/config/rs6000/rs6000-modes.def
@@ -74,6 +74,10 @@  VECTOR_MODES (FLOAT, 16);     /*       V8HF  V4SF V2DF */
 VECTOR_MODES (INT, 32);       /* V32QI V16HI V8SI V4DI */
 VECTOR_MODES (FLOAT, 32);     /*       V16HF V8SF V4DF */
 
+/* Half VMX/VSX vector (for internal use)  */
+VECTOR_MODE (FLOAT, SF, 2);   /*                 V2SF  */
+VECTOR_MODE (INT, SI, 2);     /*                 V2SI  */
+
 /* Replacement for TImode that only is allowed in GPRs.  We also use PTImode
    for quad memory atomic operations to force getting an even/odd register
    combination.  */
diff --git a/gcc/config/rs6000/vsx.md b/gcc/config/rs6000/vsx.md
index 83e4071..99b51cb 100644
--- a/gcc/config/rs6000/vsx.md
+++ b/gcc/config/rs6000/vsx.md
@@ -275,8 +275,6 @@ 
    UNSPEC_VSX_CVUXWDP
    UNSPEC_VSX_CVSXDSP
    UNSPEC_VSX_CVUXDSP
-   UNSPEC_VSX_CVSPSXDS
-   UNSPEC_VSX_CVSPUXDS
    UNSPEC_VSX_FLOAT2
    UNSPEC_VSX_UNS_FLOAT2
    UNSPEC_VSX_FLOATE
@@ -2106,14 +2104,36 @@ 
   "xscvdpsp %x0,%x1"
   [(set_attr "type" "fp")])
 
-(define_insn "vsx_xvcvspdp"
+(define_insn "vsx_xvcvspdp_be"
   [(set (match_operand:V2DF 0 "vsx_register_operand" "=v,?wa")
-	(unspec:V2DF [(match_operand:V4SF 1 "vsx_register_operand" "wa,wa")]
-			      UNSPEC_VSX_CVSPDP))]
-  "VECTOR_UNIT_VSX_P (V4SFmode)"
+     (float_extend:V2DF
+       (vec_select:V2SF (match_operand:V4SF 1 "vsx_register_operand" "wa,wa")
+	 (parallel [(const_int 0) (const_int 2)]))))]
+  "VECTOR_UNIT_VSX_P (V4SFmode) && BYTES_BIG_ENDIAN"
+  "xvcvspdp %x0,%x1"
+  [(set_attr "type" "vecdouble")])
+
+(define_insn "vsx_xvcvspdp_le"
+  [(set (match_operand:V2DF 0 "vsx_register_operand" "=v,?wa")
+     (float_extend:V2DF
+       (vec_select:V2SF (match_operand:V4SF 1 "vsx_register_operand" "wa,wa")
+	 (parallel [(const_int 1) (const_int 3)]))))]
+  "VECTOR_UNIT_VSX_P (V4SFmode) && !BYTES_BIG_ENDIAN"
   "xvcvspdp %x0,%x1"
   [(set_attr "type" "vecdouble")])
 
+(define_expand "vsx_xvcvspdp"
+  [(match_operand:V2DF 0 "vsx_register_operand")
+   (match_operand:V4SF 1 "vsx_register_operand")]
+  "VECTOR_UNIT_VSX_P (V4SFmode)"
+{
+  if (BYTES_BIG_ENDIAN)
+    emit_insn (gen_vsx_xvcvspdp_be (operands[0], operands[1]));
+  else
+    emit_insn (gen_vsx_xvcvspdp_le (operands[0], operands[1]));
+  DONE;
+})
+
 (define_insn "vsx_xvcvdpsp"
   [(set (match_operand:V4SF 0 "vsx_register_operand" "=wa,?wa")
 	(unspec:V4SF [(match_operand:V2DF 1 "vsx_register_operand" "v,wa")]
@@ -2333,16 +2353,39 @@ 
   "xvcvuxdsp %x0,%x1"
   [(set_attr "type" "vecdouble")])
 
-;; Convert from 32-bit to 64-bit types
-;; Provide both vector and scalar targets
-(define_insn "vsx_xvcvsxwdp"
+;; Convert vector of 32-bit signed/unsigned integers to vector of
+;; 64-bit floating point numbers.
+(define_insn "vsx_xvcv<su>xwdp_be"
   [(set (match_operand:V2DF 0 "vsx_register_operand" "=wa")
-	(unspec:V2DF [(match_operand:V4SI 1 "vsx_register_operand" "wa")]
-		     UNSPEC_VSX_CVSXWDP))]
-  "VECTOR_UNIT_VSX_P (V2DFmode)"
-  "xvcvsxwdp %x0,%x1"
+     (any_float:V2DF
+       (vec_select:V2SI (match_operand:V4SI 1 "vsx_register_operand" "wa")
+	 (parallel [(const_int 0) (const_int 2)]))))]
+  "VECTOR_UNIT_VSX_P (V2DFmode) && BYTES_BIG_ENDIAN"
+  "xvcv<su>xwdp %x0,%x1"
   [(set_attr "type" "vecdouble")])
 
+(define_insn "vsx_xvcv<su>xwdp_le"
+  [(set (match_operand:V2DF 0 "vsx_register_operand" "=wa")
+     (any_float:V2DF
+       (vec_select:V2SI (match_operand:V4SI 1 "vsx_register_operand" "wa")
+	 (parallel [(const_int 1) (const_int 3)]))))]
+  "VECTOR_UNIT_VSX_P (V2DFmode) && !BYTES_BIG_ENDIAN"
+  "xvcv<su>xwdp %x0,%x1"
+  [(set_attr "type" "vecdouble")])
+
+(define_expand "vsx_xvcv<su>xwdp"
+  [(match_operand:V2DF 0 "vsx_register_operand")
+   (match_operand:V4SI 1 "vsx_register_operand")
+   (any_float (pc))]
+  "VECTOR_UNIT_VSX_P (V2DFmode)"
+{
+  if (BYTES_BIG_ENDIAN)
+    emit_insn (gen_vsx_xvcv<su>xwdp_be (operands[0], operands[1]));
+  else
+    emit_insn (gen_vsx_xvcv<su>xwdp_le (operands[0], operands[1]));
+  DONE;
+})
+
 (define_insn "vsx_xvcvsxwdp_df"
   [(set (match_operand:DF 0 "vsx_register_operand" "=wa")
 	(unspec:DF [(match_operand:V4SI 1 "vsx_register_operand" "wa")]
@@ -2351,14 +2394,6 @@ 
   "xvcvsxwdp %x0,%x1"
   [(set_attr "type" "vecdouble")])
 
-(define_insn "vsx_xvcvuxwdp"
-  [(set (match_operand:V2DF 0 "vsx_register_operand" "=wa")
-	(unspec:V2DF [(match_operand:V4SI 1 "vsx_register_operand" "wa")]
-		     UNSPEC_VSX_CVUXWDP))]
-  "VECTOR_UNIT_VSX_P (V2DFmode)"
-  "xvcvuxwdp %x0,%x1"
-  [(set_attr "type" "vecdouble")])
-
 (define_insn "vsx_xvcvuxwdp_df"
   [(set (match_operand:DF 0 "vsx_register_operand" "=wa")
 	(unspec:DF [(match_operand:V4SI 1 "vsx_register_operand" "wa")]
@@ -2367,22 +2402,39 @@ 
   "xvcvuxwdp %x0,%x1"
   [(set_attr "type" "vecdouble")])
 
-(define_insn "vsx_xvcvspsxds"
+;; Convert vector of 32-bit floating point numbers to vector of
+;; 64-bit signed/unsigned integers.
+(define_insn "vsx_xvcvsp<su>xds_be"
   [(set (match_operand:V2DI 0 "vsx_register_operand" "=v,?wa")
-	(unspec:V2DI [(match_operand:V4SF 1 "vsx_register_operand" "wa,wa")]
-		     UNSPEC_VSX_CVSPSXDS))]
-  "VECTOR_UNIT_VSX_P (V2DFmode)"
-  "xvcvspsxds %x0,%x1"
+     (any_fix:V2DI
+       (vec_select:V2SF (match_operand:V4SF 1 "vsx_register_operand" "wa,wa")
+	 (parallel [(const_int 0) (const_int 2)]))))]
+  "VECTOR_UNIT_VSX_P (V2DFmode) && BYTES_BIG_ENDIAN"
+  "xvcvsp<su>xds %x0,%x1"
   [(set_attr "type" "vecdouble")])
 
-(define_insn "vsx_xvcvspuxds"
+(define_insn "vsx_xvcvsp<su>xds_le"
   [(set (match_operand:V2DI 0 "vsx_register_operand" "=v,?wa")
-	(unspec:V2DI [(match_operand:V4SF 1 "vsx_register_operand" "wa,wa")]
-		     UNSPEC_VSX_CVSPUXDS))]
-  "VECTOR_UNIT_VSX_P (V2DFmode)"
-  "xvcvspuxds %x0,%x1"
+     (any_fix:V2DI
+       (vec_select:V2SF (match_operand:V4SF 1 "vsx_register_operand" "wa,wa")
+	 (parallel [(const_int 1) (const_int 3)]))))]
+  "VECTOR_UNIT_VSX_P (V2DFmode) && !BYTES_BIG_ENDIAN"
+  "xvcvsp<su>xds %x0,%x1"
   [(set_attr "type" "vecdouble")])
 
+(define_expand "vsx_xvcvsp<su>xds"
+  [(match_operand:V2DI 0 "vsx_register_operand")
+   (match_operand:V4SF 1 "vsx_register_operand")
+   (any_fix (pc))]
+  "VECTOR_UNIT_VSX_P (V2DFmode)"
+{
+  if (BYTES_BIG_ENDIAN)
+    emit_insn (gen_vsx_xvcvsp<su>xds_be (operands[0], operands[1]));
+  else
+    emit_insn (gen_vsx_xvcvsp<su>xds_le (operands[0], operands[1]));
+  DONE;
+})
+
 ;; Generate float2 double
 ;; convert two double to float
 (define_expand "float2_v2df"