[Bug tree-optimization/53774] Reassociator generates non-canonical addition

[rguenther at suse dot de]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=53774

--- Comment #5 from rguenther at suse dot de <rguenther at suse dot de> 2012-06-27 08:28:27 UTC ---
On Tue, 26 Jun 2012, wschmidt at gcc dot gnu.org wrote:

> http://gcc.gnu.org/bugzilla/show_bug.cgi?id=53774
> 
> --- Comment #3 from William J. Schmidt <wschmidt at gcc dot gnu.org> 2012-06-26 18:42:41 UTC ---
> I wonder why fp_6(D) gets a rank of zero.  Is it an uninitialized variable or a
> parameter?  Parms are supposed to get small positive numbers for ranks.  Maybe
> the "right" fix is to force fp_6(D) to get a nonzero rank?

Yes, fp_6(D) is not initialized.

It also seems that ranks for non-constants are based on zero, too,
as for example loop-carried PHIs also might get a rank of zero.

Parameters get ranks starting off 3 (see init_reassoc).  I suppose
we could instead of walking over DECL_ARGUMENTS simply walk over all
SSA names and assign ranks to all default-defs instead.  I'm going
to test that.

Ah, it's

      if (TREE_CODE (SSA_NAME_VAR (e)) == PARM_DECL
          && SSA_NAME_IS_DEFAULT_DEF (e))
        return find_operand_rank (e);

      stmt = SSA_NAME_DEF_STMT (e);
      if (gimple_bb (stmt) == NULL)
        return 0;

in get_rank - that will return zero for all non-param default-defs,
even for the static chain we try to setup properly :/

Yeah, the following works - thanks for the hint ;)

Index: gcc/tree-ssa-reassoc.c
===================================================================
--- gcc/tree-ssa-reassoc.c      (revision 188987)
+++ gcc/tree-ssa-reassoc.c      (working copy)
@@ -383,14 +383,10 @@ get_rank (tree e)
       int i, n;
       tree op;

-      if (TREE_CODE (SSA_NAME_VAR (e)) == PARM_DECL
-         && SSA_NAME_IS_DEFAULT_DEF (e))
+      if (SSA_NAME_IS_DEFAULT_DEF (e))
        return find_operand_rank (e);

       stmt = SSA_NAME_DEF_STMT (e);
-      if (gimple_bb (stmt) == NULL)
-       return 0;
-
       if (gimple_code (stmt) == GIMPLE_PHI)
        return phi_rank (stmt);

@@ -3647,7 +3643,6 @@ init_reassoc (void)
 {
   int i;
   long rank = 2;
-  tree param;
   int *bbs = XNEWVEC (int, last_basic_block + 1);

   /* Find the loops, so that we can prevent moving calculations in
@@ -3666,24 +3661,13 @@ init_reassoc (void)
   bb_rank = XCNEWVEC (long, last_basic_block + 1);
   operand_rank = pointer_map_create ();

-  /* Give each argument a distinct rank.   */
-  for (param = DECL_ARGUMENTS (current_function_decl);
-       param;
-       param = DECL_CHAIN (param))
-    {
-      if (gimple_default_def (cfun, param) != NULL)
-       {
-         tree def = gimple_default_def (cfun, param);
-         insert_operand_rank (def, ++rank);
-       }
-    }
-
-  /* Give the chain decl a distinct rank. */
-  if (cfun->static_chain_decl != NULL)
-    {
-      tree def = gimple_default_def (cfun, cfun->static_chain_decl);
-      if (def != NULL)
-       insert_operand_rank (def, ++rank);
+  /* Give each default definition a distinct rank.  This includes
+     parameters and the static chain.  */
+  for (i = 1; i < (int) num_ssa_names; ++i)
+    {
+      tree name = ssa_name (i);
+      if (name && SSA_NAME_IS_DEFAULT_DEF (name))
+       insert_operand_rank (name, ++rank);
     }

   /* Set up rank for each BB  */