Re: [PATCH, rtl] Fix PR84878: Segmentation fault in add_cross_iteration_register_deps

["H.J. Lu" <hjl dot tools at gmail dot com>]

On Tue, Apr 3, 2018 at 11:36 AM, Peter Bergner <bergner@vnet.ibm.com> wrote:
> On 4/2/18 9:21 AM, Alexander Monakov wrote:
>> On Tue, 27 Mar 2018, Richard Biener wrote:
>>> If they only appear in the exit/entry block ignoring them should be safe.
>>>
>>> But who knows...
>>
>> Roman and I discussed a related problem a few weeks ago, so here's my 2c.
>> As I don't have any special DF knowledge, this is merely my understanding.
>>
>> (apropos i: SMS uses sched-deps for intra-loop deps, and then separately uses
>> DF for cross-iteration deps, which means that it should be ready for surprises
>> when the two scanners are not 100% in sync)
>>
>> (apropos ii: given the flexibility of RTL, it would have been really nice
>> if there were no implicit cc0-like uses that need to be special-cased in DF,
>> sched-deps and other scanners)
>>
>> In this case I believe it's fine to skip processing of r_use when the associated
>> BB is not the loop BB (i.e. 'if (DF_REF_BB (r_use->ref) != g->bb)' as Richard
>> suggested), but I'm concerned that skipping it when the artificial's use BB
>> corresponds to loop BB goes from ICE to wrong-code. It should be detected
>> earlier, in sms_schedule (see the comment starting with "Don't handle BBs with
>> calls or barriers").
>
> Ok, getting back to this, I see the r_use->ref for the global reg that
> is exposed all the way to the exit block is marked as artificial, so
> DF_REF_IS_ARTIFICIAL (r_use->ref) is true.  So using richi and Alexander's
> suggestion of DF_REF_BB(), I cobbled the change below.  I added an assert
> to catch any cases where the artificial use is the same as g->bb or where
> we still might have no insn info.  I never hit the assert in either my
> bootstrap or in my testsuite runs.  Is something like the following what
> we want?
>
> This did pass bootstrap and regtesting with no regressions on powerpc64-linux
> running the testsuite in both 32-bit and 64-bit modes.
>
> Peter
>
>
> gcc/
>         PR rtl-optimization/84878
>         * ddg.c (add_cross_iteration_register_deps): Use DF_REF_BB to determine
>         the basic block.  Assert the use reference is not artificial and that
>         it has an associated insn.
>
> gcc/testsuite/
>         PR rtl-optimization/84878
>         * gcc.dg/pr84878.c: New test.

Wrong test filename.

>
> Index: gcc/ddg.c
> ===================================================================
> --- gcc/ddg.c   (revision 258802)
> +++ gcc/ddg.c   (working copy)
> @@ -295,11 +295,14 @@ add_cross_iteration_register_deps (ddg_p
>    /* Create inter-loop true dependences and anti dependences.  */
>    for (r_use = DF_REF_CHAIN (last_def); r_use != NULL; r_use = r_use->next)
>      {
> -      rtx_insn *use_insn = DF_REF_INSN (r_use->ref);
> -
> -      if (BLOCK_FOR_INSN (use_insn) != g->bb)
> +      if (DF_REF_BB (r_use->ref) != g->bb)
>         continue;
>
> +      gcc_assert (!DF_REF_IS_ARTIFICIAL (r_use->ref)
> +                 && DF_REF_INSN_INFO (r_use->ref) != NULL);
> +
> +      rtx_insn *use_insn = DF_REF_INSN (r_use->ref);
> +
>        /* ??? Do not handle uses with DF_REF_IN_NOTE notes.  */
>        use_node = get_node_of_insn (g, use_insn);
>        gcc_assert (use_node);
> Index: gcc/testsuite/gcc.target/powerpc/pr84878.c
> ===================================================================
> --- gcc/testsuite/gcc.target/powerpc/pr84878.c  (revision 0)
> +++ gcc/testsuite/gcc.target/powerpc/pr84878.c  (working copy)
> @@ -0,0 +1,18 @@
> +/* PR target/84878 */
> +/* { dg-do compile } */
> +/* { dg-require-effective-target powerpc_altivec_ok } */
> +/* { dg-options "-O2 -maltivec -mno-vsx -fmodulo-sched -ftree-vectorize -funroll-loops -fassociative-math -fno-signed-zeros -fno-trapping-math" } */
> +
> +int ek;
> +float zu;
> +
> +int
> +k5 (int ks)
> +{
> +  while (ek < 1)
> +    {
> +      ks += (int)(0x1000000 + zu + !ek);
> +      ++ek;
> +    }
> +  return ks;
> +}
>



-- 
H.J.