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

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

[Wei Mi]

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?

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;
+}

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

[Wei Mi]

By the way, the resetting of const/pure flags loop is also executed
during profile-use， but if there is no instrumentation, the reset is
unnecessary. The flags are kept until pass_ipa_pure_const fixes them.
And because of non-instantaneous ssa update,  the fixes are reflected
on ssa only after ipa passes finish.

If it is agreed that this is a problem, I will address the
conservativeness in a separate patch.

Regards,
Wei.

Re: [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;
> +}

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

[Martin Jambor]

Hi,

On Wed, Jul 23, 2014 at 11:51:37AM +0200, Richard Biener wrote:
> 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?

No, my patch, which is attached to PR 60449 in bugzilla, introduces
leaf and noreturn gimple statement attributes and uses them to deal
with what really seems to be a very similar problem.  However, my
patch does not really look like 4.9 material.

Thanks,

Martin

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

[Richard Biener]

On Wed, Jul 23, 2014 at 2:11 PM, Martin Jambor <mjambor@suse.cz> wrote:
> Hi,
>
> On Wed, Jul 23, 2014 at 11:51:37AM +0200, Richard Biener wrote:
>> 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?
>
> No, my patch, which is attached to PR 60449 in bugzilla, introduces
> leaf and noreturn gimple statement attributes and uses them to deal
> with what really seems to be a very similar problem.  However, my
> patch does not really look like 4.9 material.

It doesn't really matter for 4.9 if you built that with release checking.

Richard.

> Thanks,
>
> Martin

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

[Wei Mi]

[-- Attachment #1: Type: text/plain, Size: 1649 bytes --]

> 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 for the comment and thanks to Martin's patch. I try the patch.
It works well to address both pr60449 and pr61776 after some
extension. One extension is to replace GF_CALL_LEAF attribute using
GF_CALL_NO_ABNORMAL_GOTO. That is because not only dropping "leaf"
attribute in lto symbol merge could introduce the control flow
verification problem in pr60449, dropping "const/pure" attributes
could introduce the same problem too. It is unnecessary to introduce
per-call attributes for all these three: ECF_LEAF/ECF_CONST/ECF_PURE,
so GF_CALL_NO_ABNORMAL_GOTO is introduced to indicate that a call stmt
has no abnormal goto.

GF_CALL_NO_ABNORMAL_GOTO will be set according to gimple_call_flags()
once gimple call stmt is created, then updated in execute_fixup_cfg
and cleanup_tree_cfg.

I posted the extended patch here. I didn't add the noreturn part in
because it has no direct impact on pr60449 and pr61776. I can help
Martin to test and post that part as an independent patch later.

bootstrap and regression pass on x86_64-linux-gnu. Is it ok?

Thanks,
Wei.

[-- Attachment #2: patch.txt --]
[-- Type: text/plain, Size: 8988 bytes --]

ChangeLog:

2014-07-27  Martin Jambor  <mjambor@suse.cz>
	    Wei Mi  <wmi@google.com>

	PR ipa/60449
	PR middle-end/61776
	* tree-cfgcleanup.c (update_no_abnormal_goto_attr): New function.
	(cleanup_tree_cfg_1): Use update_no_abnormal_goto_attr.
	* gimple.c (gimple_call_initialize_no_abnormal_goto): New function.
	(gimple_build_call_1): Use gimple_call_initialize_no_abnormal_goto.
	(gimple_build_call_internal_1): Ditto.
	* gimple.h (enum gf_mask): Added GF_NO_ABNORMAL_GOTO.
	(gimple_call_set_no_abnormal_goto): New function.
	(gimple_call_no_abnormal_goto_p): Ditto.
	* tree-cfg.c (call_can_make_abnormal_goto):
	Use gimple_call_no_abnormal_goto_p.
	(execute_fixup_cfg): Use gimple_call_set_no_abnormal_goto.

2014-07-27  Martin Jambor  <mjambor@suse.cz>
	    Wei Mi  <wmi@google.com>

	PR ipa/60449
	PR middle-end/61776
	* testsuite/gcc.dg/pr61776.c: New test.
	* testsuite/gcc.dg/lto/pr60449_1.c: New test.
	* testsuite/gcc.dg/lto/pr60449_0.c: New test.

Index: tree-cfgcleanup.c
===================================================================
--- tree-cfgcleanup.c	(revision 212442)
+++ tree-cfgcleanup.c	(working copy)
@@ -621,6 +621,28 @@ split_bbs_on_noreturn_calls (void)
   return changed;
 }
 
+/* Update GF_NO_ABNORMAL_GOTO attribute for call stmts in BB according
+   to gimple_call_flags.  */
+
+static void
+update_no_abnormal_goto_attr (basic_block bb)
+{
+  gimple_stmt_iterator gsi;
+  for (gsi = gsi_start_bb (bb); !gsi_end_p (gsi); gsi_next (&gsi))
+    {
+      gimple stmt = gsi_stmt (gsi);
+
+      if (!is_gimple_call (stmt))
+	continue;
+
+      int flags = gimple_call_flags (stmt);
+      if ((flags & (ECF_CONST | ECF_PURE)
+           && !(flags & ECF_LOOPING_CONST_OR_PURE))
+	  || (flags & ECF_LEAF))
+	gimple_call_set_no_abnormal_goto (stmt, true);
+    }
+}
+
 /* Tries to cleanup cfg in basic block BB.  Returns true if anything
    changes.  */
 
@@ -672,7 +694,10 @@ cleanup_tree_cfg_1 (void)
     {
       bb = BASIC_BLOCK_FOR_FN (cfun, i);
       if (bb)
-	retval |= cleanup_tree_cfg_bb (bb);
+	{
+	  update_no_abnormal_goto_attr (bb);
+	  retval |= cleanup_tree_cfg_bb (bb);
+	}
     }
 
   /* Now process the altered blocks, as long as any are available.  */
@@ -687,6 +712,7 @@ cleanup_tree_cfg_1 (void)
       if (!bb)
 	continue;
 
+      update_no_abnormal_goto_attr (bb);
       retval |= cleanup_tree_cfg_bb (bb);
 
       /* Rerun split_bbs_on_noreturn_calls, in case we have altered any noreturn
Index: gimple.c
===================================================================
--- gimple.c	(revision 212442)
+++ gimple.c	(working copy)
@@ -186,6 +186,19 @@ gimple_build_return (tree retval)
   return s;
 }
 
+/* Set GF_NO_ABNORMAL_GOTO attribute according to gimple_call_flags(STMT).  */
+
+void
+gimple_call_initialize_no_abnormal_goto (gimple stmt)
+{
+  int flags = gimple_call_flags (stmt);
+
+  if ((flags & (ECF_CONST | ECF_PURE)
+       && !(flags & ECF_LOOPING_CONST_OR_PURE))
+      || (flags & ECF_LEAF))
+    gimple_call_set_no_abnormal_goto (stmt, true);
+}
+
 /* Reset alias information on call S.  */
 
 void
@@ -215,6 +228,7 @@ gimple_build_call_1 (tree fn, unsigned n
   gimple_set_op (s, 1, fn);
   gimple_call_set_fntype (s, TREE_TYPE (TREE_TYPE (fn)));
   gimple_call_reset_alias_info (s);
+  gimple_call_initialize_no_abnormal_goto (s);
   return s;
 }
 
@@ -290,6 +304,7 @@ gimple_build_call_internal_1 (enum inter
   s->subcode |= GF_CALL_INTERNAL;
   gimple_call_set_internal_fn (s, fn);
   gimple_call_reset_alias_info (s);
+  gimple_call_initialize_no_abnormal_goto (s);
   return s;
 }
 
Index: gimple.h
===================================================================
--- gimple.h	(revision 212442)
+++ gimple.h	(working copy)
@@ -90,6 +90,7 @@ enum gf_mask {
     GF_CALL_NOTHROW		= 1 << 4,
     GF_CALL_ALLOCA_FOR_VAR	= 1 << 5,
     GF_CALL_INTERNAL		= 1 << 6,
+    GF_CALL_NO_ABNORMAL_GOTO    = 1 << 7,
     GF_OMP_PARALLEL_COMBINED	= 1 << 0,
     GF_OMP_FOR_KIND_MASK	= (1 << 2) - 1,
     GF_OMP_FOR_KIND_FOR		= 0,
@@ -2449,7 +2450,28 @@ gimple_call_internal_p (const_gimple gs)
 }
 
 
-/* Return the target of internal call GS.  */
+/* If NO_ABNORMAL_GOTO_P is true mark GIMPLE_CALL S as not having any
+   abnormal goto effect. It doesn't exclude EH.  */
+static inline void
+gimple_call_set_no_abnormal_goto (gimple s, bool no_abnormal_goto_p)
+{
+  GIMPLE_CHECK (s, GIMPLE_CALL);
+  if (no_abnormal_goto_p)
+    s->subcode |= GF_CALL_NO_ABNORMAL_GOTO;
+  else
+    s->subcode &= ~GF_CALL_NO_ABNORMAL_GOTO;
+}
+
+/* Return true if call GS calls an func without abnormal goto. Such call
+   could be a stmt in the middle of a bb.  */
+
+static inline bool
+gimple_call_no_abnormal_goto_p (const_gimple gs)
+{
+  GIMPLE_CHECK (gs, GIMPLE_CALL);
+  return (gs->subcode & GF_CALL_NO_ABNORMAL_GOTO) != 0;
+}
+
 
 static inline enum internal_fn
 gimple_call_internal_fn (const_gimple gs)
Index: tree-cfg.c
===================================================================
--- tree-cfg.c	(revision 212442)
+++ tree-cfg.c	(working copy)
@@ -2318,15 +2318,11 @@ call_can_make_abnormal_goto (gimple t)
       && !cfun->calls_setjmp)
    return false;
 
-  /* Likewise if the call has no side effects.  */
-  if (!gimple_has_side_effects (t))
+  /* If the call has been marked as having no abnormal goto.  */
+  if (gimple_call_no_abnormal_goto_p (t))
     return false;
-
-  /* Likewise if the called function is leaf.  */
-  if (gimple_call_flags (t) & ECF_LEAF)
-    return false;
-
-  return true;
+  else
+    return true;
 }
 
 
@@ -8485,6 +8481,11 @@ execute_fixup_cfg (void)
 		    }
 		}
 
+	      if ((flags & (ECF_CONST | ECF_PURE)
+		   && !(flags & ECF_LOOPING_CONST_OR_PURE))
+		  || (flags & ECF_LEAF))
+		gimple_call_set_no_abnormal_goto (stmt, true);
+
 	      if (flags & ECF_NORETURN
 		  && fixup_noreturn_call (stmt))
 		todo |= TODO_cleanup_cfg;
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;
+}
Index: testsuite/gcc.dg/lto/pr60449_1.c
===================================================================
--- testsuite/gcc.dg/lto/pr60449_1.c	(revision 0)
+++ testsuite/gcc.dg/lto/pr60449_1.c	(revision 0)
@@ -0,0 +1,76 @@
+extern int printf (const char *__restrict __format, ...);
+typedef long int __time_t;
+typedef long int __suseconds_t;
+struct timeval
+  {
+    __time_t tv_sec;
+    __suseconds_t tv_usec;
+  };
+struct timezone
+  {
+    int tz_minuteswest;
+    int tz_dsttime;
+  };
+typedef struct timezone *__restrict __timezone_ptr_t;
+extern int gettimeofday (struct timeval *__restrict __tv,
+    __timezone_ptr_t __tz) __attribute__ ((__nothrow__ , __leaf__)) __attribute__ ((__nonnull__ (1)));
+
+typedef long int __jmp_buf[8];
+typedef struct
+  {
+    unsigned long int __val[(1024 / (8 * sizeof (unsigned long int)))];
+  } __sigset_t;
+struct __jmp_buf_tag
+  {
+    __jmp_buf __jmpbuf;
+    int __mask_was_saved;
+    __sigset_t __saved_mask;
+  };
+typedef struct __jmp_buf_tag jmp_buf[1];
+
+extern int setjmp (jmp_buf __env) __attribute__ ((__nothrow__));
+extern void longjmp (struct __jmp_buf_tag __env[1], int __val)
+     __attribute__ ((__nothrow__)) __attribute__ ((__noreturn__));
+
+extern int bar (void);
+
+int __attribute__ ((noinline, noclone))
+get_input (void)
+{
+  return 0;
+}
+
+static jmp_buf buf;
+
+int foo (void)
+{
+  if (get_input ())
+    longjmp(buf, 1);
+  return 0;
+}
+
+volatile int z;
+
+
+int main (void)
+{
+  struct timeval tv;
+  struct timezone tz;
+
+  bar();
+  if (setjmp (buf))
+    return 1;
+
+  if (!get_input ())
+    {
+      gettimeofday (&tv, &tz);
+      z = 0;
+      printf ("This is from main %i\n", tz.tz_dsttime);
+    }
+
+  foo ();
+  bar ();
+  bar ();
+
+  return 0;
+}
Index: testsuite/gcc.dg/lto/pr60449_0.c
===================================================================
--- testsuite/gcc.dg/lto/pr60449_0.c	(revision 0)
+++ testsuite/gcc.dg/lto/pr60449_0.c	(revision 0)
@@ -0,0 +1,30 @@
+/* { dg-lto-do link } */
+
+extern int printf (const char *__restrict __format, ...);
+typedef long int __time_t;
+typedef long int __suseconds_t;
+
+struct timeval
+  {
+    __time_t tv_sec;
+    __suseconds_t tv_usec;
+  };
+
+struct timezone
+  {
+    int tz_minuteswest;
+    int tz_dsttime;
+  };
+typedef struct timezone *__restrict __timezone_ptr_t;
+
+extern int gettimeofday (struct timeval *__restrict __tv, __timezone_ptr_t __tz);
+
+int bar (void)
+{
+  struct timeval tv;
+  struct timezone tz;
+
+  gettimeofday (&tv, &tz);
+  printf ("This is from bar %i\n", tz.tz_dsttime);
+  return 5;
+}

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

[Wei Mi]

Ping.

On Sun, Jul 27, 2014 at 11:08 PM, Wei Mi <wmi@google.com> wrote:
>> 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 for the comment and thanks to Martin's patch. I try the patch.
> It works well to address both pr60449 and pr61776 after some
> extension. One extension is to replace GF_CALL_LEAF attribute using
> GF_CALL_NO_ABNORMAL_GOTO. That is because not only dropping "leaf"
> attribute in lto symbol merge could introduce the control flow
> verification problem in pr60449, dropping "const/pure" attributes
> could introduce the same problem too. It is unnecessary to introduce
> per-call attributes for all these three: ECF_LEAF/ECF_CONST/ECF_PURE,
> so GF_CALL_NO_ABNORMAL_GOTO is introduced to indicate that a call stmt
> has no abnormal goto.
>
> GF_CALL_NO_ABNORMAL_GOTO will be set according to gimple_call_flags()
> once gimple call stmt is created, then updated in execute_fixup_cfg
> and cleanup_tree_cfg.
>
> I posted the extended patch here. I didn't add the noreturn part in
> because it has no direct impact on pr60449 and pr61776. I can help
> Martin to test and post that part as an independent patch later.
>
> bootstrap and regression pass on x86_64-linux-gnu. Is it ok?
>
> Thanks,
> Wei.

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

[Richard Biener]

On Mon, Jul 28, 2014 at 8:08 AM, Wei Mi <wmi@google.com> wrote:
>> 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 for the comment and thanks to Martin's patch. I try the patch.
> It works well to address both pr60449 and pr61776 after some
> extension. One extension is to replace GF_CALL_LEAF attribute using
> GF_CALL_NO_ABNORMAL_GOTO. That is because not only dropping "leaf"
> attribute in lto symbol merge could introduce the control flow
> verification problem in pr60449, dropping "const/pure" attributes
> could introduce the same problem too. It is unnecessary to introduce
> per-call attributes for all these three: ECF_LEAF/ECF_CONST/ECF_PURE,
> so GF_CALL_NO_ABNORMAL_GOTO is introduced to indicate that a call stmt
> has no abnormal goto.
>
> GF_CALL_NO_ABNORMAL_GOTO will be set according to gimple_call_flags()
> once gimple call stmt is created, then updated in execute_fixup_cfg
> and cleanup_tree_cfg.
>
> I posted the extended patch here. I didn't add the noreturn part in
> because it has no direct impact on pr60449 and pr61776. I can help
> Martin to test and post that part as an independent patch later.
>
> bootstrap and regression pass on x86_64-linux-gnu. Is it ok?

+static void
+update_no_abnormal_goto_attr (basic_block bb)
+{
+  gimple_stmt_iterator gsi;
+  for (gsi = gsi_start_bb (bb); !gsi_end_p (gsi); gsi_next (&gsi))
+    {

it should be enough to check these on last stmts of a basic block, no?

That you call update_no_abnormal_goto_attr from two places
before cleanup_tree_cfg_bb suggests you may want to perform
this change in cleanup_control_flow_bb which already looks
at the last stmt only?

Btw, I originally had this whole idea of moving flags to the gimple
stmt level because of the "interesting" way we handle the noreturn
attribute (calls to noreturn functions also end basic-blocks).

Thus would it be possible to turn all these into a single flag,
GF_CALL_CTRL_ALTERING?  That is, cover everything
that is_ctrl_altering_stmt covers?  I suggest we initialize it at
CFG build time and only ever clear it later.

Sorry for not thinking about this earlier (and for the slow review).

Thanks,
Richard.

> Thanks,
> Wei.

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

[Wei Mi]

[-- Attachment #1: Type: text/plain, Size: 1352 bytes --]

Sorry for the late reply. I took some time to make myself more
familiar with NORETURN and related code, and finally I understood what
you mean and saw why only GF_CALL_CTRL_ALTERING was enough and
GF_CALL_NORETURN was unneeded. With your suggestion, the change looks
much briefer! Please check if the new patch attached is ok.

bootstrap and regression tests pass on x86_64-linux-gnu.

Thanks,
Wei.

> +static void
> +update_no_abnormal_goto_attr (basic_block bb)
> +{
> +  gimple_stmt_iterator gsi;
> +  for (gsi = gsi_start_bb (bb); !gsi_end_p (gsi); gsi_next (&gsi))
> +    {
>
> it should be enough to check these on last stmts of a basic block, no?

Yes, that is better.

>
> That you call update_no_abnormal_goto_attr from two places
> before cleanup_tree_cfg_bb suggests you may want to perform
> this change in cleanup_control_flow_bb which already looks
> at the last stmt only?

Changed.

>
> Btw, I originally had this whole idea of moving flags to the gimple
> stmt level because of the "interesting" way we handle the noreturn
> attribute (calls to noreturn functions also end basic-blocks).
>
> Thus would it be possible to turn all these into a single flag,
> GF_CALL_CTRL_ALTERING?  That is, cover everything
> that is_ctrl_altering_stmt covers?  I suggest we initialize it at
> CFG build time and only ever clear it later.

Good idea!

[-- Attachment #2: patch1.txt --]
[-- Type: text/plain, Size: 13932 bytes --]

ChangeLog:
2014-08-19  Martin Jambor  <mjambor@suse.cz>
	    Wei Mi  <wmi@google.com>

	PR ipa/60449
	PR middle-end/61776
	* tree-ssa-operands.c (update_stmt_operands): Remove
	MODIFIED_NORETURN_CALLS.
	* tree-cfgcleanup.c (cleanup_call_ctrl_altering_flag): New func.
	(cleanup_control_flow_bb): Use cleanup_call_ctrl_altering_flag.
	(split_bb_on_noreturn_calls): Renamed from split_bbs_on_noreturn_calls.
	(cleanup_tree_cfg_1): Use split_bb_on_noreturn_calls.
	* tree-ssanames.h: Remove MODIFIED_NORETURN_CALLS.
	* gimple.h (enum gf_mask): Add GF_CALL_CTRL_ALTERING.
	(gimple_call_set_ctrl_altering): New func.
	(gimple_call_ctrl_altering_p): Ditto.
	* tree-cfg.c (gimple_call_initialize_ctrl_altering): Ditto.
	(make_blocks): Use gimple_call_initialize_ctrl_altering.
	(is_ctrl_altering_stmt): Use gimple_call_ctrl_altering_p.
	(execute_fixup_cfg): Use gimple_call_ctrl_altering_p and
	remove MODIFIED_NORETURN_CALLS.

2014-08-19  Martin Jambor  <mjambor@suse.cz>
	    Wei Mi  <wmi@google.com>

	PR ipa/60449
	PR middle-end/61776
	* testsuite/gcc.dg/lto/pr60449_1.c: New test.
	* testsuite/gcc.dg/lto/pr60449_0.c: New test.
	* testsuite/gcc.dg/pr61776.c: New test.

Index: tree-ssa-operands.c
===================================================================
--- tree-ssa-operands.c	(revision 212442)
+++ tree-ssa-operands.c	(working copy)
@@ -1087,12 +1087,6 @@ update_stmt_operands (struct function *f
 
   timevar_push (TV_TREE_OPS);
 
-  /* If the stmt is a noreturn call queue it to be processed by
-     split_bbs_on_noreturn_calls during cfg cleanup.  */
-  if (is_gimple_call (stmt)
-      && gimple_call_noreturn_p (stmt))
-    vec_safe_push (MODIFIED_NORETURN_CALLS (fn), stmt);
-
   gcc_assert (gimple_modified_p (stmt));
   build_ssa_operands (fn, stmt);
   gimple_set_modified (stmt, false);
Index: tree-cfgcleanup.c
===================================================================
--- tree-cfgcleanup.c	(revision 212442)
+++ tree-cfgcleanup.c	(working copy)
@@ -162,6 +162,23 @@ cleanup_control_expr_graph (basic_block
   return retval;
 }
 
+/* Cleanup the GF_CALL_CTRL_ALTERING flag according to
+   to updated gimple_call_flags.  */
+
+static void
+cleanup_call_ctrl_altering_flag (gimple bb_end)
+{
+  if (!is_gimple_call (bb_end)
+      || !gimple_call_ctrl_altering_p (bb_end))
+    return;
+
+  int flags = gimple_call_flags (bb_end);
+  if (((flags & (ECF_CONST | ECF_PURE))
+       && !(flags & ECF_LOOPING_CONST_OR_PURE))
+      || (flags & ECF_LEAF))
+    gimple_call_set_ctrl_altering (bb_end, false);
+}
+
 /* Try to remove superfluous control structures in basic block BB.  Returns
    true if anything changes.  */
 
@@ -182,6 +199,9 @@ cleanup_control_flow_bb (basic_block bb)
 
   stmt = gsi_stmt (gsi);
 
+  /* Try to cleanup ctrl altering flag for call which ends bb.  */
+  cleanup_call_ctrl_altering_flag (stmt);
+
   if (gimple_code (stmt) == GIMPLE_COND
       || gimple_code (stmt) == GIMPLE_SWITCH)
     retval |= cleanup_control_expr_graph (bb, gsi);
@@ -594,30 +614,24 @@ fixup_noreturn_call (gimple stmt)
    known not to return, and remove the unreachable code.  */
 
 static bool
-split_bbs_on_noreturn_calls (void)
+split_bb_on_noreturn_calls (basic_block bb)
 {
   bool changed = false;
-  gimple stmt;
-  basic_block bb;
+  gimple_stmt_iterator gsi;
 
-  /* Detect cases where a mid-block call is now known not to return.  */
-  if (cfun->gimple_df)
-    while (vec_safe_length (MODIFIED_NORETURN_CALLS (cfun)))
-      {
-	stmt = MODIFIED_NORETURN_CALLS (cfun)->pop ();
-	bb = gimple_bb (stmt);
-	/* BB might be deleted at this point, so verify first
-	   BB is present in the cfg.  */
-	if (bb == NULL
-	    || bb->index < NUM_FIXED_BLOCKS
-	    || bb->index >= last_basic_block_for_fn (cfun)
-	    || BASIC_BLOCK_FOR_FN (cfun, bb->index) != bb
-	    || !gimple_call_noreturn_p (stmt))
-	  continue;
+  for (gsi = gsi_start_bb (bb); !gsi_end_p (gsi); gsi_next (&gsi))
+    {
+      gimple stmt = gsi_stmt (gsi);
+
+      if (!is_gimple_call (stmt))
+	continue;
 
+      if (gimple_call_noreturn_p (stmt))
 	changed |= fixup_noreturn_call (stmt);
-      }
+    }
 
+  if (changed)
+    bitmap_set_bit (cfgcleanup_altered_bbs, bb->index);
   return changed;
 }
 
@@ -655,8 +669,6 @@ cleanup_tree_cfg_1 (void)
   basic_block bb;
   unsigned i, n;
 
-  retval |= split_bbs_on_noreturn_calls ();
-
   /* Prepare the worklists of altered blocks.  */
   cfgcleanup_altered_bbs = BITMAP_ALLOC (NULL);
 
@@ -672,7 +684,10 @@ cleanup_tree_cfg_1 (void)
     {
       bb = BASIC_BLOCK_FOR_FN (cfun, i);
       if (bb)
-	retval |= cleanup_tree_cfg_bb (bb);
+	{
+	  retval |= cleanup_tree_cfg_bb (bb);
+	  retval |= split_bb_on_noreturn_calls (bb);
+	}
     }
 
   /* Now process the altered blocks, as long as any are available.  */
@@ -689,9 +704,9 @@ cleanup_tree_cfg_1 (void)
 
       retval |= cleanup_tree_cfg_bb (bb);
 
-      /* Rerun split_bbs_on_noreturn_calls, in case we have altered any noreturn
+      /* Rerun split_bb_on_noreturn_calls, in case we have altered any noreturn
 	 calls.  */
-      retval |= split_bbs_on_noreturn_calls ();
+      retval |= split_bb_on_noreturn_calls (bb);
     }
 
   end_recording_case_labels ();
Index: tree-ssanames.h
===================================================================
--- tree-ssanames.h	(revision 212442)
+++ tree-ssanames.h	(working copy)
@@ -57,7 +57,6 @@ struct GTY ((variable_size)) range_info_
 
 
 #define SSANAMES(fun) (fun)->gimple_df->ssa_names
-#define MODIFIED_NORETURN_CALLS(fun) (fun)->gimple_df->modified_noreturn_calls
 #define DEFAULT_DEFS(fun) (fun)->gimple_df->default_defs
 
 #define num_ssa_names (vec_safe_length (cfun->gimple_df->ssa_names))
Index: gimple.h
===================================================================
--- gimple.h	(revision 212442)
+++ gimple.h	(working copy)
@@ -90,6 +90,7 @@ enum gf_mask {
     GF_CALL_NOTHROW		= 1 << 4,
     GF_CALL_ALLOCA_FOR_VAR	= 1 << 5,
     GF_CALL_INTERNAL		= 1 << 6,
+    GF_CALL_CTRL_ALTERING       = 1 << 7,
     GF_OMP_PARALLEL_COMBINED	= 1 << 0,
     GF_OMP_FOR_KIND_MASK	= (1 << 2) - 1,
     GF_OMP_FOR_KIND_FOR		= 0,
@@ -2458,6 +2459,29 @@ gimple_call_internal_fn (const_gimple gs
   return static_cast <const gimple_statement_call *> (gs)->u.internal_fn;
 }
 
+/* If CTRL_ALTERING_P is true, mark GIMPLE_CALL S to be a stmt
+   that could alter control flow.  */
+
+static inline void
+gimple_call_set_ctrl_altering (gimple s, bool ctrl_altering_p)
+{
+  GIMPLE_CHECK (s, GIMPLE_CALL);
+  if (ctrl_altering_p)
+    s->subcode |= GF_CALL_CTRL_ALTERING;
+  else
+    s->subcode &= ~GF_CALL_CTRL_ALTERING;
+}
+
+/* Return true if call GS calls an func whose GF_CALL_CTRL_ALTERING
+   flag is set. Such call could not be a stmt in the middle of a bb.  */
+
+static inline bool
+gimple_call_ctrl_altering_p (const_gimple gs)
+{
+  GIMPLE_CHECK (gs, GIMPLE_CALL);
+  return (gs->subcode & GF_CALL_CTRL_ALTERING) != 0;
+}
+
 
 /* Return the function type of the function called by GS.  */
 
Index: tree-cfg.c
===================================================================
--- tree-cfg.c	(revision 212442)
+++ tree-cfg.c	(working copy)
@@ -164,6 +164,7 @@ static int gimple_verify_flow_info (void
 static void gimple_make_forwarder_block (edge);
 static gimple first_non_label_stmt (basic_block);
 static bool verify_gimple_transaction (gimple);
+static bool call_can_make_abnormal_goto (gimple);
 
 /* Flowgraph optimization and cleanup.  */
 static void gimple_merge_blocks (basic_block, basic_block);
@@ -437,6 +438,32 @@ assert_unreachable_fallthru_edge_p (edge
 }
 
 
+/* Initialize GF_CALL_CTRL_ALTERING flag, which indicates the call
+   could alter control flow except via eh. We initialize the flag at
+   CFG build time and only ever clear it later.  */
+
+static void
+gimple_call_initialize_ctrl_altering (gimple stmt)
+{
+  int flags = gimple_call_flags (stmt);
+
+  /* A call alters control flow if it can make an abnormal goto.  */
+  if (call_can_make_abnormal_goto (stmt)
+      /* A call also alters control flow if it does not return.  */
+      || flags & ECF_NORETURN
+      /* TM ending statements have backedges out of the transaction.
+	 Return true so we split the basic block containing them.
+	 Note that the TM_BUILTIN test is merely an optimization.  */
+      || ((flags & ECF_TM_BUILTIN)
+	  && is_tm_ending_fndecl (gimple_call_fndecl (stmt)))
+      /* BUILT_IN_RETURN call is same as return statement.  */
+      || gimple_call_builtin_p (stmt, BUILT_IN_RETURN))
+    gimple_call_set_ctrl_altering (stmt, true);
+  else
+    gimple_call_set_ctrl_altering (stmt, false);
+}
+
+
 /* Build a flowgraph for the sequence of stmts SEQ.  */
 
 static void
@@ -455,6 +482,9 @@ make_blocks (gimple_seq seq)
       prev_stmt = stmt;
       stmt = gsi_stmt (i);
 
+      if (stmt && is_gimple_call (stmt))
+	gimple_call_initialize_ctrl_altering (stmt);
+
       /* If the statement starts a new basic block or if we have determined
 	 in a previous pass that we need to create a new block for STMT, do
 	 so now.  */
@@ -2374,28 +2404,10 @@ is_ctrl_altering_stmt (gimple t)
   switch (gimple_code (t))
     {
     case GIMPLE_CALL:
-      {
-	int flags = gimple_call_flags (t);
-
-	/* A call alters control flow if it can make an abnormal goto.  */
-	if (call_can_make_abnormal_goto (t))
-	  return true;
-
-	/* A call also alters control flow if it does not return.  */
-	if (flags & ECF_NORETURN)
-	  return true;
-
-	/* TM ending statements have backedges out of the transaction.
-	   Return true so we split the basic block containing them.
-	   Note that the TM_BUILTIN test is merely an optimization.  */
-	if ((flags & ECF_TM_BUILTIN)
-	    && is_tm_ending_fndecl (gimple_call_fndecl (t)))
-	  return true;
-
-	/* BUILT_IN_RETURN call is same as return statement.  */
-	if (gimple_call_builtin_p (t, BUILT_IN_RETURN))
-	  return true;
-      }
+      /* Per stmt call flag indicates whether the call could alter
+	 controlflow.  */
+      if (gimple_call_ctrl_altering_p (t))
+	return true;
       break;
 
     case GIMPLE_EH_DISPATCH:
@@ -8547,6 +8559,8 @@ execute_fixup_cfg (void)
 		  && (!is_gimple_call (stmt)
 		      || (gimple_call_flags (stmt) & ECF_NORETURN) == 0)))
 	    {
+	      if (stmt && is_gimple_call (stmt))
+		gimple_call_set_ctrl_altering (stmt, false);
 	      stmt = gimple_build_call
 		  (builtin_decl_implicit (BUILT_IN_UNREACHABLE), 0);
 	      gimple_stmt_iterator gsi = gsi_last_bb (bb);
@@ -8557,10 +8571,6 @@ execute_fixup_cfg (void)
   if (count_scale != REG_BR_PROB_BASE)
     compute_function_frequency ();
 
-  /* We just processed all calls.  */
-  if (cfun->gimple_df)
-    vec_free (MODIFIED_NORETURN_CALLS (cfun));
-
   /* Dump a textual representation of the flowgraph.  */
   if (dump_file)
     gimple_dump_cfg (dump_file, dump_flags);
Index: testsuite/gcc.dg/lto/pr60449_1.c
===================================================================
--- testsuite/gcc.dg/lto/pr60449_1.c	(revision 0)
+++ testsuite/gcc.dg/lto/pr60449_1.c	(revision 0)
@@ -0,0 +1,76 @@
+extern int printf (const char *__restrict __format, ...);
+typedef long int __time_t;
+typedef long int __suseconds_t;
+struct timeval
+  {
+    __time_t tv_sec;
+    __suseconds_t tv_usec;
+  };
+struct timezone
+  {
+    int tz_minuteswest;
+    int tz_dsttime;
+  };
+typedef struct timezone *__restrict __timezone_ptr_t;
+extern int gettimeofday (struct timeval *__restrict __tv,
+    __timezone_ptr_t __tz) __attribute__ ((__nothrow__ , __leaf__)) __attribute__ ((__nonnull__ (1)));
+
+typedef long int __jmp_buf[8];
+typedef struct
+  {
+    unsigned long int __val[(1024 / (8 * sizeof (unsigned long int)))];
+  } __sigset_t;
+struct __jmp_buf_tag
+  {
+    __jmp_buf __jmpbuf;
+    int __mask_was_saved;
+    __sigset_t __saved_mask;
+  };
+typedef struct __jmp_buf_tag jmp_buf[1];
+
+extern int setjmp (jmp_buf __env) __attribute__ ((__nothrow__));
+extern void longjmp (struct __jmp_buf_tag __env[1], int __val)
+     __attribute__ ((__nothrow__)) __attribute__ ((__noreturn__));
+
+extern int bar (void);
+
+int __attribute__ ((noinline, noclone))
+get_input (void)
+{
+  return 0;
+}
+
+static jmp_buf buf;
+
+int foo (void)
+{
+  if (get_input ())
+    longjmp(buf, 1);
+  return 0;
+}
+
+volatile int z;
+
+
+int main (void)
+{
+  struct timeval tv;
+  struct timezone tz;
+
+  bar();
+  if (setjmp (buf))
+    return 1;
+
+  if (!get_input ())
+    {
+      gettimeofday (&tv, &tz);
+      z = 0;
+      printf ("This is from main %i\n", tz.tz_dsttime);
+    }
+
+  foo ();
+  bar ();
+  bar ();
+
+  return 0;
+}
Index: testsuite/gcc.dg/lto/pr60449_0.c
===================================================================
--- testsuite/gcc.dg/lto/pr60449_0.c	(revision 0)
+++ testsuite/gcc.dg/lto/pr60449_0.c	(revision 0)
@@ -0,0 +1,30 @@
+/* { dg-lto-do link } */
+
+extern int printf (const char *__restrict __format, ...);
+typedef long int __time_t;
+typedef long int __suseconds_t;
+
+struct timeval
+  {
+    __time_t tv_sec;
+    __suseconds_t tv_usec;
+  };
+
+struct timezone
+  {
+    int tz_minuteswest;
+    int tz_dsttime;
+  };
+typedef struct timezone *__restrict __timezone_ptr_t;
+
+extern int gettimeofday (struct timeval *__restrict __tv, __timezone_ptr_t __tz);
+
+int bar (void)
+{
+  struct timeval tv;
+  struct timezone tz;
+
+  gettimeofday (&tv, &tz);
+  printf ("This is from bar %i\n", tz.tz_dsttime);
+  return 5;
+}
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;
+}

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

[Richard Biener]

On Tue, Aug 19, 2014 at 8:46 PM, Wei Mi <wmi@google.com> wrote:
> Sorry for the late reply. I took some time to make myself more
> familiar with NORETURN and related code, and finally I understood what
> you mean and saw why only GF_CALL_CTRL_ALTERING was enough and
> GF_CALL_NORETURN was unneeded. With your suggestion, the change looks
> much briefer! Please check if the new patch attached is ok.

Indeed - much nicer than I originally thought.  Thanks for bearing
with me.

> bootstrap and regression tests pass on x86_64-linux-gnu.

Ok!

Thanks,
Richard.

> Thanks,
> Wei.
>
>> +static void
>> +update_no_abnormal_goto_attr (basic_block bb)
>> +{
>> +  gimple_stmt_iterator gsi;
>> +  for (gsi = gsi_start_bb (bb); !gsi_end_p (gsi); gsi_next (&gsi))
>> +    {
>>
>> it should be enough to check these on last stmts of a basic block, no?
>
> Yes, that is better.
>
>>
>> That you call update_no_abnormal_goto_attr from two places
>> before cleanup_tree_cfg_bb suggests you may want to perform
>> this change in cleanup_control_flow_bb which already looks
>> at the last stmt only?
>
> Changed.
>
>>
>> Btw, I originally had this whole idea of moving flags to the gimple
>> stmt level because of the "interesting" way we handle the noreturn
>> attribute (calls to noreturn functions also end basic-blocks).
>>
>> Thus would it be possible to turn all these into a single flag,
>> GF_CALL_CTRL_ALTERING?  That is, cover everything
>> that is_ctrl_altering_stmt covers?  I suggest we initialize it at
>> CFG build time and only ever clear it later.
>
> Good idea!