Re: [PATCH] Fix substitute_and_fold ICE (PR tree-optimization/55331)
On Mon, Nov 26, 2012 at 4:27 PM, Jakub Jelinek wrote: > On Mon, Nov 26, 2012 at 04:24:41PM +0100, Richard Biener wrote: >> On Thu, Nov 15, 2012 at 9:09 PM, Jakub Jelinek wrote: >> > 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. > > Note that fold_stmt_1 also has code to handle a call folding into nothing, > so perhaps that could be removed too if you change it to fold into a nop > instead. Yeah, I'm testing a patch to remove that. Richard. > Jakub
Re: [PATCH] Fix substitute_and_fold ICE (PR tree-optimization/55331)
On Mon, Nov 26, 2012 at 04:24:41PM +0100, Richard Biener wrote: > On Thu, Nov 15, 2012 at 9:09 PM, Jakub Jelinek wrote: > > 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. Note that fold_stmt_1 also has code to handle a call folding into nothing, so perhaps that could be removed too if you change it to fold into a nop instead. Jakub
Re: [PATCH] Fix substitute_and_fold ICE (PR tree-optimization/55331)
On Thu, Nov 15, 2012 at 9:09 PM, Jakub Jelinek 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
>
> 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.0 +0100
> +++ gcc/tree-ssa-propagate.c2012-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");
> +
[PATCH] Fix substitute_and_fold ICE (PR tree-optimization/55331)
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?
2012-11-15 Jakub Jelinek
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.0 +0100
+++ gcc/tree-ssa-propagate.c2012-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
