[rtl] Fix PR84878: Segmentation fault in add_cross_iteration_register_deps

Message ID 3fed01b9-845b-3de0-7603-1dcd4080af70@vnet.ibm.com
State New
Headers show
Series
  • [rtl] Fix PR84878: Segmentation fault in add_cross_iteration_register_deps
Related show

Commit Message

Peter Bergner March 27, 2018, 3:05 a.m.
PR84878 shows an example where we segv while creating data dependence edges
for SMS.

ddg.c:add_cross_iteration_register_deps():

  /* 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);
                           ^^^^ segv's

We currently have:
(gdb) pr def_insn
(insn 331 321 332 12 (parallel [
            (set (reg:V4SI 239 [ vect__4.11 ])
                (unspec:V4SI [
                        (reg:V4SF 134 [ vect_cst__39 ])
                        (const_int 0 [0])
                    ] UNSPEC_VCTSXS))
            (set (reg:SI 110 vscr)
                (unspec:SI [
                        (const_int 0 [0])
                    ] UNSPEC_SET_VSCR))
        ]) "bug.i":9 1812 {altivec_vctsxs}
     (expr_list:REG_UNUSED (reg:V4SI 239 [ vect__4.11 ])
        (nil)))
(gdb) p DF_REF_REGNO (last_def)
$4 = 110

So we're looking at the definition of the VSCR hard register, which is a
global register (ie, global_regs[110] == 1), but there are no following
explicit uses of the VSCR reg, so:

(gdb) p DF_REF_INSN_INFO(r_use->ref)
$5 = (df_insn_info *) 0x0

DF_REF_INSN(r_use->ref) deferences DF_REF_INSN_INFO(r_use->ref), so we segv.

The following patch fixes the problems by simply skiping over the "uses"
that do not have insn info (ie, no explicit uses or artificial ones).

This passed bootstrap and regtesting with no regressions on powerpc64-linux.
Ok for trunk?

Peter


gcc/
	PR rtl-optimization/84878
	* ddg.c (add_cross_iteration_register_deps): Skip over uses that do
	not correspond to explicit register references.

gcc/testsuite/
	PR rtl-optimization/84878
	* gcc.dg/pr84878.c: New test.

Comments

Richard Biener March 27, 2018, 8:18 a.m. | #1
On Mon, 26 Mar 2018, Peter Bergner wrote:

> PR84878 shows an example where we segv while creating data dependence edges

> for SMS.

> 

> ddg.c:add_cross_iteration_register_deps():

> 

>   /* 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);

>                            ^^^^ segv's

> 

> We currently have:

> (gdb) pr def_insn

> (insn 331 321 332 12 (parallel [

>             (set (reg:V4SI 239 [ vect__4.11 ])

>                 (unspec:V4SI [

>                         (reg:V4SF 134 [ vect_cst__39 ])

>                         (const_int 0 [0])

>                     ] UNSPEC_VCTSXS))

>             (set (reg:SI 110 vscr)

>                 (unspec:SI [

>                         (const_int 0 [0])

>                     ] UNSPEC_SET_VSCR))

>         ]) "bug.i":9 1812 {altivec_vctsxs}

>      (expr_list:REG_UNUSED (reg:V4SI 239 [ vect__4.11 ])

>         (nil)))

> (gdb) p DF_REF_REGNO (last_def)

> $4 = 110

> 

> So we're looking at the definition of the VSCR hard register, which is a

> global register (ie, global_regs[110] == 1), but there are no following

> explicit uses of the VSCR reg, so:

> 

> (gdb) p DF_REF_INSN_INFO(r_use->ref)

> $5 = (df_insn_info *) 0x0

> 

> DF_REF_INSN(r_use->ref) deferences DF_REF_INSN_INFO(r_use->ref), so we segv.

> 

> The following patch fixes the problems by simply skiping over the "uses"

> that do not have insn info (ie, no explicit uses or artificial ones).

> 

> This passed bootstrap and regtesting with no regressions on powerpc64-linux.

> Ok for trunk?

> 

> Peter

> 

> 

> gcc/

> 	PR rtl-optimization/84878

> 	* ddg.c (add_cross_iteration_register_deps): Skip over uses that do

> 	not correspond to explicit register references.

> 

> gcc/testsuite/

> 	PR rtl-optimization/84878

> 	* gcc.dg/pr84878.c: New test.

> 

> Index: gcc/ddg.c

> ===================================================================

> --- gcc/ddg.c	(revision 258802)

> +++ gcc/ddg.c	(working copy)

> @@ -295,6 +295,11 @@ 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)

>      {

> +      /* PR84878: Some definitions of global hard registers may not have

> +      any following uses or they may be artificial, so skip them.  */

> +      if (DF_REF_INSN_INFO (r_use->ref) == NULL)

> +	continue;

> +


To me a better check would be DF_REF_IS_ARTIFICIAL (r_use->ref).  But
I'm not sure simply ignoring those will be correct?  In fact
artifical refs do have a basic-block, so

>        rtx_insn *use_insn = DF_REF_INSN (r_use->ref);

>  

>        if (BLOCK_FOR_INSN (use_insn) != g->bb)


should use DF_REF_BB (r_use->ref) instead of indirection through
DF_REF_INSN.  Still use_insn is used later but then if the
artificial ref is in side g->bb we should better give up here?
We don't seem to have use_nodes for these "non-insns".

Somebody with more insight on DF should chime in here and tell
me what those "artificial" refs are about ...  there's

    /* If this flag is set for an artificial use or def, that ref
       logically happens at the top of the block.  If it is not set
       for an artificial use or def, that ref logically happens at the
       bottom of the block.  This is never set for regular refs.  */
    DF_REF_AT_TOP = 1 << 1,

so this is kind-of global regs being live across all BBs?  This sounds
a bit stupid to me, but well ... IMHO those refs should be at
specific insns like calls.

So maybe, with a big fat comment, it is OK to ignore artificial
refs in this loop...

Richard.

> Index: gcc/testsuite/gcc.dg/pr84878.c

> ===================================================================

> --- gcc/testsuite/gcc.dg/pr84878.c	(revision 0)

> +++ gcc/testsuite/gcc.dg/pr84878.c	(working copy)

> @@ -0,0 +1,20 @@

> +/* PR rtl-optimization/84878 */

> +/* { dg-do compile { target { powerpc*-*-* } } } */

> +/* { dg-skip-if "do not override -mcpu" { powerpc*-*-* } { "-mcpu=*" } { "-mcpu=G5" } } */

> +/* { dg-require-effective-target powerpc_altivec_ok } */

> +/* { dg-options "-O2 -mcpu=G5 -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;

> +}

> 

> 


-- 
Richard Biener <rguenther@suse.de>
SUSE LINUX GmbH, GF: Felix Imendoerffer, Jane Smithard, Graham Norton, HRB 21284 (AG Nuernberg)
Peter Bergner March 27, 2018, 1:15 p.m. | #2
On 3/27/18 3:18 AM, Richard Biener wrote:
> On Mon, 26 Mar 2018, Peter Bergner wrote:

>>    /* Create inter-loop true dependences and anti dependences.  */

>>    for (r_use = DF_REF_CHAIN (last_def); r_use != NULL; r_use = r_use->next)

>>      {

>> +      /* PR84878: Some definitions of global hard registers may not have

>> +      any following uses or they may be artificial, so skip them.  */

>> +      if (DF_REF_INSN_INFO (r_use->ref) == NULL)

>> +	continue;

>> +

> 

> To me a better check would be DF_REF_IS_ARTIFICIAL (r_use->ref).  But

> I'm not sure simply ignoring those will be correct?


I see now I made a massive mistake in nomenclature in calling these
"artificial" uses.  :-(  What I meant was the forcing of liveness
for global registers at the exit block similar to what you mentioned
in your reply.  Sorry about that.



> In fact artifical refs do have a basic-block, so

>

>>        rtx_insn *use_insn = DF_REF_INSN (r_use->ref);

>>  

>>        if (BLOCK_FOR_INSN (use_insn) != g->bb)

> 

> should use DF_REF_BB (r_use->ref) instead of indirection through

> DF_REF_INSN.  Still use_insn is used later but then if the

> artificial ref is in side g->bb we should better give up here?

> We don't seem to have use_nodes for these "non-insns".


Maybe the problem is that we have a r_use->ref at all for these
non-insns?


> Somebody with more insight on DF should chime in here and tell

> me what those "artificial" refs are about ...  there's

> 

>     /* If this flag is set for an artificial use or def, that ref

>        logically happens at the top of the block.  If it is not set

>        for an artificial use or def, that ref logically happens at the

>        bottom of the block.  This is never set for regular refs.  */

>     DF_REF_AT_TOP = 1 << 1,

> 

> so this is kind-of global regs being live across all BBs?  This sounds

> a bit stupid to me, but well ... IMHO those refs should be at

> specific insns like calls.

> 

> So maybe, with a big fat comment, it is OK to ignore artificial

> refs in this loop...


Yeah, I'd like someone else's opinion too, as I know even less about
real artificial uses (as opposed to my incorrect mention in my first
post). :-)

Peter
Richard Biener March 27, 2018, 1:19 p.m. | #3
On Tue, 27 Mar 2018, Peter Bergner wrote:

> On 3/27/18 3:18 AM, Richard Biener wrote:

> > On Mon, 26 Mar 2018, Peter Bergner wrote:

> >>    /* Create inter-loop true dependences and anti dependences.  */

> >>    for (r_use = DF_REF_CHAIN (last_def); r_use != NULL; r_use = r_use->next)

> >>      {

> >> +      /* PR84878: Some definitions of global hard registers may not have

> >> +      any following uses or they may be artificial, so skip them.  */

> >> +      if (DF_REF_INSN_INFO (r_use->ref) == NULL)

> >> +	continue;

> >> +

> > 

> > To me a better check would be DF_REF_IS_ARTIFICIAL (r_use->ref).  But

> > I'm not sure simply ignoring those will be correct?

> 

> I see now I made a massive mistake in nomenclature in calling these

> "artificial" uses.  :-(  What I meant was the forcing of liveness

> for global registers at the exit block similar to what you mentioned

> in your reply.  Sorry about that.

> 

> 

> 

> > In fact artifical refs do have a basic-block, so

> >

> >>        rtx_insn *use_insn = DF_REF_INSN (r_use->ref);

> >>  

> >>        if (BLOCK_FOR_INSN (use_insn) != g->bb)

> > 

> > should use DF_REF_BB (r_use->ref) instead of indirection through

> > DF_REF_INSN.  Still use_insn is used later but then if the

> > artificial ref is in side g->bb we should better give up here?

> > We don't seem to have use_nodes for these "non-insns".

> 

> Maybe the problem is that we have a r_use->ref at all for these

> non-insns?

> 

> 

> > Somebody with more insight on DF should chime in here and tell

> > me what those "artificial" refs are about ...  there's

> > 

> >     /* If this flag is set for an artificial use or def, that ref

> >        logically happens at the top of the block.  If it is not set

> >        for an artificial use or def, that ref logically happens at the

> >        bottom of the block.  This is never set for regular refs.  */

> >     DF_REF_AT_TOP = 1 << 1,

> > 

> > so this is kind-of global regs being live across all BBs?  This sounds

> > a bit stupid to me, but well ... IMHO those refs should be at

> > specific insns like calls.

> > 

> > So maybe, with a big fat comment, it is OK to ignore artificial

> > refs in this loop...

> 

> Yeah, I'd like someone else's opinion too, as I know even less about

> real artificial uses (as opposed to my incorrect mention in my first

> post). :-)


If they only appear in the exit/entry block ignoring them should be safe.

But who knows...

Richard.
Alexander Monakov April 2, 2018, 2:21 p.m. | #4
On Tue, 27 Mar 2018, Richard Biener wrote:
> > > so this is kind-of global regs being live across all BBs?  This sounds

> > > a bit stupid to me, but well ... IMHO those refs should be at

> > > specific insns like calls.

> > > 

> > > So maybe, with a big fat comment, it is OK to ignore artificial

> > > refs in this loop...

> > 

> > Yeah, I'd like someone else's opinion too, as I know even less about

> > real artificial uses (as opposed to my incorrect mention in my first

> > post). :-)

> 

> 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").

Alexander
Peter Bergner April 3, 2018, 6:36 p.m. | #5
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.

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. Lu April 3, 2018, 6:40 p.m. | #6
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.
Peter Bergner April 3, 2018, 7:05 p.m. | #7
On 4/3/18 1:40 PM, H.J. Lu wrote:
> On Tue, Apr 3, 2018 at 11:36 AM, Peter Bergner <bergner@vnet.ibm.com> wrote:

>> gcc/testsuite/

>>         PR rtl-optimization/84878

>>         * gcc.dg/pr84878.c: New test.

> 

> Wrong test filename.


Ooops, thanks for spotting that!  Will fix.

Peter
Richard Biener April 4, 2018, 7:15 a.m. | #8
On Tue, 3 Apr 2018, Peter Bergner wrote:

> On 4/3/18 1:40 PM, H.J. Lu wrote:

> > On Tue, Apr 3, 2018 at 11:36 AM, Peter Bergner <bergner@vnet.ibm.com> wrote:

> >> gcc/testsuite/

> >>         PR rtl-optimization/84878

> >>         * gcc.dg/pr84878.c: New test.

> > 

> > Wrong test filename.

> 

> Ooops, thanks for spotting that!  Will fix.


Patch is OK.

Thanks,
Richard.
Peter Bergner April 4, 2018, 3:43 p.m. | #9
On 4/4/18 2:15 AM, Richard Biener wrote:
> On Tue, 3 Apr 2018, Peter Bergner wrote:

> 

>> On 4/3/18 1:40 PM, H.J. Lu wrote:

>>> On Tue, Apr 3, 2018 at 11:36 AM, Peter Bergner <bergner@vnet.ibm.com> wrote:

>>>> gcc/testsuite/

>>>>         PR rtl-optimization/84878

>>>>         * gcc.dg/pr84878.c: New test.

>>>

>>> Wrong test filename.

>>

>> Ooops, thanks for spotting that!  Will fix.

> 

> Patch is OK.


Ok, committed to trunk.  Thanks.

Nobody mentioned if this was a regression or not, so I did some testing
and it ICEs on GCC 7 but not on GCC 6.  Is it ok to back port to GCC 7
assuming bootstrap and regtesting are clean?

Peter
Peter Bergner April 4, 2018, 6:25 p.m. | #10
On 4/4/18 10:43 AM, Peter Bergner wrote:
> On 4/4/18 2:15 AM, Richard Biener wrote:

>> On Tue, 3 Apr 2018, Peter Bergner wrote:

>>

>>> On 4/3/18 1:40 PM, H.J. Lu wrote:

>>>> On Tue, Apr 3, 2018 at 11:36 AM, Peter Bergner <bergner@vnet.ibm.com> wrote:

>>>>> gcc/testsuite/

>>>>>         PR rtl-optimization/84878

>>>>>         * gcc.dg/pr84878.c: New test.

>>>>

>>>> Wrong test filename.

>>>

>>> Ooops, thanks for spotting that!  Will fix.

>>

>> Patch is OK.

> 

> Ok, committed to trunk.  Thanks.

> 

> Nobody mentioned if this was a regression or not, so I did some testing

> and it ICEs on GCC 7 but not on GCC 6.  Is it ok to back port to GCC 7

> assuming bootstrap and regtesting are clean?


FYI, the GCC 7 bootstrap and regtesting completed with no regressions.

Peter
Richard Biener April 4, 2018, 7:23 p.m. | #11
On April 4, 2018 8:25:25 PM GMT+02:00, Peter Bergner <bergner@vnet.ibm.com> wrote:
>On 4/4/18 10:43 AM, Peter Bergner wrote:

>> On 4/4/18 2:15 AM, Richard Biener wrote:

>>> On Tue, 3 Apr 2018, Peter Bergner wrote:

>>>

>>>> On 4/3/18 1:40 PM, H.J. Lu wrote:

>>>>> On Tue, Apr 3, 2018 at 11:36 AM, Peter Bergner

><bergner@vnet.ibm.com> wrote:

>>>>>> gcc/testsuite/

>>>>>>         PR rtl-optimization/84878

>>>>>>         * gcc.dg/pr84878.c: New test.

>>>>>

>>>>> Wrong test filename.

>>>>

>>>> Ooops, thanks for spotting that!  Will fix.

>>>

>>> Patch is OK.

>> 

>> Ok, committed to trunk.  Thanks.

>> 

>> Nobody mentioned if this was a regression or not, so I did some

>testing

>> and it ICEs on GCC 7 but not on GCC 6.  Is it ok to back port to GCC

>7

>> assuming bootstrap and regtesting are clean?


Sure. 

>FYI, the GCC 7 bootstrap and regtesting completed with no regressions.

>

>Peter
Peter Bergner April 4, 2018, 9:07 p.m. | #12
On 4/4/18 2:23 PM, Richard Biener wrote:
> On April 4, 2018 8:25:25 PM GMT+02:00, Peter Bergner <bergner@vnet.ibm.com> wrote:

>>> Nobody mentioned if this was a regression or not, so I did some testing

>>> and it ICEs on GCC 7 but not on GCC 6.  Is it ok to back port to GCC 7

>>> assuming bootstrap and regtesting are clean?

>

> Sure.


The backport testing showed no regressions, so I committed it to GCC 7.
Thanks!

Peter

Patch

Index: gcc/ddg.c
===================================================================
--- gcc/ddg.c	(revision 258802)
+++ gcc/ddg.c	(working copy)
@@ -295,6 +295,11 @@  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)
     {
+      /* PR84878: Some definitions of global hard registers may not have
+      any following uses or they may be artificial, so skip them.  */
+      if (DF_REF_INSN_INFO (r_use->ref) == NULL)
+	continue;
+
       rtx_insn *use_insn = DF_REF_INSN (r_use->ref);
 
       if (BLOCK_FOR_INSN (use_insn) != g->bb)
Index: gcc/testsuite/gcc.dg/pr84878.c
===================================================================
--- gcc/testsuite/gcc.dg/pr84878.c	(revision 0)
+++ gcc/testsuite/gcc.dg/pr84878.c	(working copy)
@@ -0,0 +1,20 @@ 
+/* PR rtl-optimization/84878 */
+/* { dg-do compile { target { powerpc*-*-* } } } */
+/* { dg-skip-if "do not override -mcpu" { powerpc*-*-* } { "-mcpu=*" } { "-mcpu=G5" } } */
+/* { dg-require-effective-target powerpc_altivec_ok } */
+/* { dg-options "-O2 -mcpu=G5 -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;
+}