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

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

[Marc Glisse]

[-- Attachment #1: Type: TEXT/PLAIN, Size: 1308 bytes --]

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.glisse@inria.fr>

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

-- 
Marc Glisse

[-- Attachment #2: Type: TEXT/PLAIN, Size: 1121 bytes --]

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");

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.glisse@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.glisse@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");
>

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.glisse@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