On 06/07/12 13:01, Richard Guenther wrote:
> On Thu, Jul 5, 2012 at 8:45 PM, Tom de Vries <[email protected]> wrote:
>> On 05/07/12 15:30, Michael Matz wrote:
>>> Hi,
>>>
>>> On Thu, 5 Jul 2012, Tom de Vries wrote:
>>>
>>>> The asserts allow the return result to be optimized, but not the cfg
>>>> conditions.
>>>>
>>>> AFAIU, we can insert the asserts earlier. F.i., we can insert
>>>> aD.1711_6 = ASSERT_EXPR <aD.1711_1(D), aD.1711_1(D) > 0>
>>>> before the GIMPLE_COND in bb2.
>>>
>>> Nope. That would require some more checks, in particular that the BB
>>> containing builtin_unreachable doesn't contain any other side-effects.
>>> Given this:
>>>
>>> if (i < 0)
>>> { do_something_interesting();
>>> __builtin_unreachable();
>>> }
>>>
>>> moving the assert before the if would remove the if condition, hence
>>> the call to do_something_interesting. You need to retain side-effects if
>>> there are any.
>>>
>>
>> Michael,
>>
>> Thanks for pointing that out.
>>
>> I tried a first stab at your suggestion of implementing the optimization in
>> pass_fold_builtins, it works for the test-case.
>
> +static bool
> +optimize_unreachable (gimple_stmt_iterator i)
> +{
> + gimple stmt;
> + basic_block bb;
> + edge_iterator ei;
> + edge e;
> +
> + for (gsi_prev (&i); !gsi_end_p (i); gsi_prev (&i))
> + {
> + stmt = gsi_stmt (i);
> + if (gimple_has_side_effects (stmt))
> + return false;
> + }
>
> I think we should rely on DCE to remove stmts without side-effects before
> __builtin_unreachable. Thus I'd make this
>
> basic_block bb = gsi_bb (i);
> for (gsi = gsi_start (bb); !gsi_end_p (i); gsi_next (&gsi))
> {
> if (is_gimple_debug ())
> continue;
> if (gimple_code () == GIMPLE_LABEL)
> /* Verify we do not need to preserve the label. */;
> if (gsi_stmt () != gsi_stmt (i))
> return false;
> }
>
> ...
>
> thus simply require the builtin be the first statement in the block.
Done.
>
> As for the label I'm concerned about
>
> void foo (int b, int c)
> {
> void *x = &&lab;
> if (b)
> {
> lab:
> __builtin_unreachable ();
> }
> lab2:
> if (c)
> x = &&lab2;
> goto *x;
> }
>
> non-sensical, of course, but "valid".
>
Added this example as test-case.
Bootstrapped and reg-tested (ada inclusive) on x86_64.
OK for trunk?
Thanks,
- Tom
2012-07-06 Tom de Vries <[email protected]>
Richard Guenther <[email protected]>
* tree-ssa-ccp.c (optimize_unreachable): New function.
(execute_fold_all_builtins): Use optimize_unreachable to optimize
BUILT_IN_UNREACHABLE. Don't optimize after BUILT_IN_UNREACHABLE.
* gcc.dg/builtin-unreachable-6.c: New test.
* gcc.dg/builtin-unreachable-5.c: New test.
Index: gcc/tree-ssa-ccp.c
===================================================================
--- gcc/tree-ssa-ccp.c (revision 189007)
+++ gcc/tree-ssa-ccp.c (working copy)
@@ -2318,6 +2318,69 @@ optimize_stdarg_builtin (gimple call)
}
}
+/* Attemp to make the block of __builtin_unreachable I unreachable by changing
+ the incoming jumps. Return true if at least one jump was changed. */
+
+static bool
+optimize_unreachable (gimple_stmt_iterator i)
+{
+ basic_block bb = gsi_bb (i);
+ gimple_stmt_iterator gsi;
+ gimple stmt;
+ edge_iterator ei;
+ edge e;
+ bool ret;
+
+ for (gsi = gsi_start_bb (bb); !gsi_end_p (gsi); gsi_next (&gsi))
+ {
+ stmt = gsi_stmt (gsi);
+
+ if (is_gimple_debug (stmt))
+ continue;
+
+ if (gimple_code (stmt) == GIMPLE_LABEL)
+ {
+ /* Verify we do not need to preserve the label. */
+ if (FORCED_LABEL (gimple_label_label (stmt)))
+ return false;
+
+ continue;
+ }
+
+ /* Only handle the case that __builtin_unreachable is the first statement
+ in the block. We rely on DCE to remove stmts without side-effects
+ before __builtin_unreachable. */
+ if (gsi_stmt (gsi) != gsi_stmt (i))
+ return false;
+ }
+
+ ret = false;
+ FOR_EACH_EDGE (e, ei, bb->preds)
+ {
+ gsi = gsi_last_bb (e->src);
+ stmt = gsi_stmt (gsi);
+
+ if (stmt && gimple_code (stmt) == GIMPLE_COND)
+ {
+ if (e->flags & EDGE_TRUE_VALUE)
+ gimple_cond_make_false (stmt);
+ else if (e->flags & EDGE_FALSE_VALUE)
+ gimple_cond_make_true (stmt);
+ else
+ gcc_unreachable ();
+ }
+ else
+ {
+ /* Todo: handle other cases, f.i. switch statement. */
+ continue;
+ }
+
+ ret = true;
+ }
+
+ return ret;
+}
+
/* A simple pass that attempts to fold all builtin functions. This pass
is run after we've propagated as many constants as we can. */
@@ -2379,6 +2442,11 @@ execute_fold_all_builtins (void)
gsi_next (&i);
continue;
+ case BUILT_IN_UNREACHABLE:
+ if (optimize_unreachable (i))
+ cfg_changed = true;
+ break;
+
case BUILT_IN_VA_START:
case BUILT_IN_VA_END:
case BUILT_IN_VA_COPY:
@@ -2393,6 +2461,9 @@ execute_fold_all_builtins (void)
continue;
}
+ if (result == NULL_TREE)
+ break;
+
if (dump_file && (dump_flags & TDF_DETAILS))
{
fprintf (dump_file, "Simplified\n ");
Index: gcc/testsuite/gcc.dg/builtin-unreachable-6.c
===================================================================
--- /dev/null (new file)
+++ gcc/testsuite/gcc.dg/builtin-unreachable-6.c (revision 0)
@@ -0,0 +1,21 @@
+/* { dg-do compile } */
+/* { dg-options "-O2 -fdump-tree-fab" } */
+
+void
+foo (int b, int c)
+{
+ void *x = &&lab;
+ if (b)
+ {
+lab:
+ __builtin_unreachable ();
+ }
+lab2:
+ if (c)
+ x = &&lab2;
+ goto *x;
+}
+
+/* { dg-final { scan-tree-dump-times "lab:" 1 "fab" } } */
+/* { dg-final { scan-tree-dump-times "__builtin_unreachable" 1 "fab" } } */
+/* { dg-final { cleanup-tree-dump "fab" } } */
Index: gcc/testsuite/gcc.dg/builtin-unreachable-5.c
===================================================================
--- /dev/null (new file)
+++ gcc/testsuite/gcc.dg/builtin-unreachable-5.c (revision 0)
@@ -0,0 +1,23 @@
+/* { dg-do compile } */
+/* { dg-options "-O2 -fdump-tree-fab" } */
+
+int
+foo (int a)
+{
+ if (a <= 0)
+ {
+ L1:
+ __builtin_unreachable ();
+ }
+
+ if (a > 2)
+ goto L1;
+
+ return a > 0;
+}
+
+/* { dg-final { scan-tree-dump-times "if \\(" 0 "fab" } } */
+/* { dg-final { scan-tree-dump-times "goto" 0 "fab" } } */
+/* { dg-final { scan-tree-dump-times "L1:" 0 "fab" } } */
+/* { dg-final { scan-tree-dump-times "__builtin_unreachable" 0 "fab" } } */
+/* { dg-final { cleanup-tree-dump "fab" } } */
