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!
ChangeLog:
2014-08-19 Martin Jambor <mjam...@suse.cz>
Wei Mi <w...@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 <mjam...@suse.cz>
Wei Mi <w...@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;
+}
