rs6000: fix creation of VEC_COND_EXPR

Message ID fc4877ec-5ebc-8860-64db-a092992f01c5@suse.cz
State New
Headers show
Series
  • rs6000: fix creation of VEC_COND_EXPR
Related show

Commit Message

Martin Liška June 18, 2020, 8:21 a.m.
On 6/18/20 10:00 AM, Segher Boessenkool wrote:
> Hi!

> 

> On Thu, Jun 18, 2020 at 09:44:58AM +0200, Martin Liška wrote:

>> This breaks quite some powerpc.exp tests. Right now VEC_COND_EXPR expects

>> first

>> argument to be a SSA_NAME (or constant) and so the patch fixes that.

> 

> What does this mean?  All context is missing here.

> 

> Also, is expecting that correct or not?  Was that a change?  Please

> explain.

> 

>> Using the patch, I survive powerpc.exp test-suite.

> 

> So this patch does *not* break quite some tests, it fixes them instead?

> 

> Please fix your commit message (and the Subject: even).

> 

>> diff --git a/gcc/config/rs6000/rs6000-call.c

>> b/gcc/config/rs6000/rs6000-call.c

>> index 817a14c9c0d..f613d372a13 100644

>> --- a/gcc/config/rs6000/rs6000-call.c

>> +++ b/gcc/config/rs6000/rs6000-call.c

>> @@ -10716,14 +10716,16 @@ rs6000_builtin_valid_without_lhs (enum

>> rs6000_builtins fn_code)

>>      CODE indicates which comparison is to be made. (EQ, GT, ...).

>>      TYPE indicates the type of the result.  */

>>   static tree

>> -fold_build_vec_cmp (tree_code code, tree type,

>> -		    tree arg0, tree arg1)

>> +fold_build_vec_cmp (tree_code code, tree type, tree arg0, tree arg1,

>> +		    gimple_stmt_iterator *gsi)

> 

> The comment needs changing, explaining what the new arg is.

> 

> 

> Segher

> 


All right, let's do it better.

Martin

Comments

Martin Sebor via Gcc-patches June 18, 2020, 8:53 a.m. | #1
On Thu, Jun 18, 2020 at 10:21 AM Martin Liška <mliska@suse.cz> wrote:
>

> On 6/18/20 10:00 AM, Segher Boessenkool wrote:

> > Hi!

> >

> > On Thu, Jun 18, 2020 at 09:44:58AM +0200, Martin Liška wrote:

> >> This breaks quite some powerpc.exp tests. Right now VEC_COND_EXPR expects

> >> first

> >> argument to be a SSA_NAME (or constant) and so the patch fixes that.

> >

> > What does this mean?  All context is missing here.

> >

> > Also, is expecting that correct or not?  Was that a change?  Please

> > explain.


And to explain - yes, VEC_COND_EXPR was changed to no longer allow
an embedded comparison operand.

> >> Using the patch, I survive powerpc.exp test-suite.

> >

> > So this patch does *not* break quite some tests, it fixes them instead?

> >

> > Please fix your commit message (and the Subject: even).

> >

> >> diff --git a/gcc/config/rs6000/rs6000-call.c

> >> b/gcc/config/rs6000/rs6000-call.c

> >> index 817a14c9c0d..f613d372a13 100644

> >> --- a/gcc/config/rs6000/rs6000-call.c

> >> +++ b/gcc/config/rs6000/rs6000-call.c

> >> @@ -10716,14 +10716,16 @@ rs6000_builtin_valid_without_lhs (enum

> >> rs6000_builtins fn_code)

> >>      CODE indicates which comparison is to be made. (EQ, GT, ...).

> >>      TYPE indicates the type of the result.  */

> >>   static tree

> >> -fold_build_vec_cmp (tree_code code, tree type,

> >> -                tree arg0, tree arg1)

> >> +fold_build_vec_cmp (tree_code code, tree type, tree arg0, tree arg1,

> >> +                gimple_stmt_iterator *gsi)

> >

> > The comment needs changing, explaining what the new arg is.

> >

> >

> > Segher

> >

>

> All right, let's do it better.

>

> Martin
Segher Boessenkool June 18, 2020, 1:58 p.m. | #2
On Thu, Jun 18, 2020 at 10:21:00AM +0200, Martin Liška wrote:
> All right, let's do it better.


Thanks!

(Btw, the MIME sub-type of "x-patch" makes this unviewable on many
browsers, in the gcc-patches@ archive (and unusable in many mail
clients of course).  Anything "x-*" should never be used on public
mailing lists, or anywhere else where you didn't get all people
receiving this to agree what this type means).

> >From 9ba94cec649ef84399531f43d5c7171328a3f704 Mon Sep 17 00:00:00 2001

> From: Martin Liska <mliska@suse.cz>

> Date: Thu, 18 Jun 2020 09:25:32 +0200

> Subject: [PATCH] rs6000: fix creation of VEC_COND_EXPR


Capital F on Fix.  Easy check is to run "git log --oneline" and see
if your commit seems out of place ;-)

> 	* config/rs6000/rs6000-call.c (fold_build_vec_cmp):

> 	Since 502d63b6d6141597bb18fd23c87736a1b384cf8f, first argument

> 	of a VEC_COND_EXPR cannot be tcc_comparison and so that

> 	a SSA_NAME needs to be created before we use it for the first

> 	argument of the VEC_COND_EXPR.


This isn't documented (in generic.texi).  Please fix that?  (In a
separate patch of course.)

> @@ -10714,16 +10714,19 @@ rs6000_builtin_valid_without_lhs (enum rs6000_builtins fn_code)

>     operation.  This sets up true/false vectors, and uses the

>     VEC_COND_EXPR operation.

>     CODE indicates which comparison is to be made. (EQ, GT, ...).

> -   TYPE indicates the type of the result.  */

> +   TYPE indicates the type of the result.  


(trailing spaces)

> +   GSI points to a GIMPLE statement that we are currently folding.  */


That isn't a useful thing to say.  Write that this function inserts new
code before there?

Okay for trunk with those changes.  Thanks!


Segher

Patch

From 9ba94cec649ef84399531f43d5c7171328a3f704 Mon Sep 17 00:00:00 2001
From: Martin Liska <mliska@suse.cz>
Date: Thu, 18 Jun 2020 09:25:32 +0200
Subject: [PATCH] rs6000: fix creation of VEC_COND_EXPR

gcc/ChangeLog:

	* config/rs6000/rs6000-call.c (fold_build_vec_cmp):
	Since 502d63b6d6141597bb18fd23c87736a1b384cf8f, first argument
	of a VEC_COND_EXPR cannot be tcc_comparison and so that
	a SSA_NAME needs to be created before we use it for the first
	argument of the VEC_COND_EXPR.
	(fold_compare_helper): Pass gsi to fold_build_vec_cmp.
---
 gcc/config/rs6000/rs6000-call.c | 15 +++++++++------
 1 file changed, 9 insertions(+), 6 deletions(-)

diff --git a/gcc/config/rs6000/rs6000-call.c b/gcc/config/rs6000/rs6000-call.c
index 817a14c9c0d..5bc6952214c 100644
--- a/gcc/config/rs6000/rs6000-call.c
+++ b/gcc/config/rs6000/rs6000-call.c
@@ -10714,16 +10714,19 @@  rs6000_builtin_valid_without_lhs (enum rs6000_builtins fn_code)
    operation.  This sets up true/false vectors, and uses the
    VEC_COND_EXPR operation.
    CODE indicates which comparison is to be made. (EQ, GT, ...).
-   TYPE indicates the type of the result.  */
+   TYPE indicates the type of the result.  
+   GSI points to a GIMPLE statement that we are currently folding.  */
 static tree
-fold_build_vec_cmp (tree_code code, tree type,
-		    tree arg0, tree arg1)
+fold_build_vec_cmp (tree_code code, tree type, tree arg0, tree arg1,
+		    gimple_stmt_iterator *gsi)
 {
   tree cmp_type = truth_type_for (type);
   tree zero_vec = build_zero_cst (type);
   tree minus_one_vec = build_minus_one_cst (type);
-  tree cmp = fold_build2 (code, cmp_type, arg0, arg1);
-  return fold_build3 (VEC_COND_EXPR, type, cmp, minus_one_vec, zero_vec);
+  tree temp = create_tmp_reg_or_ssa_name (cmp_type);
+  gimple *g = gimple_build_assign (temp, code, arg0, arg1);
+  gsi_insert_before (gsi, g, GSI_SAME_STMT);
+  return fold_build3 (VEC_COND_EXPR, type, temp, minus_one_vec, zero_vec);
 }
 
 /* Helper function to handle the in-between steps for the
@@ -10734,7 +10737,7 @@  fold_compare_helper (gimple_stmt_iterator *gsi, tree_code code, gimple *stmt)
   tree arg0 = gimple_call_arg (stmt, 0);
   tree arg1 = gimple_call_arg (stmt, 1);
   tree lhs = gimple_call_lhs (stmt);
-  tree cmp = fold_build_vec_cmp (code, TREE_TYPE (lhs), arg0, arg1);
+  tree cmp = fold_build_vec_cmp (code, TREE_TYPE (lhs), arg0, arg1, gsi);
   gimple *g = gimple_build_assign (lhs, cmp);
   gimple_set_location (g, gimple_location (stmt));
   gsi_replace (gsi, g, true);
-- 
2.27.0