On April 11, 2015 6:34:43 PM GMT+02:00, Jakub Jelinek <ja...@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 <ja...@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
