The following patch tries to work towards fixing PR62291 by moving
NEW_SETS/AVAIL_OUT adding strictly to insert_into_preds_of_block
and the value / expression we wanted to insert.  If doing that for
other unrelated expressions this may cause fake partial
redundancies to be detected and dead code will be inserted such
as for gcc.dg/tree-ssa/ssa-pre-28.c which is now fixed.

The idea is that we could now "simulate" insertion and its recursion
without actually performing the insertions (which requires AVAIL_OUT)
and instead postpone that to elimination time.

Well.  Idea...

Bootstrap and regtest running on x86_64-unknown-linux-gnu.

Richard.

2014-09-01  Richard Biener  <rguent...@suse.de>

        * tree-ssa-pre.c (find_or_generate_expression): Expand comment.
        (create_expression_by_pieces): Do not add to NEW_SETS or
        AVAIL_OUT here.
        (insert_into_preds_of_block): Instead do it here and only
        for the partial redundant value we inserted.

Index: gcc/tree-ssa-pre.c
===================================================================
--- gcc/tree-ssa-pre.c  (revision 214795)
+++ gcc/tree-ssa-pre.c  (working copy)
@@ -2797,9 +2797,11 @@ find_or_generate_expression (basic_block
       return NULL_TREE;
     }
 
-  /* It must be a complex expression, so generate it recursively.  Note
-     that this is only necessary to handle gcc.dg/tree-ssa/ssa-pre28.c
-     where the insert algorithm fails to insert a required expression.  */
+  /* It must be a complex expression, so generate it recursively.
+     Note that this is only necessary to handle cases like
+     gcc.dg/tree-ssa/ssa-pre-28.c where the insert algorithm fails to
+     insert a required expression because the dependent expression
+     isn't partially redundant.  */
   bitmap exprset = value_expressions[lookfor];
   bitmap_iterator bi;
   unsigned int i;
@@ -2846,7 +2848,6 @@ create_expression_by_pieces (basic_block
   unsigned int value_id;
   gimple_stmt_iterator gsi;
   tree exprtype = type ? type : get_expr_type (expr);
-  pre_expr nameexpr;
   gimple newstmt;
 
   switch (expr->kind)
@@ -2941,17 +2942,12 @@ create_expression_by_pieces (basic_block
        {
          gimple stmt = gsi_stmt (gsi);
          tree forcedname = gimple_get_lhs (stmt);
-         pre_expr nameexpr;
 
          if (TREE_CODE (forcedname) == SSA_NAME)
            {
              bitmap_set_bit (inserted_exprs, SSA_NAME_VERSION (forcedname));
              VN_INFO_GET (forcedname)->valnum = forcedname;
              VN_INFO (forcedname)->value_id = get_next_value_id ();
-             nameexpr = get_or_alloc_expr_for_name (forcedname);
-             add_to_value (VN_INFO (forcedname)->value_id, nameexpr);
-             bitmap_value_replace_in_set (NEW_SETS (block), nameexpr);
-             bitmap_value_replace_in_set (AVAIL_OUT (block), nameexpr);
            }
        }
       gimple_seq_add_seq (stmts, forced_stmts);
@@ -2979,12 +2975,6 @@ create_expression_by_pieces (basic_block
   VN_INFO (name)->valnum = sccvn_valnum_from_value_id (value_id);
   if (VN_INFO (name)->valnum == NULL_TREE)
     VN_INFO (name)->valnum = name;
-  gcc_assert (VN_INFO (name)->valnum != NULL_TREE);
-  nameexpr = get_or_alloc_expr_for_name (name);
-  add_to_value (value_id, nameexpr);
-  if (NEW_SETS (block))
-    bitmap_value_replace_in_set (NEW_SETS (block), nameexpr);
-  bitmap_value_replace_in_set (AVAIL_OUT (block), nameexpr);
 
   pre_stats.insertions++;
   if (dump_file && (dump_flags & TDF_DETAILS))
@@ -3061,7 +3051,11 @@ insert_into_preds_of_block (basic_block
              nophi = true;
              continue;
            }
-         avail[pred->dest_idx] = get_or_alloc_expr_for_name (builtexpr);
+         pre_expr nameexpr = get_or_alloc_expr_for_name (builtexpr);
+         avail[pred->dest_idx] = nameexpr;
+         add_to_value (get_expr_value_id (eprime), nameexpr);
+         bitmap_value_replace_in_set (NEW_SETS (bprime), nameexpr);
+         bitmap_value_replace_in_set (AVAIL_OUT (bprime), nameexpr);
          insertions = true;
        }
       else if (eprime->kind == CONSTANT)

Index: gcc/testsuite/gcc.dg/tree-ssa/ssa-pre-28.c
===================================================================
--- gcc/testsuite/gcc.dg/tree-ssa/ssa-pre-28.c  (revision 214795)
+++ gcc/testsuite/gcc.dg/tree-ssa/ssa-pre-28.c  (working copy)
@@ -15,7 +15,13 @@ int foo (int i, int b, int result)
 }
 
 /* We should insert i + 1 into the if (b) path as well as the simplified
-   i + 1 & -2 expression.  And do replacement with two PHI temps.  */
+   i + 1 & -2 expression.  And do replacement of the partially
+   redundant result & mask with one PHI temps.  In particular we
+   should avoid inserting i + 1 into the if (!b) path during
+   insert iteration 2.  */
 
-/* { dg-final { scan-tree-dump-times "with prephitmp" 2 "pre" } } */
+/* { dg-final { scan-tree-dump-times "Inserted pretmp" 2 "pre" } } */
+/* { dg-final { scan-tree-dump-times "Created phi prephitmp" 1 "pre" } } */
+/* { dg-final { scan-tree-dump-times "with prephitmp" 1 "pre" } } */
+/* { dg-final { scan-tree-dump-times "Starting insert iteration" 2 "pre" } } */
 /* { dg-final { cleanup-tree-dump "pre" } } */

Reply via email to