Re: [PATCH] Fix PR rtl-optimization/61278

[Richard Biener <richard dot guenther at gmail dot com>]

On Fri, May 23, 2014 at 12:33 PM, Zhenqiang Chen
<zhenqiang.chen@linaro.org> wrote:
> On 23 May 2014 17:05, Richard Biener <richard.guenther@gmail.com> wrote:
>> On Fri, May 23, 2014 at 9:23 AM, Zhenqiang Chen
>> <zhenqiang.chen@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.chen@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.chen@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;
>>> +}