diff mbox series

[rs6000] Fix PR83332 (missing vcond patterns)

Message ID 184e4c3d-02f9-6623-bf0e-451852e7d47b@linux.vnet.ibm.com
State New
Headers show
Series [rs6000] Fix PR83332 (missing vcond patterns) | expand

Commit Message

Bill Schmidt Dec. 11, 2017, 9:55 p.m. UTC 
Hi,

A new test case introduced for PR81303 failed on powerpc64 (BE, LE).  This
turns out to be due to a missing standard pattern (vcondv2div2df).  This
and a couple of other patterns are easy to support with existing logic
by just adding new patterns with appropriate modes.  That's all this patch
does.  That's sufficient to cause the failing test to pass.

Bootstrapped and tested on powerpc64le-linux-gnu with no regressions.  Is
this okay for trunk?

Thanks,
Bill


2017-12-11  Bill Schmidt  <wschmidt@linux.vnet.ibm.com>

	PR target/83332
	* config/rs6000/vector.md (vcondv2dfv2di): New define_expand.
	(vcondv2div2df): Likewise.
	(vconduv2dfv2di): Likewise.

Comments

Richard Biener Dec. 12, 2017, 8:02 a.m. UTC | #1 
On Mon, Dec 11, 2017 at 10:55 PM, Bill Schmidt
<wschmidt@linux.vnet.ibm.com> wrote:
> Hi,
>
> A new test case introduced for PR81303 failed on powerpc64 (BE, LE).  This
> turns out to be due to a missing standard pattern (vcondv2div2df).  This
> and a couple of other patterns are easy to support with existing logic
> by just adding new patterns with appropriate modes.  That's all this patch
> does.  That's sufficient to cause the failing test to pass.
>
> Bootstrapped and tested on powerpc64le-linux-gnu with no regressions.  Is
> this okay for trunk?
>
> Thanks,
> Bill
>
>
> 2017-12-11  Bill Schmidt  <wschmidt@linux.vnet.ibm.com>
>
>         PR target/83332
>         * config/rs6000/vector.md (vcondv2dfv2di): New define_expand.
>         (vcondv2div2df): Likewise.
>         (vconduv2dfv2di): Likewise.
>
> Index: gcc/config/rs6000/vector.md
> ===================================================================
> --- gcc/config/rs6000/vector.md (revision 255539)
> +++ gcc/config/rs6000/vector.md (working copy)
> @@ -455,6 +455,44 @@
>      FAIL;
>  }")
>
> +(define_expand "vcondv2dfv2di"

given you already have vcondv4siv4sf this asks for macroization, no?  I see you
have vcond<mode><mode> already, you can use

vcond<VEC_I:mode><VEC_F:mode>

and guard with GET_MODE_NUNITS () == GET_MODE_NUNINTS ()?

> +  [(set (match_operand:V2DF 0 "vfloat_operand" "")
> +       (if_then_else:V2DF
> +        (match_operator 3 "comparison_operator"
> +                        [(match_operand:V2DI 4 "vint_operand" "")
> +                         (match_operand:V2DI 5 "vint_operand" "")])
> +        (match_operand:V2DF 1 "vfloat_operand" "")
> +        (match_operand:V2DF 2 "vfloat_operand" "")))]
> +  "VECTOR_UNIT_ALTIVEC_OR_VSX_P (V2DFmode)
> +   && VECTOR_UNIT_ALTIVEC_OR_VSX_P (V2DImode)"
> +  "
> +{
> +  if (rs6000_emit_vector_cond_expr (operands[0], operands[1], operands[2],
> +                                   operands[3], operands[4], operands[5]))
> +    DONE;
> +  else
> +    FAIL;
> +}")
> +
> +(define_expand "vcondv2div2df"
> +  [(set (match_operand:V2DI 0 "vint_operand" "")
> +       (if_then_else:V2DI
> +        (match_operator 3 "comparison_operator"
> +                        [(match_operand:V2DF 4 "vfloat_operand" "")
> +                         (match_operand:V2DF 5 "vfloat_operand" "")])
> +        (match_operand:V2DI 1 "vint_operand" "")
> +        (match_operand:V2DI 2 "vint_operand" "")))]
> +  "VECTOR_UNIT_ALTIVEC_OR_VSX_P (V2DFmode)
> +   && VECTOR_UNIT_ALTIVEC_OR_VSX_P (V2DImode)"
> +  "
> +{
> +  if (rs6000_emit_vector_cond_expr (operands[0], operands[1], operands[2],
> +                                   operands[3], operands[4], operands[5]))
> +    DONE;
> +  else
> +    FAIL;
> +}")
> +
>  (define_expand "vcondu<mode><mode>"
>    [(set (match_operand:VEC_I 0 "vint_operand")
>         (if_then_else:VEC_I
> @@ -492,6 +530,25 @@
>      FAIL;
>  }")
>
> +(define_expand "vconduv2dfv2di"
> +  [(set (match_operand:V2DF 0 "vfloat_operand" "")
> +       (if_then_else:V2DF
> +        (match_operator 3 "comparison_operator"
> +                        [(match_operand:V2DI 4 "vint_operand" "")
> +                         (match_operand:V2DI 5 "vint_operand" "")])
> +        (match_operand:V2DF 1 "vfloat_operand" "")
> +        (match_operand:V2DF 2 "vfloat_operand" "")))]
> +  "VECTOR_UNIT_ALTIVEC_OR_VSX_P (V2DFmode)
> +   && VECTOR_UNIT_ALTIVEC_OR_VSX_P (V2DImode)"
> +  "
> +{
> +  if (rs6000_emit_vector_cond_expr (operands[0], operands[1], operands[2],
> +                                   operands[3], operands[4], operands[5]))
> +    DONE;
> +  else
> +    FAIL;
> +}")
> +
>  (define_expand "vector_eq<mode>"
>    [(set (match_operand:VEC_C 0 "vlogical_operand" "")
>         (eq:VEC_C (match_operand:VEC_C 1 "vlogical_operand" "")
>
Bill Schmidt Dec. 12, 2017, 1:46 p.m. UTC | #2 
> On Dec 12, 2017, at 2:02 AM, Richard Biener <richard.guenther@gmail.com> wrote:
> 
> On Mon, Dec 11, 2017 at 10:55 PM, Bill Schmidt
> <wschmidt@linux.vnet.ibm.com> wrote:
>> Hi,
>> 
>> A new test case introduced for PR81303 failed on powerpc64 (BE, LE).  This
>> turns out to be due to a missing standard pattern (vcondv2div2df).  This
>> and a couple of other patterns are easy to support with existing logic
>> by just adding new patterns with appropriate modes.  That's all this patch
>> does.  That's sufficient to cause the failing test to pass.
>> 
>> Bootstrapped and tested on powerpc64le-linux-gnu with no regressions.  Is
>> this okay for trunk?
>> 
>> Thanks,
>> Bill
>> 
>> 
>> 2017-12-11  Bill Schmidt  <wschmidt@linux.vnet.ibm.com>
>> 
>>        PR target/83332
>>        * config/rs6000/vector.md (vcondv2dfv2di): New define_expand.
>>        (vcondv2div2df): Likewise.
>>        (vconduv2dfv2di): Likewise.
>> 
>> Index: gcc/config/rs6000/vector.md
>> ===================================================================
>> --- gcc/config/rs6000/vector.md (revision 255539)
>> +++ gcc/config/rs6000/vector.md (working copy)
>> @@ -455,6 +455,44 @@
>>     FAIL;
>> }")
>> 
>> +(define_expand "vcondv2dfv2di"
> 
> given you already have vcondv4siv4sf this asks for macroization, no?  I see you
> have vcond<mode><mode> already, you can use
> 
> vcond<VEC_I:mode><VEC_F:mode>
> 
> and guard with GET_MODE_NUNITS () == GET_MODE_NUNINTS ()?

Yes, that can be done -- although the guard conditions start to get kind of ugly
now, since the v4siv4sf patterns require VECTOR_UNIT_ALTIVEC_P (V4SImode)
while the v2div2df ones require VECTOR_UNIT_ALTIVEC_OR_VSX_P (V2DImode).
What I have may be more straightforward to understand.  I can implement
whichever is considered preferable.

Thanks,
Bill

> 
>> +  [(set (match_operand:V2DF 0 "vfloat_operand" "")
>> +       (if_then_else:V2DF
>> +        (match_operator 3 "comparison_operator"
>> +                        [(match_operand:V2DI 4 "vint_operand" "")
>> +                         (match_operand:V2DI 5 "vint_operand" "")])
>> +        (match_operand:V2DF 1 "vfloat_operand" "")
>> +        (match_operand:V2DF 2 "vfloat_operand" "")))]
>> +  "VECTOR_UNIT_ALTIVEC_OR_VSX_P (V2DFmode)
>> +   && VECTOR_UNIT_ALTIVEC_OR_VSX_P (V2DImode)"
>> +  "
>> +{
>> +  if (rs6000_emit_vector_cond_expr (operands[0], operands[1], operands[2],
>> +                                   operands[3], operands[4], operands[5]))
>> +    DONE;
>> +  else
>> +    FAIL;
>> +}")
>> +
>> +(define_expand "vcondv2div2df"
>> +  [(set (match_operand:V2DI 0 "vint_operand" "")
>> +       (if_then_else:V2DI
>> +        (match_operator 3 "comparison_operator"
>> +                        [(match_operand:V2DF 4 "vfloat_operand" "")
>> +                         (match_operand:V2DF 5 "vfloat_operand" "")])
>> +        (match_operand:V2DI 1 "vint_operand" "")
>> +        (match_operand:V2DI 2 "vint_operand" "")))]
>> +  "VECTOR_UNIT_ALTIVEC_OR_VSX_P (V2DFmode)
>> +   && VECTOR_UNIT_ALTIVEC_OR_VSX_P (V2DImode)"
>> +  "
>> +{
>> +  if (rs6000_emit_vector_cond_expr (operands[0], operands[1], operands[2],
>> +                                   operands[3], operands[4], operands[5]))
>> +    DONE;
>> +  else
>> +    FAIL;
>> +}")
>> +
>> (define_expand "vcondu<mode><mode>"
>>   [(set (match_operand:VEC_I 0 "vint_operand")
>>        (if_then_else:VEC_I
>> @@ -492,6 +530,25 @@
>>     FAIL;
>> }")
>> 
>> +(define_expand "vconduv2dfv2di"
>> +  [(set (match_operand:V2DF 0 "vfloat_operand" "")
>> +       (if_then_else:V2DF
>> +        (match_operator 3 "comparison_operator"
>> +                        [(match_operand:V2DI 4 "vint_operand" "")
>> +                         (match_operand:V2DI 5 "vint_operand" "")])
>> +        (match_operand:V2DF 1 "vfloat_operand" "")
>> +        (match_operand:V2DF 2 "vfloat_operand" "")))]
>> +  "VECTOR_UNIT_ALTIVEC_OR_VSX_P (V2DFmode)
>> +   && VECTOR_UNIT_ALTIVEC_OR_VSX_P (V2DImode)"
>> +  "
>> +{
>> +  if (rs6000_emit_vector_cond_expr (operands[0], operands[1], operands[2],
>> +                                   operands[3], operands[4], operands[5]))
>> +    DONE;
>> +  else
>> +    FAIL;
>> +}")
>> +
>> (define_expand "vector_eq<mode>"
>>   [(set (match_operand:VEC_C 0 "vlogical_operand" "")
>>        (eq:VEC_C (match_operand:VEC_C 1 "vlogical_operand" "")
Richard Biener Dec. 12, 2017, 2:04 p.m. UTC | #3 
On Tue, Dec 12, 2017 at 2:46 PM, Bill Schmidt
<wschmidt@linux.vnet.ibm.com> wrote:
>
>> On Dec 12, 2017, at 2:02 AM, Richard Biener <richard.guenther@gmail.com> wrote:
>>
>> On Mon, Dec 11, 2017 at 10:55 PM, Bill Schmidt
>> <wschmidt@linux.vnet.ibm.com> wrote:
>>> Hi,
>>>
>>> A new test case introduced for PR81303 failed on powerpc64 (BE, LE).  This
>>> turns out to be due to a missing standard pattern (vcondv2div2df).  This
>>> and a couple of other patterns are easy to support with existing logic
>>> by just adding new patterns with appropriate modes.  That's all this patch
>>> does.  That's sufficient to cause the failing test to pass.
>>>
>>> Bootstrapped and tested on powerpc64le-linux-gnu with no regressions.  Is
>>> this okay for trunk?
>>>
>>> Thanks,
>>> Bill
>>>
>>>
>>> 2017-12-11  Bill Schmidt  <wschmidt@linux.vnet.ibm.com>
>>>
>>>        PR target/83332
>>>        * config/rs6000/vector.md (vcondv2dfv2di): New define_expand.
>>>        (vcondv2div2df): Likewise.
>>>        (vconduv2dfv2di): Likewise.
>>>
>>> Index: gcc/config/rs6000/vector.md
>>> ===================================================================
>>> --- gcc/config/rs6000/vector.md (revision 255539)
>>> +++ gcc/config/rs6000/vector.md (working copy)
>>> @@ -455,6 +455,44 @@
>>>     FAIL;
>>> }")
>>>
>>> +(define_expand "vcondv2dfv2di"
>>
>> given you already have vcondv4siv4sf this asks for macroization, no?  I see you
>> have vcond<mode><mode> already, you can use
>>
>> vcond<VEC_I:mode><VEC_F:mode>
>>
>> and guard with GET_MODE_NUNITS () == GET_MODE_NUNINTS ()?
>
> Yes, that can be done -- although the guard conditions start to get kind of ugly
> now, since the v4siv4sf patterns require VECTOR_UNIT_ALTIVEC_P (V4SImode)
> while the v2div2df ones require VECTOR_UNIT_ALTIVEC_OR_VSX_P (V2DImode).
> What I have may be more straightforward to understand.  I can implement
> whichever is considered preferable.

Up to the target maintainers of course.

Richard.

> Thanks,
> Bill
>
>>
>>> +  [(set (match_operand:V2DF 0 "vfloat_operand" "")
>>> +       (if_then_else:V2DF
>>> +        (match_operator 3 "comparison_operator"
>>> +                        [(match_operand:V2DI 4 "vint_operand" "")
>>> +                         (match_operand:V2DI 5 "vint_operand" "")])
>>> +        (match_operand:V2DF 1 "vfloat_operand" "")
>>> +        (match_operand:V2DF 2 "vfloat_operand" "")))]
>>> +  "VECTOR_UNIT_ALTIVEC_OR_VSX_P (V2DFmode)
>>> +   && VECTOR_UNIT_ALTIVEC_OR_VSX_P (V2DImode)"
>>> +  "
>>> +{
>>> +  if (rs6000_emit_vector_cond_expr (operands[0], operands[1], operands[2],
>>> +                                   operands[3], operands[4], operands[5]))
>>> +    DONE;
>>> +  else
>>> +    FAIL;
>>> +}")
>>> +
>>> +(define_expand "vcondv2div2df"
>>> +  [(set (match_operand:V2DI 0 "vint_operand" "")
>>> +       (if_then_else:V2DI
>>> +        (match_operator 3 "comparison_operator"
>>> +                        [(match_operand:V2DF 4 "vfloat_operand" "")
>>> +                         (match_operand:V2DF 5 "vfloat_operand" "")])
>>> +        (match_operand:V2DI 1 "vint_operand" "")
>>> +        (match_operand:V2DI 2 "vint_operand" "")))]
>>> +  "VECTOR_UNIT_ALTIVEC_OR_VSX_P (V2DFmode)
>>> +   && VECTOR_UNIT_ALTIVEC_OR_VSX_P (V2DImode)"
>>> +  "
>>> +{
>>> +  if (rs6000_emit_vector_cond_expr (operands[0], operands[1], operands[2],
>>> +                                   operands[3], operands[4], operands[5]))
>>> +    DONE;
>>> +  else
>>> +    FAIL;
>>> +}")
>>> +
>>> (define_expand "vcondu<mode><mode>"
>>>   [(set (match_operand:VEC_I 0 "vint_operand")
>>>        (if_then_else:VEC_I
>>> @@ -492,6 +530,25 @@
>>>     FAIL;
>>> }")
>>>
>>> +(define_expand "vconduv2dfv2di"
>>> +  [(set (match_operand:V2DF 0 "vfloat_operand" "")
>>> +       (if_then_else:V2DF
>>> +        (match_operator 3 "comparison_operator"
>>> +                        [(match_operand:V2DI 4 "vint_operand" "")
>>> +                         (match_operand:V2DI 5 "vint_operand" "")])
>>> +        (match_operand:V2DF 1 "vfloat_operand" "")
>>> +        (match_operand:V2DF 2 "vfloat_operand" "")))]
>>> +  "VECTOR_UNIT_ALTIVEC_OR_VSX_P (V2DFmode)
>>> +   && VECTOR_UNIT_ALTIVEC_OR_VSX_P (V2DImode)"
>>> +  "
>>> +{
>>> +  if (rs6000_emit_vector_cond_expr (operands[0], operands[1], operands[2],
>>> +                                   operands[3], operands[4], operands[5]))
>>> +    DONE;
>>> +  else
>>> +    FAIL;
>>> +}")
>>> +
>>> (define_expand "vector_eq<mode>"
>>>   [(set (match_operand:VEC_C 0 "vlogical_operand" "")
>>>        (eq:VEC_C (match_operand:VEC_C 1 "vlogical_operand" "")
>
Segher Boessenkool Dec. 12, 2017, 5:13 p.m. UTC | #4 
Hi!

On Mon, Dec 11, 2017 at 03:55:20PM -0600, Bill Schmidt wrote:
> A new test case introduced for PR81303 failed on powerpc64 (BE, LE).  This
> turns out to be due to a missing standard pattern (vcondv2div2df).  This
> and a couple of other patterns are easy to support with existing logic
> by just adding new patterns with appropriate modes.  That's all this patch
> does.  That's sufficient to cause the failing test to pass.

Nitpicking:

> +(define_expand "vcondv2dfv2di"
> +  [(set (match_operand:V2DF 0 "vfloat_operand" "")

No empty default "" arguments please.

> +	(if_then_else:V2DF
> +	 (match_operator 3 "comparison_operator"
> +			 [(match_operand:V2DI 4 "vint_operand" "")
> +			  (match_operand:V2DI 5 "vint_operand" "")])
> +	 (match_operand:V2DF 1 "vfloat_operand" "")
> +	 (match_operand:V2DF 2 "vfloat_operand" "")))]
> +  "VECTOR_UNIT_ALTIVEC_OR_VSX_P (V2DFmode)
> +   && VECTOR_UNIT_ALTIVEC_OR_VSX_P (V2DImode)"
> +  "
> +{
> +  if (rs6000_emit_vector_cond_expr (operands[0], operands[1], operands[2],
> +				    operands[3], operands[4], operands[5]))
> +    DONE;
> +  else
> +    FAIL;
> +}")

And no "" around the block.

Okay for trunk with that fixed up.  Thanks!


Segher
diff mbox series

Patch

Index: gcc/config/rs6000/vector.md
===================================================================
--- gcc/config/rs6000/vector.md	(revision 255539)
+++ gcc/config/rs6000/vector.md	(working copy)
@@ -455,6 +455,44 @@ 
     FAIL;
 }")
 
+(define_expand "vcondv2dfv2di"
+  [(set (match_operand:V2DF 0 "vfloat_operand" "")
+	(if_then_else:V2DF
+	 (match_operator 3 "comparison_operator"
+			 [(match_operand:V2DI 4 "vint_operand" "")
+			  (match_operand:V2DI 5 "vint_operand" "")])
+	 (match_operand:V2DF 1 "vfloat_operand" "")
+	 (match_operand:V2DF 2 "vfloat_operand" "")))]
+  "VECTOR_UNIT_ALTIVEC_OR_VSX_P (V2DFmode)
+   && VECTOR_UNIT_ALTIVEC_OR_VSX_P (V2DImode)"
+  "
+{
+  if (rs6000_emit_vector_cond_expr (operands[0], operands[1], operands[2],
+				    operands[3], operands[4], operands[5]))
+    DONE;
+  else
+    FAIL;
+}")
+
+(define_expand "vcondv2div2df"
+  [(set (match_operand:V2DI 0 "vint_operand" "")
+	(if_then_else:V2DI
+	 (match_operator 3 "comparison_operator"
+			 [(match_operand:V2DF 4 "vfloat_operand" "")
+			  (match_operand:V2DF 5 "vfloat_operand" "")])
+	 (match_operand:V2DI 1 "vint_operand" "")
+	 (match_operand:V2DI 2 "vint_operand" "")))]
+  "VECTOR_UNIT_ALTIVEC_OR_VSX_P (V2DFmode)
+   && VECTOR_UNIT_ALTIVEC_OR_VSX_P (V2DImode)"
+  "
+{
+  if (rs6000_emit_vector_cond_expr (operands[0], operands[1], operands[2],
+				    operands[3], operands[4], operands[5]))
+    DONE;
+  else
+    FAIL;
+}")
+
 (define_expand "vcondu<mode><mode>"
   [(set (match_operand:VEC_I 0 "vint_operand")
 	(if_then_else:VEC_I
@@ -492,6 +530,25 @@ 
     FAIL;
 }")
 
+(define_expand "vconduv2dfv2di"
+  [(set (match_operand:V2DF 0 "vfloat_operand" "")
+	(if_then_else:V2DF
+	 (match_operator 3 "comparison_operator"
+			 [(match_operand:V2DI 4 "vint_operand" "")
+			  (match_operand:V2DI 5 "vint_operand" "")])
+	 (match_operand:V2DF 1 "vfloat_operand" "")
+	 (match_operand:V2DF 2 "vfloat_operand" "")))]
+  "VECTOR_UNIT_ALTIVEC_OR_VSX_P (V2DFmode)
+   && VECTOR_UNIT_ALTIVEC_OR_VSX_P (V2DImode)"
+  "
+{
+  if (rs6000_emit_vector_cond_expr (operands[0], operands[1], operands[2],
+				    operands[3], operands[4], operands[5]))
+    DONE;
+  else
+    FAIL;
+}")
+
 (define_expand "vector_eq<mode>"
   [(set (match_operand:VEC_C 0 "vlogical_operand" "")
 	(eq:VEC_C (match_operand:VEC_C 1 "vlogical_operand" "")