* [PATCH] Fix substitute_and_fold ICE (PR tree-optimization/55331)
@ 2012-11-15 20:09 Jakub Jelinek
2012-11-26 15:24 ` Richard Biener
0 siblings, 1 reply; 4+ messages in thread
From: Jakub Jelinek @ 2012-11-15 20:09 UTC (permalink / raw)
To: gcc-patches
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 <jakub@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
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH] Fix substitute_and_fold ICE (PR tree-optimization/55331)
2012-11-15 20:09 [PATCH] Fix substitute_and_fold ICE (PR tree-optimization/55331) Jakub Jelinek
@ 2012-11-26 15:24 ` Richard Biener
2012-11-26 15:27 ` Jakub Jelinek
0 siblings, 1 reply; 4+ messages in thread
From: Richard Biener @ 2012-11-26 15:24 UTC (permalink / raw)
To: Jakub Jelinek; +Cc: gcc-patches
On Thu, Nov 15, 2012 at 9:09 PM, Jakub Jelinek <jakub@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 <jakub@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
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH] Fix substitute_and_fold ICE (PR tree-optimization/55331)
2012-11-26 15:24 ` Richard Biener
@ 2012-11-26 15:27 ` Jakub Jelinek
2012-11-27 10:10 ` Richard Biener
0 siblings, 1 reply; 4+ messages in thread
From: Jakub Jelinek @ 2012-11-26 15:27 UTC (permalink / raw)
To: Richard Biener; +Cc: gcc-patches
On Mon, Nov 26, 2012 at 04:24:41PM +0100, Richard Biener wrote:
> On Thu, Nov 15, 2012 at 9:09 PM, Jakub Jelinek <jakub@redhat.com> 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
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH] Fix substitute_and_fold ICE (PR tree-optimization/55331)
2012-11-26 15:27 ` Jakub Jelinek
@ 2012-11-27 10:10 ` Richard Biener
0 siblings, 0 replies; 4+ messages in thread
From: Richard Biener @ 2012-11-27 10:10 UTC (permalink / raw)
To: Jakub Jelinek; +Cc: gcc-patches
On Mon, Nov 26, 2012 at 4:27 PM, Jakub Jelinek <jakub@redhat.com> 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 <jakub@redhat.com> 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
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2012-11-27 10:10 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2012-11-15 20:09 [PATCH] Fix substitute_and_fold ICE (PR tree-optimization/55331) Jakub Jelinek
2012-11-26 15:24 ` Richard Biener
2012-11-26 15:27 ` Jakub Jelinek
2012-11-27 10:10 ` Richard Biener
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).