[PATCH, PR61776] verify_flow_info failed: control flow in the middle of basic block with -fprofile-generate

[Richard Biener]

On Mon, Jul 21, 2014 at 7:06 PM, Wei Mi <wmi@google.com> wrote:
> Hi,
>
> This patch is to fix:
> https://gcc.gnu.org/bugzilla/show_bug.cgi?id=61776
>
> It records func decls whose const/pure flags are reset during
> instrumentation. After the loop resetting const/pure flags, find out
> stmts calling those recorded funcs and perform cfg fixup on them.
>
> bootstrap and regression test pass on x86_64-linux-gnu. ok for trunk
> and gcc-4_9?

But fact is that it is _not_ necessary to split the block because there
are no outgoing abnormal edges from it.

The verifier failure is an artifact from using the same predicates during
CFG building and CFG verifying (usually ok, but for this particular
case it leads to this issue).

So I don't think your patch is the proper way to address this issue
(while it certainly works).

Instead whether a call can make abnormal gotos should be recorded
per-call and stored on the gimple-call.  Martin - this is exactly
one of the cases your patch would address?

Thanks,
Richard.

> Thanks,
> Wei.
>
> ChangeLog:
>
> 2014-07-21  Wei Mi  <wmi@google.com>
>
>         PR middle-end/61776
>         * tree-profile.c (tree_profiling): Fix cfg after the const/pure
>         flags of some funcs are reset after instrumentation.
>
> 2014-07-21  Wei Mi  <wmi@google.com>
>
>         PR middle-end/61776
>         * testsuite/gcc.dg/pr61776.c: New test.
>
> Index: tree-profile.c
> ===================================================================
> --- tree-profile.c      (revision 212442)
> +++ tree-profile.c      (working copy)
> @@ -56,6 +56,7 @@ along with GCC; see the file COPYING3.
>  #include "target.h"
>  #include "tree-cfgcleanup.h"
>  #include "tree-nested.h"
> +#include "pointer-set.h"
>
>  static GTY(()) tree gcov_type_node;
>  static GTY(()) tree tree_interval_profiler_fn;
> @@ -562,6 +563,9 @@ static unsigned int
>  tree_profiling (void)
>  {
>    struct cgraph_node *node;
> +  int i;
> +  struct pointer_set_t *modified_constpure_decls;
> +  vec<gimple> modified_constpure_stmts;
>
>    /* This is a small-ipa pass that gets called only once, from
>       cgraphunit.c:ipa_passes().  */
> @@ -603,6 +607,9 @@ tree_profiling (void)
>        pop_cfun ();
>      }
>
> +  modified_constpure_decls = pointer_set_create ();
> +  modified_constpure_stmts.create (0);
> +
>    /* Drop pure/const flags from instrumented functions.  */
>    FOR_EACH_DEFINED_FUNCTION (node)
>      {
> @@ -615,6 +622,11 @@ tree_profiling (void)
>        if (DECL_SOURCE_LOCATION (node->decl) == BUILTINS_LOCATION)
>         continue;
>
> +      /* If the const/pure flag of node is about to change, record
> +        node->decl in modified_constpure_decls.  */
> +      if (DECL_PURE_P (node->decl) || TREE_READONLY (node->decl))
> +       pointer_set_insert (modified_constpure_decls, node->decl);
> +
>        cgraph_set_const_flag (node, false, false);
>        cgraph_set_pure_flag (node, false, false);
>      }
> @@ -623,6 +635,7 @@ tree_profiling (void)
>    FOR_EACH_DEFINED_FUNCTION (node)
>      {
>        basic_block bb;
> +      gimple stmt;
>
>        if (!gimple_has_body_p (node->decl)
>           || !(!node->clone_of
> @@ -642,10 +655,29 @@ tree_profiling (void)
>             {
>               gimple stmt = gsi_stmt (gsi);
>               if (is_gimple_call (stmt))
> -               update_stmt (stmt);
> +               {
> +                 tree decl = gimple_call_fndecl(stmt);
> +                 if (decl && pointer_set_contains (modified_constpure_decls,
> +                                                   decl))
> +                   modified_constpure_stmts.safe_push (stmt);
> +                 update_stmt (stmt);
> +               }
>             }
>         }
>
> +      /* The const/pure flag of the decl of call stmt in
> modified_constpure_stmts
> +        is changed because of instrumentation. Split block if the
> call stmt is not
> +        the last stmt of bb and the call stmt ends bb.  */
> +      FOR_EACH_VEC_ELT (modified_constpure_stmts, i, stmt)
> +       {
> +         basic_block bb = gimple_bb (stmt);
> +
> +         if (stmt != gsi_stmt (gsi_last_bb (bb))
> +             && stmt_ends_bb_p (stmt))
> +           split_block (bb, stmt);
> +       }
> +      modified_constpure_stmts.release ();
> +
>        /* re-merge split blocks.  */
>        cleanup_tree_cfg ();
>        update_ssa (TODO_update_ssa);
> @@ -657,6 +689,7 @@ tree_profiling (void)
>
>    handle_missing_profiles ();
>
> +  pointer_set_destroy (modified_constpure_decls);
>    del_node_map ();
>    return 0;
>  }
> Index: testsuite/gcc.dg/pr61776.c
> ===================================================================
> --- testsuite/gcc.dg/pr61776.c  (revision 0)
> +++ testsuite/gcc.dg/pr61776.c  (revision 0)
> @@ -0,0 +1,27 @@
> +/* { dg-do compile } */
> +/* { dg-options "-O2 -fprofile-generate" } */
> +
> +#include <setjmp.h>
> +
> +int cond1, cond2;
> +
> +int goo() __attribute__((noinline));
> +
> +int goo() {
> + if (cond1)
> +   return 1;
> + else
> +   return 2;
> +}
> +
> +jmp_buf env;
> +int foo() {
> + int a;
> +
> + setjmp(env);
> + if (cond2)
> +   a = goo();
> + else
> +   a = 3;
> + return a;
> +}