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" } } */