On Fri, Jul 6, 2012 at 6:36 PM, Tom de Vries <tom_devr...@mentor.com> wrote:
> On 06/07/12 13:01, Richard Guenther wrote:
>> On Thu, Jul 5, 2012 at 8:45 PM, Tom de Vries <tom_devr...@mentor.com> 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?
Ok.
Thanks,
Richard.
> Thanks,
> - Tom
>
>
> 2012-07-06 Tom de Vries <t...@codesourcery.com>
> Richard Guenther <rguent...@suse.de>
>
> * 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.
