* [RFA][PATCH] Run CFG cleanups after reassociation as needed
@ 2015-12-03 17:55 Jeff Law
2015-12-04 10:19 ` Richard Biener
0 siblings, 1 reply; 4+ messages in thread
From: Jeff Law @ 2015-12-03 17:55 UTC (permalink / raw)
To: gcc-patches
[-- Attachment #1: Type: text/plain, Size: 793 bytes --]
This is something I noticed while working on fixing 67816.
Essentially I was seeing trivially true or trivially false conditionals
left in the IL for DOM to clean up.
While DOM can and will clean that crud up, but a trivially true or
trivially false conditional ought to be detected and cleaned up by
cleanup_cfg.
It turns out the reassociation pass does not schedule a CFG cleanup even
in cases where it optimizes a conditional to TRUE or FALSE.
Bubbling up an indicator that we optimized away a conditional and using
that to trigger a CFG cleanup is trivial.
While I have a slight preference to see this fix in GCC 6, if folks
object and want this to wait for GCC 7 stage1, I'd understand.
Bootstrapped and regression tested on x86_64-linux-gnu.
OK for the trunk?
Thanks,
Jeff
[-- Attachment #2: patch --]
[-- Type: text/plain, Size: 8497 bytes --]
diff --git a/gcc/ChangeLog b/gcc/ChangeLog
index 04dbcb0..61a5e54 100644
--- a/gcc/ChangeLog
+++ b/gcc/ChangeLog
@@ -1,3 +1,12 @@
+2015-12-03 Jeff Law <law@redhat.com>
+
+ * tree-ssa-reassoc.c (maybe_optimize_range_tests): Return boolean
+ indicating if a gimple conditional was optimized to true/false.
+ (reassociate_bb): Bubble up return value from
+ maybe_optimize_range_tests.
+ (do_reassoc): Similarly, but for reassociate_bb.
+ (execute_reassoc): Return TODO_cleanup_cfg as needed.
+
2015-11-27 Jiri Engelthaler <engycz@gmail.com>
PR driver/68029
diff --git a/gcc/testsuite/ChangeLog b/gcc/testsuite/ChangeLog
index 4e62a06..893aab1 100644
--- a/gcc/testsuite/ChangeLog
+++ b/gcc/testsuite/ChangeLog
@@ -1,3 +1,7 @@
+2015-12-02 Jeff Law <law@redhat.com>
+
+ * gcc.dg/tree-ssa/reassoc-43.c: New test.
+
2015-12-02 Andreas Krebbel <krebbel@linux.vnet.ibm.com>
* gcc.dg/optimize-bswapdi-1.c: Force using -mzarch on s390 and
diff --git a/gcc/testsuite/gcc.dg/tree-ssa/reassoc-43.c b/gcc/testsuite/gcc.dg/tree-ssa/reassoc-43.c
new file mode 100644
index 0000000..ea44f30
--- /dev/null
+++ b/gcc/testsuite/gcc.dg/tree-ssa/reassoc-43.c
@@ -0,0 +1,53 @@
+/* { dg-do compile } */
+/* { dg-options "-O2 -fdump-tree-reassoc -w" } */
+
+typedef union tree_node *tree;
+enum cpp_ttype { CPP_COLON, CPP_SEMICOLON, CPP_CLOSE_BRACE, CPP_COMMA };
+enum rid { RID_STATIC = 0, RID_ATTRIBUTE, };
+typedef struct c_token
+{
+ enum cpp_ttype type:8;
+}
+c_token;
+typedef struct c_parser
+{
+ c_token tokens[2];
+ short tokens_avail;
+}
+c_parser;
+__inline__ c_token *
+c_parser_peek_token (c_parser * parser)
+{
+ if (parser->tokens_avail == 0)
+ {
+ parser->tokens_avail = 1;
+ }
+ return &parser->tokens[0];
+}
+
+__inline__ unsigned char
+c_parser_next_token_is (c_parser * parser, enum cpp_ttype type)
+{
+ return c_parser_peek_token (parser)->type == type;
+}
+
+void
+c_parser_translation_unit (c_parser * parser)
+{
+ tree prefix_attrs;
+ tree all_prefix_attrs;
+ while (1)
+ {
+ if (c_parser_next_token_is (parser, CPP_COLON)
+ || c_parser_next_token_is (parser, CPP_COMMA)
+ || c_parser_next_token_is (parser, CPP_SEMICOLON)
+ || c_parser_next_token_is (parser, CPP_CLOSE_BRACE)
+ || c_parser_next_token_is_keyword (parser, RID_ATTRIBUTE))
+ {
+ if (c_parser_next_token_is_keyword (parser, RID_ATTRIBUTE))
+ all_prefix_attrs =
+ chainon (c_parser_attributes (parser), prefix_attrs);
+ }
+ }
+}
+/* { dg-final { scan-tree-dump-not "0 != 0" "reassoc2"} } */
diff --git a/gcc/tree-ssa-reassoc.c b/gcc/tree-ssa-reassoc.c
index dfd0da1..315b0bf 100644
--- a/gcc/tree-ssa-reassoc.c
+++ b/gcc/tree-ssa-reassoc.c
@@ -2976,9 +2976,15 @@ struct inter_bb_range_test_entry
unsigned int first_idx, last_idx;
};
-/* Inter-bb range test optimization. */
+/* Inter-bb range test optimization.
-static void
+ Returns TRUE if a gimple conditional is optimized to a true/false,
+ otherwise return FALSE.
+
+ This indicates to the caller that it should run a CFG cleanup pass
+ once reassociation is completed. */
+
+static bool
maybe_optimize_range_tests (gimple *stmt)
{
basic_block first_bb = gimple_bb (stmt);
@@ -2990,6 +2996,7 @@ maybe_optimize_range_tests (gimple *stmt)
auto_vec<operand_entry *> ops;
auto_vec<inter_bb_range_test_entry> bbinfo;
bool any_changes = false;
+ bool cfg_cleanup_needed = false;
/* Consider only basic blocks that end with GIMPLE_COND or
a cast statement satisfying final_range_test_p. All
@@ -2998,15 +3005,15 @@ maybe_optimize_range_tests (gimple *stmt)
if (gimple_code (stmt) == GIMPLE_COND)
{
if (EDGE_COUNT (first_bb->succs) != 2)
- return;
+ return cfg_cleanup_needed;
}
else if (final_range_test_p (stmt))
other_bb = single_succ (first_bb);
else
- return;
+ return cfg_cleanup_needed;
if (stmt_could_throw_p (stmt))
- return;
+ return cfg_cleanup_needed;
/* As relative ordering of post-dominator sons isn't fixed,
maybe_optimize_range_tests can be called first on any
@@ -3030,14 +3037,14 @@ maybe_optimize_range_tests (gimple *stmt)
/* As non-GIMPLE_COND last stmt always terminates the range,
if forward search didn't discover anything, just give up. */
if (gimple_code (stmt) != GIMPLE_COND)
- return;
+ return cfg_cleanup_needed;
/* Look at both successors. Either it ends with a GIMPLE_COND
and satisfies suitable_cond_bb, or ends with a cast and
other_bb is that cast's successor. */
FOR_EACH_EDGE (e, ei, first_bb->succs)
if (!(e->flags & (EDGE_TRUE_VALUE | EDGE_FALSE_VALUE))
|| e->dest == first_bb)
- return;
+ return cfg_cleanup_needed;
else if (single_pred_p (e->dest))
{
stmt = last_stmt (e->dest);
@@ -3060,7 +3067,7 @@ maybe_optimize_range_tests (gimple *stmt)
}
}
if (other_bb == NULL)
- return;
+ return cfg_cleanup_needed;
}
/* Now do the forward search, moving last_bb to successor bbs
that aren't other_bb. */
@@ -3080,7 +3087,7 @@ maybe_optimize_range_tests (gimple *stmt)
last_bb = e->dest;
}
if (first_bb == last_bb)
- return;
+ return cfg_cleanup_needed;
/* Here basic blocks first_bb through last_bb's predecessor
end with GIMPLE_COND, all of them have one of the edges to
other_bb and another to another block in the range,
@@ -3289,10 +3296,18 @@ maybe_optimize_range_tests (gimple *stmt)
&& ops[bbinfo[idx].first_idx]->op != NULL_TREE)
{
gcond *cond_stmt = as_a <gcond *> (last_stmt (bb));
+ /* If we collapse the conditional to a true/false
+ condition, then bubble that knowledge up to our caller. */
if (integer_zerop (ops[bbinfo[idx].first_idx]->op))
- gimple_cond_make_false (cond_stmt);
+ {
+ gimple_cond_make_false (cond_stmt);
+ cfg_cleanup_needed = true;
+ }
else if (integer_onep (ops[bbinfo[idx].first_idx]->op))
- gimple_cond_make_true (cond_stmt);
+ {
+ gimple_cond_make_true (cond_stmt);
+ cfg_cleanup_needed = true;
+ }
else
{
gimple_cond_set_code (cond_stmt, NE_EXPR);
@@ -3306,6 +3321,7 @@ maybe_optimize_range_tests (gimple *stmt)
break;
}
}
+ return cfg_cleanup_needed;
}
/* Return true if OPERAND is defined by a PHI node which uses the LHS
@@ -4753,17 +4769,20 @@ transform_stmt_to_multiply (gimple_stmt_iterator *gsi, gimple *stmt,
}
/* Reassociate expressions in basic block BB and its post-dominator as
- children. */
+ children.
-static void
+ Bubble up return status from maybe_optimize_range_tests. */
+
+static bool
reassociate_bb (basic_block bb)
{
gimple_stmt_iterator gsi;
basic_block son;
gimple *stmt = last_stmt (bb);
+ bool cfg_cleanup_needed = false;
if (stmt && !gimple_visited_p (stmt))
- maybe_optimize_range_tests (stmt);
+ cfg_cleanup_needed |= maybe_optimize_range_tests (stmt);
for (gsi = gsi_last_bb (bb); !gsi_end_p (gsi); gsi_prev (&gsi))
{
@@ -4922,7 +4941,9 @@ reassociate_bb (basic_block bb)
for (son = first_dom_son (CDI_POST_DOMINATORS, bb);
son;
son = next_dom_son (CDI_POST_DOMINATORS, son))
- reassociate_bb (son);
+ cfg_cleanup_needed |= reassociate_bb (son);
+
+ return cfg_cleanup_needed;
}
/* Add jumps around shifts for range tests turned into bit tests.
@@ -5028,11 +5049,13 @@ debug_ops_vector (vec<operand_entry *> ops)
dump_ops_vector (stderr, ops);
}
-static void
+/* Bubble up return status from reassociate_bb. */
+
+static bool
do_reassoc (void)
{
break_up_subtract_bb (ENTRY_BLOCK_PTR_FOR_FN (cfun));
- reassociate_bb (EXIT_BLOCK_PTR_FOR_FN (cfun));
+ return reassociate_bb (EXIT_BLOCK_PTR_FOR_FN (cfun));
}
/* Initialize the reassociation pass. */
@@ -5106,7 +5129,10 @@ fini_reassoc (void)
}
/* Gate and execute functions for Reassociation. If INSERT_POWI_P, enable
- insertion of __builtin_powi calls. */
+ insertion of __builtin_powi calls.
+
+ Returns TODO_cfg_cleanup if a CFG cleanup pass is desired due to
+ optimization of a gimple conditional. Otherwise returns zero. */
static unsigned int
execute_reassoc (bool insert_powi_p)
@@ -5115,12 +5141,13 @@ execute_reassoc (bool insert_powi_p)
init_reassoc ();
- do_reassoc ();
+ bool cfg_cleanup_needed;
+ cfg_cleanup_needed = do_reassoc ();
repropagate_negates ();
branch_fixup ();
fini_reassoc ();
- return 0;
+ return cfg_cleanup_needed ? TODO_cleanup_cfg : 0;
}
namespace {
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [RFA][PATCH] Run CFG cleanups after reassociation as needed
2015-12-03 17:55 [RFA][PATCH] Run CFG cleanups after reassociation as needed Jeff Law
@ 2015-12-04 10:19 ` Richard Biener
2015-12-04 16:20 ` Jeff Law
0 siblings, 1 reply; 4+ messages in thread
From: Richard Biener @ 2015-12-04 10:19 UTC (permalink / raw)
To: Jeff Law; +Cc: GCC Patches
On Thu, Dec 3, 2015 at 6:54 PM, Jeff Law <law@redhat.com> wrote:
> This is something I noticed while working on fixing 67816.
>
> Essentially I was seeing trivially true or trivially false conditionals left
> in the IL for DOM to clean up.
>
> While DOM can and will clean that crud up, but a trivially true or trivially
> false conditional ought to be detected and cleaned up by cleanup_cfg.
>
> It turns out the reassociation pass does not schedule a CFG cleanup even in
> cases where it optimizes a conditional to TRUE or FALSE.
>
> Bubbling up an indicator that we optimized away a conditional and using that
> to trigger a CFG cleanup is trivial.
>
> While I have a slight preference to see this fix in GCC 6, if folks object
> and want this to wait for GCC 7 stage1, I'd understand.
>
> Bootstrapped and regression tested on x86_64-linux-gnu.
>
> OK for the trunk?
Ok. [I always hoped we can at some point assert we don't have trivially
optimizable control flow in the IL]
Richard.
> Thanks,
> Jeff
>
> diff --git a/gcc/ChangeLog b/gcc/ChangeLog
> index 04dbcb0..61a5e54 100644
> --- a/gcc/ChangeLog
> +++ b/gcc/ChangeLog
> @@ -1,3 +1,12 @@
> +2015-12-03 Jeff Law <law@redhat.com>
> +
> + * tree-ssa-reassoc.c (maybe_optimize_range_tests): Return boolean
> + indicating if a gimple conditional was optimized to true/false.
> + (reassociate_bb): Bubble up return value from
> + maybe_optimize_range_tests.
> + (do_reassoc): Similarly, but for reassociate_bb.
> + (execute_reassoc): Return TODO_cleanup_cfg as needed.
> +
> 2015-11-27 Jiri Engelthaler <engycz@gmail.com>
>
> PR driver/68029
> diff --git a/gcc/testsuite/ChangeLog b/gcc/testsuite/ChangeLog
> index 4e62a06..893aab1 100644
> --- a/gcc/testsuite/ChangeLog
> +++ b/gcc/testsuite/ChangeLog
> @@ -1,3 +1,7 @@
> +2015-12-02 Jeff Law <law@redhat.com>
> +
> + * gcc.dg/tree-ssa/reassoc-43.c: New test.
> +
> 2015-12-02 Andreas Krebbel <krebbel@linux.vnet.ibm.com>
>
> * gcc.dg/optimize-bswapdi-1.c: Force using -mzarch on s390 and
> diff --git a/gcc/testsuite/gcc.dg/tree-ssa/reassoc-43.c
> b/gcc/testsuite/gcc.dg/tree-ssa/reassoc-43.c
> new file mode 100644
> index 0000000..ea44f30
> --- /dev/null
> +++ b/gcc/testsuite/gcc.dg/tree-ssa/reassoc-43.c
> @@ -0,0 +1,53 @@
> +/* { dg-do compile } */
> +/* { dg-options "-O2 -fdump-tree-reassoc -w" } */
> +
> +typedef union tree_node *tree;
> +enum cpp_ttype { CPP_COLON, CPP_SEMICOLON, CPP_CLOSE_BRACE, CPP_COMMA };
> +enum rid { RID_STATIC = 0, RID_ATTRIBUTE, };
> +typedef struct c_token
> +{
> + enum cpp_ttype type:8;
> +}
> +c_token;
> +typedef struct c_parser
> +{
> + c_token tokens[2];
> + short tokens_avail;
> +}
> +c_parser;
> +__inline__ c_token *
> +c_parser_peek_token (c_parser * parser)
> +{
> + if (parser->tokens_avail == 0)
> + {
> + parser->tokens_avail = 1;
> + }
> + return &parser->tokens[0];
> +}
> +
> +__inline__ unsigned char
> +c_parser_next_token_is (c_parser * parser, enum cpp_ttype type)
> +{
> + return c_parser_peek_token (parser)->type == type;
> +}
> +
> +void
> +c_parser_translation_unit (c_parser * parser)
> +{
> + tree prefix_attrs;
> + tree all_prefix_attrs;
> + while (1)
> + {
> + if (c_parser_next_token_is (parser, CPP_COLON)
> + || c_parser_next_token_is (parser, CPP_COMMA)
> + || c_parser_next_token_is (parser, CPP_SEMICOLON)
> + || c_parser_next_token_is (parser, CPP_CLOSE_BRACE)
> + || c_parser_next_token_is_keyword (parser, RID_ATTRIBUTE))
> + {
> + if (c_parser_next_token_is_keyword (parser, RID_ATTRIBUTE))
> + all_prefix_attrs =
> + chainon (c_parser_attributes (parser), prefix_attrs);
> + }
> + }
> +}
> +/* { dg-final { scan-tree-dump-not "0 != 0" "reassoc2"} } */
> diff --git a/gcc/tree-ssa-reassoc.c b/gcc/tree-ssa-reassoc.c
> index dfd0da1..315b0bf 100644
> --- a/gcc/tree-ssa-reassoc.c
> +++ b/gcc/tree-ssa-reassoc.c
> @@ -2976,9 +2976,15 @@ struct inter_bb_range_test_entry
> unsigned int first_idx, last_idx;
> };
>
> -/* Inter-bb range test optimization. */
> +/* Inter-bb range test optimization.
>
> -static void
> + Returns TRUE if a gimple conditional is optimized to a true/false,
> + otherwise return FALSE.
> +
> + This indicates to the caller that it should run a CFG cleanup pass
> + once reassociation is completed. */
> +
> +static bool
> maybe_optimize_range_tests (gimple *stmt)
> {
> basic_block first_bb = gimple_bb (stmt);
> @@ -2990,6 +2996,7 @@ maybe_optimize_range_tests (gimple *stmt)
> auto_vec<operand_entry *> ops;
> auto_vec<inter_bb_range_test_entry> bbinfo;
> bool any_changes = false;
> + bool cfg_cleanup_needed = false;
>
> /* Consider only basic blocks that end with GIMPLE_COND or
> a cast statement satisfying final_range_test_p. All
> @@ -2998,15 +3005,15 @@ maybe_optimize_range_tests (gimple *stmt)
> if (gimple_code (stmt) == GIMPLE_COND)
> {
> if (EDGE_COUNT (first_bb->succs) != 2)
> - return;
> + return cfg_cleanup_needed;
> }
> else if (final_range_test_p (stmt))
> other_bb = single_succ (first_bb);
> else
> - return;
> + return cfg_cleanup_needed;
>
> if (stmt_could_throw_p (stmt))
> - return;
> + return cfg_cleanup_needed;
>
> /* As relative ordering of post-dominator sons isn't fixed,
> maybe_optimize_range_tests can be called first on any
> @@ -3030,14 +3037,14 @@ maybe_optimize_range_tests (gimple *stmt)
> /* As non-GIMPLE_COND last stmt always terminates the range,
> if forward search didn't discover anything, just give up. */
> if (gimple_code (stmt) != GIMPLE_COND)
> - return;
> + return cfg_cleanup_needed;
> /* Look at both successors. Either it ends with a GIMPLE_COND
> and satisfies suitable_cond_bb, or ends with a cast and
> other_bb is that cast's successor. */
> FOR_EACH_EDGE (e, ei, first_bb->succs)
> if (!(e->flags & (EDGE_TRUE_VALUE | EDGE_FALSE_VALUE))
> || e->dest == first_bb)
> - return;
> + return cfg_cleanup_needed;
> else if (single_pred_p (e->dest))
> {
> stmt = last_stmt (e->dest);
> @@ -3060,7 +3067,7 @@ maybe_optimize_range_tests (gimple *stmt)
> }
> }
> if (other_bb == NULL)
> - return;
> + return cfg_cleanup_needed;
> }
> /* Now do the forward search, moving last_bb to successor bbs
> that aren't other_bb. */
> @@ -3080,7 +3087,7 @@ maybe_optimize_range_tests (gimple *stmt)
> last_bb = e->dest;
> }
> if (first_bb == last_bb)
> - return;
> + return cfg_cleanup_needed;
> /* Here basic blocks first_bb through last_bb's predecessor
> end with GIMPLE_COND, all of them have one of the edges to
> other_bb and another to another block in the range,
> @@ -3289,10 +3296,18 @@ maybe_optimize_range_tests (gimple *stmt)
> && ops[bbinfo[idx].first_idx]->op != NULL_TREE)
> {
> gcond *cond_stmt = as_a <gcond *> (last_stmt (bb));
> + /* If we collapse the conditional to a true/false
> + condition, then bubble that knowledge up to our caller. */
> if (integer_zerop (ops[bbinfo[idx].first_idx]->op))
> - gimple_cond_make_false (cond_stmt);
> + {
> + gimple_cond_make_false (cond_stmt);
> + cfg_cleanup_needed = true;
> + }
> else if (integer_onep (ops[bbinfo[idx].first_idx]->op))
> - gimple_cond_make_true (cond_stmt);
> + {
> + gimple_cond_make_true (cond_stmt);
> + cfg_cleanup_needed = true;
> + }
> else
> {
> gimple_cond_set_code (cond_stmt, NE_EXPR);
> @@ -3306,6 +3321,7 @@ maybe_optimize_range_tests (gimple *stmt)
> break;
> }
> }
> + return cfg_cleanup_needed;
> }
>
> /* Return true if OPERAND is defined by a PHI node which uses the LHS
> @@ -4753,17 +4769,20 @@ transform_stmt_to_multiply (gimple_stmt_iterator
> *gsi, gimple *stmt,
> }
>
> /* Reassociate expressions in basic block BB and its post-dominator as
> - children. */
> + children.
>
> -static void
> + Bubble up return status from maybe_optimize_range_tests. */
> +
> +static bool
> reassociate_bb (basic_block bb)
> {
> gimple_stmt_iterator gsi;
> basic_block son;
> gimple *stmt = last_stmt (bb);
> + bool cfg_cleanup_needed = false;
>
> if (stmt && !gimple_visited_p (stmt))
> - maybe_optimize_range_tests (stmt);
> + cfg_cleanup_needed |= maybe_optimize_range_tests (stmt);
>
> for (gsi = gsi_last_bb (bb); !gsi_end_p (gsi); gsi_prev (&gsi))
> {
> @@ -4922,7 +4941,9 @@ reassociate_bb (basic_block bb)
> for (son = first_dom_son (CDI_POST_DOMINATORS, bb);
> son;
> son = next_dom_son (CDI_POST_DOMINATORS, son))
> - reassociate_bb (son);
> + cfg_cleanup_needed |= reassociate_bb (son);
> +
> + return cfg_cleanup_needed;
> }
>
> /* Add jumps around shifts for range tests turned into bit tests.
> @@ -5028,11 +5049,13 @@ debug_ops_vector (vec<operand_entry *> ops)
> dump_ops_vector (stderr, ops);
> }
>
> -static void
> +/* Bubble up return status from reassociate_bb. */
> +
> +static bool
> do_reassoc (void)
> {
> break_up_subtract_bb (ENTRY_BLOCK_PTR_FOR_FN (cfun));
> - reassociate_bb (EXIT_BLOCK_PTR_FOR_FN (cfun));
> + return reassociate_bb (EXIT_BLOCK_PTR_FOR_FN (cfun));
> }
>
> /* Initialize the reassociation pass. */
> @@ -5106,7 +5129,10 @@ fini_reassoc (void)
> }
>
> /* Gate and execute functions for Reassociation. If INSERT_POWI_P, enable
> - insertion of __builtin_powi calls. */
> + insertion of __builtin_powi calls.
> +
> + Returns TODO_cfg_cleanup if a CFG cleanup pass is desired due to
> + optimization of a gimple conditional. Otherwise returns zero. */
>
> static unsigned int
> execute_reassoc (bool insert_powi_p)
> @@ -5115,12 +5141,13 @@ execute_reassoc (bool insert_powi_p)
>
> init_reassoc ();
>
> - do_reassoc ();
> + bool cfg_cleanup_needed;
> + cfg_cleanup_needed = do_reassoc ();
> repropagate_negates ();
> branch_fixup ();
>
> fini_reassoc ();
> - return 0;
> + return cfg_cleanup_needed ? TODO_cleanup_cfg : 0;
> }
>
> namespace {
>
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [RFA][PATCH] Run CFG cleanups after reassociation as needed
2015-12-04 10:19 ` Richard Biener
@ 2015-12-04 16:20 ` Jeff Law
2015-12-04 17:21 ` Richard Biener
0 siblings, 1 reply; 4+ messages in thread
From: Jeff Law @ 2015-12-04 16:20 UTC (permalink / raw)
To: Richard Biener; +Cc: GCC Patches
On 12/04/2015 03:19 AM, Richard Biener wrote:
> On Thu, Dec 3, 2015 at 6:54 PM, Jeff Law <law@redhat.com> wrote:
>> This is something I noticed while working on fixing 67816.
>>
>> Essentially I was seeing trivially true or trivially false conditionals left
>> in the IL for DOM to clean up.
>>
>> While DOM can and will clean that crud up, but a trivially true or trivially
>> false conditional ought to be detected and cleaned up by cleanup_cfg.
>>
>> It turns out the reassociation pass does not schedule a CFG cleanup even in
>> cases where it optimizes a conditional to TRUE or FALSE.
>>
>> Bubbling up an indicator that we optimized away a conditional and using that
>> to trigger a CFG cleanup is trivial.
>>
>> While I have a slight preference to see this fix in GCC 6, if folks object
>> and want this to wait for GCC 7 stage1, I'd understand.
>>
>> Bootstrapped and regression tested on x86_64-linux-gnu.
>>
>> OK for the trunk?
>
> Ok. [I always hoped we can at some point assert we don't have trivially
> optimizable control flow in the IL]
Presumably verification when a pass has completed, but not set
TODO_cleanup_cfg?
Jeff
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [RFA][PATCH] Run CFG cleanups after reassociation as needed
2015-12-04 16:20 ` Jeff Law
@ 2015-12-04 17:21 ` Richard Biener
0 siblings, 0 replies; 4+ messages in thread
From: Richard Biener @ 2015-12-04 17:21 UTC (permalink / raw)
To: Jeff Law; +Cc: GCC Patches
On December 4, 2015 5:20:52 PM GMT+01:00, Jeff Law <law@redhat.com> wrote:
>On 12/04/2015 03:19 AM, Richard Biener wrote:
>> On Thu, Dec 3, 2015 at 6:54 PM, Jeff Law <law@redhat.com> wrote:
>>> This is something I noticed while working on fixing 67816.
>>>
>>> Essentially I was seeing trivially true or trivially false
>conditionals left
>>> in the IL for DOM to clean up.
>>>
>>> While DOM can and will clean that crud up, but a trivially true or
>trivially
>>> false conditional ought to be detected and cleaned up by
>cleanup_cfg.
>>>
>>> It turns out the reassociation pass does not schedule a CFG cleanup
>even in
>>> cases where it optimizes a conditional to TRUE or FALSE.
>>>
>>> Bubbling up an indicator that we optimized away a conditional and
>using that
>>> to trigger a CFG cleanup is trivial.
>>>
>>> While I have a slight preference to see this fix in GCC 6, if folks
>object
>>> and want this to wait for GCC 7 stage1, I'd understand.
>>>
>>> Bootstrapped and regression tested on x86_64-linux-gnu.
>>>
>>> OK for the trunk?
>>
>> Ok. [I always hoped we can at some point assert we don't have
>trivially
>> optimizable control flow in the IL]
>Presumably verification when a pass has completed, but not set
>TODO_cleanup_cfg?
Well, kind-of, yes.
Richard.
>Jeff
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2015-12-04 17:21 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2015-12-03 17:55 [RFA][PATCH] Run CFG cleanups after reassociation as needed Jeff Law
2015-12-04 10:19 ` Richard Biener
2015-12-04 16:20 ` Jeff Law
2015-12-04 17:21 ` 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).