* [PATCH] Fix a FSM threading ICE (PR tree-optimization/65735)
@ 2015-04-11 16:34 Jakub Jelinek
2015-04-11 17:19 ` Marc Glisse
2015-04-11 17:22 ` Richard Biener
0 siblings, 2 replies; 4+ messages in thread
From: Jakub Jelinek @ 2015-04-11 16:34 UTC (permalink / raw)
To: Richard Biener, Sebastian Pop, Jeff Law; +Cc: gcc-patches
Hi!
On the following testcase, starting with r221675 aka PR65177 fix
we get ICE, because FSM discovery finds a path that includes the same blocks
multiple times, like:
Registering FSM jump thread: (9, 4) incoming edge; (4, 5) (5, 12) (12, 14) (14, 5) (5, 12) nocopy; (5, 12)
All these bbs belong to the same loop, with bb14 being the header and bb12
the latch. And the copy_bbs/duplicate_thread_path don't seem to be really
prepared to duplicate the same basic block more than once.
fsm_find_control_statement_thread_paths has guard against recursion, but it
adds to the hash_set the PHI nodes. On the testcase, bb5 is added to the
path first through one of the PHIs:
# c_3 = PHI <c_39(4), b_17(14)>
# b_33 = PHI <b_32(4), b_17(14)>
and the second time through the other PHI.
The following patch fixes that by adding to the has_set the basic blocks
containing the PHIs instead of the PHIs.
Bootstrapped/regtested on x86_64-linux and i686-linux, ok for trunk?
2015-04-11 Jakub Jelinek <jakub@redhat.com>
PR tree-optimization/65735
* tree-ssa-threadedge.c (fsm_find_control_statement_thread_paths):
Remove visited_phis argument, add visited_bbs, avoid recursing into the
same bb rather than just into the same phi node.
(thread_through_normal_block): Adjust caller.
* gcc.c-torture/compile/pr65735.c: New test.
--- gcc/tree-ssa-threadedge.c.jj 2015-02-16 22:18:34.000000000 +0100
+++ gcc/tree-ssa-threadedge.c 2015-04-11 16:13:51.906916300 +0200
@@ -1015,7 +1015,7 @@ static int max_threaded_paths;
static void
fsm_find_control_statement_thread_paths (tree expr,
- hash_set<gimple> *visited_phis,
+ hash_set<basic_block> *visited_bbs,
vec<basic_block, va_gc> *&path,
bool seen_loop_phi)
{
@@ -1034,7 +1034,7 @@ fsm_find_control_statement_thread_paths
return;
/* Avoid infinite recursion. */
- if (visited_phis->add (def_stmt))
+ if (visited_bbs->add (var_bb))
return;
gphi *phi = as_a <gphi *> (def_stmt);
@@ -1109,7 +1109,7 @@ fsm_find_control_statement_thread_paths
{
vec_safe_push (path, bbi);
/* Recursively follow SSA_NAMEs looking for a constant definition. */
- fsm_find_control_statement_thread_paths (arg, visited_phis, path,
+ fsm_find_control_statement_thread_paths (arg, visited_bbs, path,
seen_loop_phi);
path->pop ();
@@ -1391,13 +1391,13 @@ thread_through_normal_block (edge e,
vec<basic_block, va_gc> *bb_path;
vec_alloc (bb_path, n_basic_blocks_for_fn (cfun));
vec_safe_push (bb_path, e->dest);
- hash_set<gimple> *visited_phis = new hash_set<gimple>;
+ hash_set<basic_block> *visited_bbs = new hash_set<basic_block>;
max_threaded_paths = PARAM_VALUE (PARAM_MAX_FSM_THREAD_PATHS);
- fsm_find_control_statement_thread_paths (cond, visited_phis, bb_path,
+ fsm_find_control_statement_thread_paths (cond, visited_bbs, bb_path,
false);
- delete visited_phis;
+ delete visited_bbs;
vec_free (bb_path);
}
return 0;
--- gcc/testsuite/gcc.c-torture/compile/pr65735.c.jj 2015-04-11 16:14:33.173263982 +0200
+++ gcc/testsuite/gcc.c-torture/compile/pr65735.c 2015-04-11 16:14:06.000000000 +0200
@@ -0,0 +1,21 @@
+/* PR tree-optimization/65735 */
+
+int foo (void);
+
+void
+bar (int a, int b, int c)
+{
+ while (!a)
+ {
+ c = foo ();
+ if (c == 7)
+ c = b;
+ switch (c)
+ {
+ case 1:
+ a = b++;
+ if (b)
+ b = 1;
+ }
+ }
+}
Jakub
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH] Fix a FSM threading ICE (PR tree-optimization/65735)
2015-04-11 16:34 [PATCH] Fix a FSM threading ICE (PR tree-optimization/65735) Jakub Jelinek
@ 2015-04-11 17:19 ` Marc Glisse
2015-04-11 17:33 ` Jakub Jelinek
2015-04-11 17:22 ` Richard Biener
1 sibling, 1 reply; 4+ messages in thread
From: Marc Glisse @ 2015-04-11 17:19 UTC (permalink / raw)
To: Jakub Jelinek; +Cc: Richard Biener, Sebastian Pop, Jeff Law, gcc-patches
On Sat, 11 Apr 2015, Jakub Jelinek wrote:
> @@ -1391,13 +1391,13 @@ thread_through_normal_block (edge e,
> vec<basic_block, va_gc> *bb_path;
> vec_alloc (bb_path, n_basic_blocks_for_fn (cfun));
> vec_safe_push (bb_path, e->dest);
> - hash_set<gimple> *visited_phis = new hash_set<gimple>;
> + hash_set<basic_block> *visited_bbs = new hash_set<basic_block>;
>
> max_threaded_paths = PARAM_VALUE (PARAM_MAX_FSM_THREAD_PATHS);
> - fsm_find_control_statement_thread_paths (cond, visited_phis, bb_path,
> + fsm_find_control_statement_thread_paths (cond, visited_bbs, bb_path,
> false);
>
> - delete visited_phis;
> + delete visited_bbs;
> vec_free (bb_path);
> }
> return 0;
I understand minimizing the patches right before the release. At any other
time, it would have been a great occasion to remove this new/delete
anti-pattern.
--
Marc Glisse
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH] Fix a FSM threading ICE (PR tree-optimization/65735)
2015-04-11 16:34 [PATCH] Fix a FSM threading ICE (PR tree-optimization/65735) Jakub Jelinek
2015-04-11 17:19 ` Marc Glisse
@ 2015-04-11 17:22 ` Richard Biener
1 sibling, 0 replies; 4+ messages in thread
From: Richard Biener @ 2015-04-11 17:22 UTC (permalink / raw)
To: Jakub Jelinek, Sebastian Pop, Jeff Law; +Cc: gcc-patches
On April 11, 2015 6:34:43 PM GMT+02:00, Jakub Jelinek <jakub@redhat.com> wrote:
>Hi!
>
>On the following testcase, starting with r221675 aka PR65177 fix
>we get ICE, because FSM discovery finds a path that includes the same
>blocks
>multiple times, like:
>Registering FSM jump thread: (9, 4) incoming edge; (4, 5) (5, 12)
>(12, 14) (14, 5) (5, 12) nocopy; (5, 12)
>All these bbs belong to the same loop, with bb14 being the header and
>bb12
>the latch. And the copy_bbs/duplicate_thread_path don't seem to be
>really
>prepared to duplicate the same basic block more than once.
>
>fsm_find_control_statement_thread_paths has guard against recursion,
>but it
>adds to the hash_set the PHI nodes. On the testcase, bb5 is added to
>the
>path first through one of the PHIs:
># c_3 = PHI <c_39(4), b_17(14)>
># b_33 = PHI <b_32(4), b_17(14)>
>and the second time through the other PHI.
>
>The following patch fixes that by adding to the has_set the basic
>blocks
>containing the PHIs instead of the PHIs.
>
>Bootstrapped/regtested on x86_64-linux and i686-linux, ok for trunk?
OK.
Thanks,
Richard.
>2015-04-11 Jakub Jelinek <jakub@redhat.com>
>
> PR tree-optimization/65735
> * tree-ssa-threadedge.c (fsm_find_control_statement_thread_paths):
> Remove visited_phis argument, add visited_bbs, avoid recursing into
>the
> same bb rather than just into the same phi node.
> (thread_through_normal_block): Adjust caller.
>
> * gcc.c-torture/compile/pr65735.c: New test.
>
>--- gcc/tree-ssa-threadedge.c.jj 2015-02-16 22:18:34.000000000 +0100
>+++ gcc/tree-ssa-threadedge.c 2015-04-11 16:13:51.906916300 +0200
>@@ -1015,7 +1015,7 @@ static int max_threaded_paths;
>
> static void
> fsm_find_control_statement_thread_paths (tree expr,
>- hash_set<gimple> *visited_phis,
>+ hash_set<basic_block> *visited_bbs,
> vec<basic_block, va_gc> *&path,
> bool seen_loop_phi)
> {
>@@ -1034,7 +1034,7 @@ fsm_find_control_statement_thread_paths
> return;
>
> /* Avoid infinite recursion. */
>- if (visited_phis->add (def_stmt))
>+ if (visited_bbs->add (var_bb))
> return;
>
> gphi *phi = as_a <gphi *> (def_stmt);
>@@ -1109,7 +1109,7 @@ fsm_find_control_statement_thread_paths
> {
> vec_safe_push (path, bbi);
> /* Recursively follow SSA_NAMEs looking for a constant definition.
>*/
>- fsm_find_control_statement_thread_paths (arg, visited_phis, path,
>+ fsm_find_control_statement_thread_paths (arg, visited_bbs, path,
> seen_loop_phi);
>
> path->pop ();
>@@ -1391,13 +1391,13 @@ thread_through_normal_block (edge e,
> vec<basic_block, va_gc> *bb_path;
> vec_alloc (bb_path, n_basic_blocks_for_fn (cfun));
> vec_safe_push (bb_path, e->dest);
>- hash_set<gimple> *visited_phis = new hash_set<gimple>;
>+ hash_set<basic_block> *visited_bbs = new hash_set<basic_block>;
>
> max_threaded_paths = PARAM_VALUE (PARAM_MAX_FSM_THREAD_PATHS);
>- fsm_find_control_statement_thread_paths (cond, visited_phis,
>bb_path,
>+ fsm_find_control_statement_thread_paths (cond, visited_bbs,
>bb_path,
> false);
>
>- delete visited_phis;
>+ delete visited_bbs;
> vec_free (bb_path);
> }
> return 0;
>--- gcc/testsuite/gcc.c-torture/compile/pr65735.c.jj 2015-04-11
>16:14:33.173263982 +0200
>+++ gcc/testsuite/gcc.c-torture/compile/pr65735.c 2015-04-11
>16:14:06.000000000 +0200
>@@ -0,0 +1,21 @@
>+/* PR tree-optimization/65735 */
>+
>+int foo (void);
>+
>+void
>+bar (int a, int b, int c)
>+{
>+ while (!a)
>+ {
>+ c = foo ();
>+ if (c == 7)
>+ c = b;
>+ switch (c)
>+ {
>+ case 1:
>+ a = b++;
>+ if (b)
>+ b = 1;
>+ }
>+ }
>+}
>
> Jakub
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH] Fix a FSM threading ICE (PR tree-optimization/65735)
2015-04-11 17:19 ` Marc Glisse
@ 2015-04-11 17:33 ` Jakub Jelinek
0 siblings, 0 replies; 4+ messages in thread
From: Jakub Jelinek @ 2015-04-11 17:33 UTC (permalink / raw)
To: gcc-patches; +Cc: Richard Biener, Sebastian Pop, Jeff Law
On Sat, Apr 11, 2015 at 07:19:00PM +0200, Marc Glisse wrote:
> On Sat, 11 Apr 2015, Jakub Jelinek wrote:
>
> >@@ -1391,13 +1391,13 @@ thread_through_normal_block (edge e,
> > vec<basic_block, va_gc> *bb_path;
> > vec_alloc (bb_path, n_basic_blocks_for_fn (cfun));
> > vec_safe_push (bb_path, e->dest);
> >- hash_set<gimple> *visited_phis = new hash_set<gimple>;
> >+ hash_set<basic_block> *visited_bbs = new hash_set<basic_block>;
> >
> > max_threaded_paths = PARAM_VALUE (PARAM_MAX_FSM_THREAD_PATHS);
> >- fsm_find_control_statement_thread_paths (cond, visited_phis, bb_path,
> >+ fsm_find_control_statement_thread_paths (cond, visited_bbs, bb_path,
> > false);
> >
> >- delete visited_phis;
> >+ delete visited_bbs;
> > vec_free (bb_path);
> > }
> > return 0;
>
> I understand minimizing the patches right before the release. At any other
> time, it would have been a great occasion to remove this new/delete
> anti-pattern.
Not at this point, I really wanted to do RC1 on Friday, now it looks more
likely for Monday, but really only blocker bugs at this point should go in.
Jakub
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2015-04-11 17:33 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2015-04-11 16:34 [PATCH] Fix a FSM threading ICE (PR tree-optimization/65735) Jakub Jelinek
2015-04-11 17:19 ` Marc Glisse
2015-04-11 17:33 ` Jakub Jelinek
2015-04-11 17:22 ` 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).