Re: genmatch: guess the type of a?b:c as b instead of a

[Jeff Law]

On 06/06/2015 05:34 AM, Marc Glisse wrote:

Hello,

as discussed around
https://gcc.gnu.org/ml/gcc-patches/2015-06/msg00041.html
we are currently guessing the type of a?b:c incorrectly. This does not
affect current simplifications, because the only 'cond' in output
patterns are at the outermost level, so their type is forced to 'type'
and never guessed. Indeed, the patch does not change the generated
*-match.c. It would allow removing an explicit cond:itype in a patch
posted by Jeff.

I tested it on a dummy .pd file containing:
(simplify
  (plus @0 (plus @1 @2))
  (negate (cond @0 @1 @2)))

and the generated files differ by:

-  res = fold_build3_loc (loc, COND_EXPR, TREE_TYPE (ops1[0]), ops1[0],
ops1[1], ops1[2]);
+  res = fold_build3_loc (loc, COND_EXPR, TREE_TYPE (ops1[1]), ops1[0],
ops1[1], ops1[2]);

(and something similar for gimple)

I wondered about using something like
VOID_TYPE_P (TREE_TYPE (ops1[1])) ? TREE_TYPE (ops1[2]) : TREE_TYPE
(ops1[1])
but I don't think that will be necessary.

Bootstrap is currently broken on many platforms with comparison
failures, but since it went that far and generated the same *-match.c
files, that seems sufficient testing.

2015-06-08  Marc Glisse  marc.gli...@inria.fr

 * genmatch.c (expr::gen_transform): For conditions, guess the type
 from the second operand.
Thanks for taking care of this.  I'd gone back and verified the type was 
needed, but didn't have to time reduce a testcase for it prior to going 
on PTO.


Jeff

Re: genmatch: guess the type of a?b:c as b instead of a

[Richard Biener]

On Sat, Jun 6, 2015 at 1:34 PM, Marc Glisse marc.gli...@inria.fr wrote:
 Hello,

 as discussed around
 https://gcc.gnu.org/ml/gcc-patches/2015-06/msg00041.html
 we are currently guessing the type of a?b:c incorrectly. This does not
 affect current simplifications, because the only 'cond' in output patterns
 are at the outermost level, so their type is forced to 'type' and never
 guessed. Indeed, the patch does not change the generated *-match.c. It would
 allow removing an explicit cond:itype in a patch posted by Jeff.

 I tested it on a dummy .pd file containing:
 (simplify
  (plus @0 (plus @1 @2))
  (negate (cond @0 @1 @2)))

 and the generated files differ by:

 -  res = fold_build3_loc (loc, COND_EXPR, TREE_TYPE (ops1[0]), ops1[0],
 ops1[1], ops1[2]);
 +  res = fold_build3_loc (loc, COND_EXPR, TREE_TYPE (ops1[1]), ops1[0],
 ops1[1], ops1[2]);

 (and something similar for gimple)

 I wondered about using something like
 VOID_TYPE_P (TREE_TYPE (ops1[1])) ? TREE_TYPE (ops1[2]) : TREE_TYPE
 (ops1[1])
 but I don't think that will be necessary.

Yeah, I think we can't currently match this anyway.

 Bootstrap is currently broken on many platforms with comparison failures,
 but since it went that far and generated the same *-match.c files, that
 seems sufficient testing.

Ok.  (this is indeed how I test genmatch.c patches - look at differences
in generated {generic,gimple}-match.c and play with toy patterns and
check their handling)

Thanks,
Richard.

 2015-06-08  Marc Glisse  marc.gli...@inria.fr

 * genmatch.c (expr::gen_transform): For conditions, guess the type
 from the second operand.

 --
 Marc Glisse
 Index: gcc/genmatch.c
 ===
 --- gcc/genmatch.c  (revision 224186)
 +++ gcc/genmatch.c  (working copy)
 @@ -1702,20 +1702,27 @@ expr::gen_transform (FILE *f, const char
type = optype;
  }
else if (is_a operator_id * (operation)
 !strcmp (as_a operator_id * (operation)-tcc,
 tcc_comparison))
  {
/* comparisons use boolean_type_node (or what gets in), but
   their operands need to figure out the types themselves.  */
sprintf (optype, boolean_type_node);
type = optype;
  }
 +  else if (*operation == COND_EXPR
 +  || *operation == VEC_COND_EXPR)
 +{
 +  /* Conditions are of the same type as their first alternative.  */
 +  sprintf (optype, TREE_TYPE (ops%d[1]), depth);
 +  type = optype;
 +}
else
  {
/* Other operations are of the same type as their first operand.  */
sprintf (optype, TREE_TYPE (ops%d[0]), depth);
type = optype;
  }
if (!type)
  fatal (two conversions in a row);

fprintf (f, {\n);

genmatch: guess the type of a?b:c as b instead of a

[Marc Glisse]

Hello,

as discussed around
https://gcc.gnu.org/ml/gcc-patches/2015-06/msg00041.html
we are currently guessing the type of a?b:c incorrectly. This does not 
affect current simplifications, because the only 'cond' in output patterns 
are at the outermost level, so their type is forced to 'type' and never 
guessed. Indeed, the patch does not change the generated *-match.c. It 
would allow removing an explicit cond:itype in a patch posted by Jeff.


I tested it on a dummy .pd file containing:
(simplify
 (plus @0 (plus @1 @2))
 (negate (cond @0 @1 @2)))

and the generated files differ by:

-  res = fold_build3_loc (loc, COND_EXPR, TREE_TYPE (ops1[0]), ops1[0], 
ops1[1], ops1[2]);
+  res = fold_build3_loc (loc, COND_EXPR, TREE_TYPE (ops1[1]), ops1[0], 
ops1[1], ops1[2]);

(and something similar for gimple)

I wondered about using something like
VOID_TYPE_P (TREE_TYPE (ops1[1])) ? TREE_TYPE (ops1[2]) : TREE_TYPE (ops1[1])
but I don't think that will be necessary.

Bootstrap is currently broken on many platforms with comparison failures, 
but since it went that far and generated the same *-match.c files, that 
seems sufficient testing.


2015-06-08  Marc Glisse  marc.gli...@inria.fr

* genmatch.c (expr::gen_transform): For conditions, guess the type
from the second operand.

--
Marc GlisseIndex: gcc/genmatch.c
===
--- gcc/genmatch.c  (revision 224186)
+++ gcc/genmatch.c  (working copy)
@@ -1702,20 +1702,27 @@ expr::gen_transform (FILE *f, const char
   type = optype;
 }
   else if (is_a operator_id * (operation)
!strcmp (as_a operator_id * (operation)-tcc, tcc_comparison))
 {
   /* comparisons use boolean_type_node (or what gets in), but
  their operands need to figure out the types themselves.  */
   sprintf (optype, boolean_type_node);
   type = optype;
 }
+  else if (*operation == COND_EXPR
+  || *operation == VEC_COND_EXPR)
+{
+  /* Conditions are of the same type as their first alternative.  */
+  sprintf (optype, TREE_TYPE (ops%d[1]), depth);
+  type = optype;
+}
   else
 {
   /* Other operations are of the same type as their first operand.  */
   sprintf (optype, TREE_TYPE (ops%d[0]), depth);
   type = optype;
 }
   if (!type)
 fatal (two conversions in a row);
 
   fprintf (f, {\n);