[tuples] fix thinkos in forwprop

[tuples] fix thinkos in forwprop

[Aldy Hernandez]

This fixes a handful of regressions caused by enabling forwprop.

Committed to branch.

	* tree-ssa-forwprop.c: Remove obsolete comment.
	(get_prop_source_stmt): Wrap call to gimple_assign_lhs with a
	TREE_TYPE.
	(forward_propagate_comparison): Use build2 instead of
	fold_binary.

Index: tree-ssa-forwprop.c
===================================================================
--- tree-ssa-forwprop.c	(revision 137150)
+++ tree-ssa-forwprop.c	(working copy)
@@ -40,11 +40,6 @@ along with GCC; see the file COPYING3.  
    form of tree combination.   It is hoped all of this can disappear
    when we have a generalized tree combiner.
 
-   Note carefully that after propagation the resulting statement
-   must still be a proper gimple statement.  Right now we simply
-   only perform propagations we know will result in valid gimple
-   code.  One day we'll want to generalize this code.
-
    One class of common cases we handle is forward propagating a single use
    variable into a COND_EXPR.  
 
@@ -239,7 +234,7 @@ get_prop_source_stmt (tree name, bool si
 	rhs = gimple_assign_rhs1 (def_stmt);
 	if (IS_CONVERT_EXPR_CODE_P (gimple_assign_rhs_code (def_stmt))
 	    && TREE_CODE (rhs) == SSA_NAME
-	    && POINTER_TYPE_P (gimple_assign_lhs (def_stmt))
+	    && POINTER_TYPE_P (TREE_TYPE (gimple_assign_lhs (def_stmt)))
 	    && POINTER_TYPE_P (TREE_TYPE (rhs)))
 	  name = rhs;
 	else
@@ -970,13 +965,14 @@ forward_propagate_comparison (gimple stm
       {
         enum tree_code code = gimple_assign_rhs_code (use_stmt);
         tree cst = gimple_assign_rhs2 (use_stmt);
+	tree cond;
+
+	cond = build2 (gimple_assign_rhs_code (stmt),
+		       TREE_TYPE (cst),
+		       gimple_assign_rhs1 (stmt),
+		       gimple_assign_rhs2 (stmt));
 
-        tmp = combine_cond_expr_cond (code, TREE_TYPE (lhs),
-                                      fold_binary (code,
-                                                   TREE_TYPE (cst),
-                                                   gimple_assign_rhs1 (stmt),
-                                                   gimple_assign_rhs2 (stmt)),
-                                      cst, false);
+        tmp = combine_cond_expr_cond (code, TREE_TYPE (lhs), cond, cst, false);
         if (tmp == NULL_TREE)
           return false;
       }

Re: [tuples] fix thinkos in forwprop

[Richard Guenther]

On Mon, Jun 30, 2008 at 8:09 PM, Aldy Hernandez <aldyh@redhat.com> wrote:
> This fixes a handful of regressions caused by enabling forwprop.
>
> Committed to branch.
>
>        * tree-ssa-forwprop.c: Remove obsolete comment.
>        (get_prop_source_stmt): Wrap call to gimple_assign_lhs with a
>        TREE_TYPE.
>        (forward_propagate_comparison): Use build2 instead of
>        fold_binary.

Why that?  Trees should be always canonicalized.

Richard.

> Index: tree-ssa-forwprop.c
> ===================================================================
> --- tree-ssa-forwprop.c (revision 137150)
> +++ tree-ssa-forwprop.c (working copy)
> @@ -40,11 +40,6 @@ along with GCC; see the file COPYING3.
>    form of tree combination.   It is hoped all of this can disappear
>    when we have a generalized tree combiner.
>
> -   Note carefully that after propagation the resulting statement
> -   must still be a proper gimple statement.  Right now we simply
> -   only perform propagations we know will result in valid gimple
> -   code.  One day we'll want to generalize this code.
> -
>    One class of common cases we handle is forward propagating a single use
>    variable into a COND_EXPR.
>
> @@ -239,7 +234,7 @@ get_prop_source_stmt (tree name, bool si
>        rhs = gimple_assign_rhs1 (def_stmt);
>        if (IS_CONVERT_EXPR_CODE_P (gimple_assign_rhs_code (def_stmt))
>            && TREE_CODE (rhs) == SSA_NAME
> -           && POINTER_TYPE_P (gimple_assign_lhs (def_stmt))
> +           && POINTER_TYPE_P (TREE_TYPE (gimple_assign_lhs (def_stmt)))
>            && POINTER_TYPE_P (TREE_TYPE (rhs)))
>          name = rhs;
>        else
> @@ -970,13 +965,14 @@ forward_propagate_comparison (gimple stm
>       {
>         enum tree_code code = gimple_assign_rhs_code (use_stmt);
>         tree cst = gimple_assign_rhs2 (use_stmt);
> +       tree cond;
> +
> +       cond = build2 (gimple_assign_rhs_code (stmt),
> +                      TREE_TYPE (cst),
> +                      gimple_assign_rhs1 (stmt),
> +                      gimple_assign_rhs2 (stmt));
>
> -        tmp = combine_cond_expr_cond (code, TREE_TYPE (lhs),
> -                                      fold_binary (code,
> -                                                   TREE_TYPE (cst),
> -                                                   gimple_assign_rhs1 (stmt),
> -                                                   gimple_assign_rhs2 (stmt)),
> -                                      cst, false);
> +        tmp = combine_cond_expr_cond (code, TREE_TYPE (lhs), cond, cst, false);
>         if (tmp == NULL_TREE)
>           return false;
>       }
>

Re: [tuples] fix thinkos in forwprop

[Aldy Hernandez]

On Tue, Jul 01, 2008 at 12:01:13AM +0200, Richard Guenther wrote:
> On Mon, Jun 30, 2008 at 8:09 PM, Aldy Hernandez <aldyh@redhat.com> wrote:
> > This fixes a handful of regressions caused by enabling forwprop.
> >
> > Committed to branch.
> >
> >        * tree-ssa-forwprop.c: Remove obsolete comment.
> >        (get_prop_source_stmt): Wrap call to gimple_assign_lhs with a
> >        TREE_TYPE.
> >        (forward_propagate_comparison): Use build2 instead of
> >        fold_binary.
> 
> Why that?  Trees should be always canonicalized.

fold_binary was returning NULL because it couldn't do anything with a
testcase I found while boostrapping.  I guess I could check the return
value from fold_binary-- and use build2 if fold_binary returns NULL.

I'll look into it.

Re: [tuples] fix thinkos in forwprop

[Andrew Pinski]

On Mon, Jun 30, 2008 at 3:12 PM, Aldy Hernandez <aldyh@redhat.com> wrote:
> fold_binary was returning NULL because it couldn't do anything with a
> testcase I found while boostrapping.  I guess I could check the return
> value from fold_binary-- and use build2 if fold_binary returns NULL.

Or better just use fold_build2 which does that already :).

-- Pinski

Re: [tuples] fix thinkos in forwprop

[Aldy Hernandez]

On Mon, Jun 30, 2008 at 03:17:40PM -0700, Andrew Pinski wrote:
> On Mon, Jun 30, 2008 at 3:12 PM, Aldy Hernandez <aldyh@redhat.com> wrote:
> > fold_binary was returning NULL because it couldn't do anything with a
> > testcase I found while boostrapping.  I guess I could check the return
> > value from fold_binary-- and use build2 if fold_binary returns NULL.
> 
> Or better just use fold_build2 which does that already :).

Heh.  Did not know about this.  I'll use it.

Thanks.