On Fri, May 23, 2014 at 12:33 PM, Zhenqiang Chen
<zhenqiang.c...@linaro.org> wrote:
> On 23 May 2014 17:05, Richard Biener <richard.guent...@gmail.com> wrote:
>> On Fri, May 23, 2014 at 9:23 AM, Zhenqiang Chen
>> <zhenqiang.c...@linaro.org> wrote:
>>> Hi,
>>>
>>> The patch fixes PR rtl-optimization/61278. Root cause for issue is
>>> that df_live does not exist at -O1.
>>>
>>> Bootstrap and no make check regression on X86-64.
>>>
>>> OK for trunk?
>>
>> Why do you need to give up? It seems you can simply avoid
>> marking the block as dirty (though df_get_live_in/out also hands you
>> back DF_LR_IN/OUT if !df_live). So isn't the df_grow_bb_info the
>> "real" fix?
>
> The df_get_live_in of the new basic block will be used to analyse
> later INSNs. If it is not set or incorrect, it will impact on later
> analysis.
> df_grow_bb_info is to make sure the live_in data structure is
> allocated for the new basic block (although I have not found any case
> fail without it). After bitmap_copy(...), we can use it for later
> INSNs.
>
>> Note that df_get_live_in/out are functions tailored to IRA that
>> knows that they handle both df_live and df_lr dependent on
>> optimization level. Is shrink-wrapping supposed to work with
>> both problems as well?
>
> Yes. But it seams not perfect to handle df_lr problem. When I fixed PR
> 57637 (https://gcc.gnu.org/ml/gcc-patches/2013-07/msg00897.html), we
> selected "if DF_LIVE doesn't exist, i.e. at -O1, just give up
> searching NEXT_BLOCK."
Ok, I see. Maybe it would be better to completely disable
shrink-wrapping when LIVE is not available.
Patch is ok.
Thanks,
Richard.
> Thanks!
> -Zhenqiang
>
>>>
>>> ChangeLog:
>>> 2014-05-23 Zhenqiang Chen <zhenqiang.c...@linaro.org>
>>>
>>> PR rtl-optimization/61278
>>> * shrink-wrap.c (move_insn_for_shrink_wrap): Check df_live.
>>>
>>> testsuite/ChangeLog:
>>> 2014-05-23 Zhenqiang Chen <zhenqiang.c...@linaro.org>
>>>
>>> * gcc.dg/lto/pr61278_0.c: New test.
>>> * gcc.dg/lto/pr61278_1.c: New test.
>>>
>>> diff --git a/gcc/shrink-wrap.c b/gcc/shrink-wrap.c
>>> index f09cfe7..be17829 100644
>>> --- a/gcc/shrink-wrap.c
>>> +++ b/gcc/shrink-wrap.c
>>> @@ -204,8 +204,15 @@ move_insn_for_shrink_wrap (basic_block bb, rtx insn,
>>> /* Create a new basic block on the edge. */
>>> if (EDGE_COUNT (next_block->preds) == 2)
>>> {
>>> + /* If DF_LIVE doesn't exist, i.e. at -O1, just give up. */
>>> + if (!df_live)
>>> + return false;
>>> +
>>> next_block = split_edge (live_edge);
>>>
>>> + /* We create a new basic block. Call df_grow_bb_info to make sure
>>> + all data structures are allocated. */
>>> + df_grow_bb_info (df_live);
>>> bitmap_copy (df_get_live_in (next_block), df_get_live_out (bb));
>>> df_set_bb_dirty (next_block);
>>>
>>> diff --git a/gcc/testsuite/gcc.dg/lto/pr61278_0.c
>>> b/gcc/testsuite/gcc.dg/lto/pr61278_0.c
>>> new file mode 100644
>>> index 0000000..03a24ae
>>> --- /dev/null
>>> +++ b/gcc/testsuite/gcc.dg/lto/pr61278_0.c
>>> @@ -0,0 +1,30 @@
>>> +/* { dg-lto-do link } */
>>> +/* { dg-lto-options { { -flto -O0 } } } */
>>> +/* { dg-extra-ld-options " -flto -O1 " } */
>>> +
>>> +static unsigned int
>>> +fn1 (int p1, int p2)
>>> +{
>>> + return 0;
>>> +}
>>> +
>>> +char a, b, c;
>>> +
>>> +char
>>> +foo (char *p)
>>> +{
>>> + int i;
>>> + for (b = 1 ; b > 0; b++)
>>> + {
>>> + for (i = 0; i < 2; i++)
>>> + ;
>>> + for (a = 1; a > 0; a++)
>>> + {
>>> + char d[1] = { 0 };
>>> + if (*p)
>>> + break;
>>> + c ^= fn1 (fn1 (fn1 (0, 0), 0), 0);
>>> + }
>>> + }
>>> + return 0;
>>> +}
>>> diff --git a/gcc/testsuite/gcc.dg/lto/pr61278_1.c
>>> b/gcc/testsuite/gcc.dg/lto/pr61278_1.c
>>> new file mode 100644
>>> index 0000000..b02c8ac
>>> --- /dev/null
>>> +++ b/gcc/testsuite/gcc.dg/lto/pr61278_1.c
>>> @@ -0,0 +1,13 @@
>>> +/* { dg-lto-do link } */
>>> +/* { dg-lto-options { { -flto -O1 } } } */
>>> +
>>> +extern char foo (char *);
>>> +
>>> +char d;
>>> +
>>> +int
>>> +main ()
>>> +{
>>> + foo (&d);
>>> + return 0;
>>> +}
