[PATCH] Scalar evolution and hidden casts

[Richard Guenther]

On Tue, May 19, 2009 at 11:18 AM, Eric Botcazou <ebotcazou@adacore.com> wrote:
> Hi,
>
> I've attached an Ada testcase that should totally collapse at -O -gnatp, i.e.
> generates only "A = A + 1000000000", but doesn't because scev is very picky
> about casts, even hidden casts (tree-chrec.c contains many type assertions).
> The .lim dump file is entirely clean (attached, no casts) but .sccp shows that
> scev rematerializes some of them, yielding to scev_not_known:
>
> Analyzing # of iterations of loop 2
>  exit condition [2, + , 1] <= 1000000
>  bounds on difference of bases: 999998 ... 999998
>  result:
>    # of iterations 999999, bounded by 999999
>  (set_nb_iterations_in_loop = 999999))
> (chrec_apply
>  (varying_loop = 2
> )
>  (chrec = {(integer) p__a_lsm.11_11, +, 1}_2)
>  (x = 999999)
>  (res = (integer) p__a_lsm.11_11 + 999999))
>  (evolution_function = scev_not_known))
>
>
> Teaching tree-chrec.c to disregard useless casts appears to be tricky, you
> very quickly run into one of the type assertions.  Another strategy, which
> works at least in this case, is to change the type of the chrec and use that
> of the SSA name.  Patch attached, tested on i586-suse-linux, OK for mainline?

@@ -1181,6 +1181,7 @@ follow_ssa_edge_expr (struct loop *loop,
       /* This case is under the form "rhs0 +- rhs1".  */
       rhs0 = TREE_OPERAND (expr, 0);
       rhs1 = TREE_OPERAND (expr, 1);
+      type = TREE_TYPE (rhs0);
       STRIP_TYPE_NOPS (rhs0);
       STRIP_TYPE_NOPS (rhs1);
       return follow_ssa_edge_binary (loop, at_stmt, type, rhs0, code, rhs1,

that piece is ok.  Can you change STRIP_TYPE_NOPS to
STRIP_USELESS_TYPE_CONVERSIONS?

@@ -1215,16 +1216,17 @@ static t_bool
 follow_ssa_edge_in_rhs (struct loop *loop, gimple stmt,
                        gimple halting_phi, tree *evolution_of_loop, int limit)
 {
-  tree type = TREE_TYPE (gimple_assign_lhs (stmt));
   enum tree_code code = gimple_assign_rhs_code (stmt);

   switch (get_gimple_rhs_class (code))
     {
     case GIMPLE_BINARY_RHS:
-      return follow_ssa_edge_binary (loop, stmt, type,
-                                    gimple_assign_rhs1 (stmt), code,
-                                    gimple_assign_rhs2 (stmt),
-                                    halting_phi, evolution_of_loop, limit);
+      {
+       tree rhs1 = gimple_assign_rhs1 (stmt);
+       return follow_ssa_edge_binary (loop, stmt, TREE_TYPE (rhs1), rhs1,
+                                      code, gimple_assign_rhs2 (stmt),
+                                      halting_phi, evolution_of_loop, limit);
+      }

this is the symmetric change though the function should have
used

  tree type = gimple_expr_type (stmt);

and that function should probably change from

        switch (gimple_assign_rhs_code (stmt))
          {
          case POINTER_PLUS_EXPR:
            type = TREE_TYPE (gimple_assign_rhs1 (stmt));
            break;

          default:
            /* As fallback use the type of the LHS.  */
            type = TREE_TYPE (gimple_get_lhs (stmt));
            break;
          }

to be more specific.

Beware!  COMPLEX_EXPR is tcc_binary and your changes will
cause the wrong type to be used!  COMPLEX_EXPR should
probably be moved to tcc_expression.  Likewise for
WIDEN_SUM_EXPR and WIDEN_MULT_EXPR (though they do
not appear in the IL yet).  There doesn't seem to be useful
rules for tcc_binary - that the result type should match that of
the first operand would be a very useful one (and make your
patch valid).

I suppose you specifically hit the POINTER_PLUS_EXPR case?
Maybe just special-case that like in gimple_expr_type and use
that in follow_ssa_edge_in_rhs.

Thanks,
Richard.



>
> 2009-05-19  Eric Botcazou  <ebotcazou@adacore.com>
>
>        * tree-scalar-evolution.c (follow_ssa_edge_expr) <PLUS_EXPR>: Propagate
>        the type of the first operand.
>        (follow_ssa_edge_in_rhs) <GIMPLE_BINARY_RHS>: Likewise.
>
>
> --
> Eric Botcazou
>