On Thu, Nov 15, 2012 at 9:09 PM, Jakub Jelinek <ja...@redhat.com> wrote:
> Hi!
>
> On the following testcase substitute_and_fold ICEs because memmove
> of length 1 on an empty class is optimized away, and this function wasn't
> prepared to handle that.
>
> Fixed thusly, bootstrapped/regtested on x86_64-linux and i686-linux, ok for
> trunk/4.7?
I think the bug is that the stmt is removed. fold_stmt is not supposed to
"remove" the stmt in any case - it may replace it with a gimple_nop at most.
Thus, gimplify_and_update_call_from_tree is at fault here.
I am testing a patch that fixes it.
Thanks,
Richard.
> 2012-11-15 Jakub Jelinek <ja...@redhat.com>
>
> PR tree-optimization/55331
> * tree-ssa-propagate.c (substitute_and_fold): Handle fold_stmt
> turning a stmt into nothing.
>
> * g++.dg/opt/pr55331.C: New test.
>
> --- gcc/tree-ssa-propagate.c.jj 2012-11-02 09:01:55.000000000 +0100
> +++ gcc/tree-ssa-propagate.c 2012-11-15 12:27:37.218543417 +0100
> @@ -1094,7 +1094,7 @@ substitute_and_fold (ssa_prop_get_value_
> gimple stmt = gsi_stmt (i);
> gimple old_stmt;
> enum gimple_code code = gimple_code (stmt);
> - gimple_stmt_iterator oldi;
> + gimple_stmt_iterator oldi, nexti;
>
> oldi = i;
> if (do_dce)
> @@ -1147,6 +1147,8 @@ substitute_and_fold (ssa_prop_get_value_
> }
>
> old_stmt = stmt;
> + nexti = oldi;
> + gsi_next (&nexti);
>
> /* Some statements may be simplified using propagator
> specific information. Do this before propagating
> @@ -1171,36 +1173,63 @@ substitute_and_fold (ssa_prop_get_value_
> /* Now cleanup. */
> if (did_replace)
> {
> - stmt = gsi_stmt (oldi);
> -
> - /* If we cleaned up EH information from the statement,
> - remove EH edges. */
> - if (maybe_clean_or_replace_eh_stmt (old_stmt, stmt))
> - gimple_purge_dead_eh_edges (bb);
> -
> - if (is_gimple_assign (stmt)
> - && (get_gimple_rhs_class (gimple_assign_rhs_code (stmt))
> - == GIMPLE_SINGLE_RHS))
> - {
> - tree rhs = gimple_assign_rhs1 (stmt);
> -
> - if (TREE_CODE (rhs) == ADDR_EXPR)
> - recompute_tree_invariant_for_addr_expr (rhs);
> - }
> + /* old_stmt might have been also replaced by nothing,
> + in that case set stmt to NULL. */
> + if (gsi_end_p (oldi))
> + {
> + gcc_checking_assert (gsi_end_p (nexti));
> + stmt = NULL;
> + }
> + else
> + {
> + stmt = gsi_stmt (oldi);
> + if (!gsi_end_p (nexti) && gsi_stmt (nexti) == stmt)
> + stmt = NULL;
> + }
>
> - /* Determine what needs to be done to update the SSA form. */
> - update_stmt (stmt);
> - if (!is_gimple_debug (stmt))
> - something_changed = true;
> + if (stmt == NULL)
> + {
> + if (maybe_clean_eh_stmt (old_stmt))
> + gimple_purge_dead_eh_edges (bb);
> + something_changed = true;
> + }
> + else
> + {
> + /* If we cleaned up EH information from the statement,
> + remove EH edges. */
> + if (maybe_clean_or_replace_eh_stmt (old_stmt, stmt))
> + gimple_purge_dead_eh_edges (bb);
> +
> + if (is_gimple_assign (stmt)
> + && (get_gimple_rhs_class (gimple_assign_rhs_code (stmt))
> + == GIMPLE_SINGLE_RHS))
> + {
> + tree rhs = gimple_assign_rhs1 (stmt);
> +
> + if (TREE_CODE (rhs) == ADDR_EXPR)
> + recompute_tree_invariant_for_addr_expr (rhs);
> + }
> +
> + /* Determine what needs to be done to update the SSA
> + form. */
> + update_stmt (stmt);
> + if (!is_gimple_debug (stmt))
> + something_changed = true;
> + }
> }
>
> if (dump_file && (dump_flags & TDF_DETAILS))
> {
> if (did_replace)
> {
> - fprintf (dump_file, "Folded into: ");
> - print_gimple_stmt (dump_file, stmt, 0, TDF_SLIM);
> - fprintf (dump_file, "\n");
> + if (stmt == NULL)
> + fprintf (dump_file, "Folded into nothing\n");
> + else
> + {
> + fprintf (dump_file, "Folded into: ");
> + print_gimple_stmt (dump_file, stmt, 0, TDF_SLIM);
> + fprintf (dump_file, "\n");
> + }
> }
> else
> fprintf (dump_file, "Not folded\n");
> --- gcc/testsuite/g++.dg/opt/pr55331.C.jj 2012-11-15 12:53:26.893631190
> +0100
> +++ gcc/testsuite/g++.dg/opt/pr55331.C 2012-11-15 12:53:52.482504446 +0100
> @@ -0,0 +1,14 @@
> +// PR tree-optimization/55331
> +// { dg-do compile }
> +// { dg-options "-O2 -fno-tree-fre" }
> +
> +struct A {};
> +
> +void
> +foo (A *p, bool x)
> +{
> + A a;
> + char *e = (char *) (&a + 1);
> + if (x)
> + __builtin_memmove (p, &a, e - (char *) &a);
> +}
>
> Jakub
