diff mbox series

[regrename] Fix PR87330 : ICE in scan_rtx_reg, at regrename.c

Message ID CAAdirjzj2+Zg2frrZJG8gd4O4urZRWFy9kqJ90CPA8inpq0hbQ@mail.gmail.com
State New
Headers show
Series [regrename] Fix PR87330 : ICE in scan_rtx_reg, at regrename.c | expand

Commit Message

Sameera Deshpande Oct. 8, 2018, 10:23 p.m. UTC 
Hi!

Please find attached the patch fixing the issue PR87330 : ICE in
scan_rtx_reg, at regrename.c:1097.
The regrename pass does not rename the registers which are in notes,
because of which the REG_DEAD note had previous register names, which
caused conflicting liveness information generated for tag collision
pass.

It is better to do it in regrename_do_replace instead while
regrename_analyze, because the note information does not really
contribute into the regrename analysis, hence need not be added in the
def-use chains that are computed. regrename_do_replace is where the
decision to finally rename the register is made - where the note can
be altered with new regname.

Other notes need not be changed, as they don't hold renamed register
information.

Ok for trunk?

Changelog:

2018-10-09 Sameera Deshpande <sameera.deshpande@linaro.org

* gcc/regrename.c (regrename_do_replace): Add condition to alter
regname if note has same register marked dead in notes.

-- 
- Thanks and regards,
  Sameera D.

Comments

Eric Botcazou Oct. 8, 2018, 10:38 p.m. UTC | #1 
> Other notes need not be changed, as they don't hold renamed register

> information.

> 

> Ok for trunk?


No, REG_DEAD & REG_UNUSED note must be recomputed by passes consuming them.

> 2018-10-09 Sameera Deshpande <sameera.deshpande@linaro.org

> 

> * gcc/regrename.c (regrename_do_replace): Add condition to alter

> regname if note has same register marked dead in notes.


No gcc/ prefix in gcc/ChangeLog.

-- 
Eric Botcazou
Sameera Deshpande Oct. 30, 2018, 10:09 a.m. UTC | #2 
On Tue, 9 Oct 2018 at 04:08, Eric Botcazou <ebotcazou@adacore.com> wrote:
>

> > Other notes need not be changed, as they don't hold renamed register

> > information.

> >

> > Ok for trunk?

>

> No, REG_DEAD & REG_UNUSED note must be recomputed by passes consuming them.

>

> > 2018-10-09 Sameera Deshpande <sameera.deshpande@linaro.org

> >

> > * gcc/regrename.c (regrename_do_replace): Add condition to alter

> > regname if note has same register marked dead in notes.

>

> No gcc/ prefix in gcc/ChangeLog.

>

> --

> Eric Botcazou


Hi Eric,

Thanks for your comments.

Please find attached updated patch invoking data flow for updating the
REG_DEAD and REG_UNUSED notes.

As this change is made in falkor specific file, adding James and
Richard for review.

Ok for trunk?

Changelog:

2018-10-30 Sameera Deshpande <sameera.deshpande@linaro.org

* gcc/config/aarch64/falkor-tag-collision-avoidance.c
(execute_tag_collision_avoidance): Invoke df_note_add_problem to
recompute REG_DEAD and REG_UNUSED notes before analysis.
-- 
- Thanks and regards,
  Sameera D.
diff --git a/gcc/config/aarch64/falkor-tag-collision-avoidance.c b/gcc/config/aarch64/falkor-tag-collision-avoidance.c
index fb6568f..4ca9d66 100644
--- a/gcc/config/aarch64/falkor-tag-collision-avoidance.c
+++ b/gcc/config/aarch64/falkor-tag-collision-avoidance.c
@@ -805,6 +805,7 @@ execute_tag_collision_avoidance ()
   df_set_flags (DF_RD_PRUNE_DEAD_DEFS);
   df_chain_add_problem (DF_UD_CHAIN);
   df_compute_regs_ever_live (true);
+  df_note_add_problem ();
   df_analyze ();
   df_set_flags (DF_DEFER_INSN_RESCAN);
Richard Earnshaw (lists) Oct. 30, 2018, 10:45 a.m. UTC | #3 
On 30/10/2018 10:09, Sameera Deshpande wrote:
> On Tue, 9 Oct 2018 at 04:08, Eric Botcazou <ebotcazou@adacore.com> wrote:

>>

>>> Other notes need not be changed, as they don't hold renamed register

>>> information.

>>>

>>> Ok for trunk?

>>

>> No, REG_DEAD & REG_UNUSED note must be recomputed by passes consuming them.

>>

>>> 2018-10-09 Sameera Deshpande <sameera.deshpande@linaro.org

>>>

>>> * gcc/regrename.c (regrename_do_replace): Add condition to alter

>>> regname if note has same register marked dead in notes.

>>

>> No gcc/ prefix in gcc/ChangeLog.

>>

>> --

>> Eric Botcazou

> 

> Hi Eric,

> 

> Thanks for your comments.

> 

> Please find attached updated patch invoking data flow for updating the

> REG_DEAD and REG_UNUSED notes.

> 

> As this change is made in falkor specific file, adding James and

> Richard for review.

> 

> Ok for trunk?

> 

> Changelog:

> 

> 2018-10-30 Sameera Deshpande <sameera.deshpande@linaro.org

> 

> * gcc/config/aarch64/falkor-tag-collision-avoidance.c

> (execute_tag_collision_avoidance): Invoke df_note_add_problem to

> recompute REG_DEAD and REG_UNUSED notes before analysis.

> 


'Call df_note_add_problem.' is enough.

OK with that change.

R.

> 

> bug87330.patch

> 

> diff --git a/gcc/config/aarch64/falkor-tag-collision-avoidance.c b/gcc/config/aarch64/falkor-tag-collision-avoidance.c

> index fb6568f..4ca9d66 100644

> --- a/gcc/config/aarch64/falkor-tag-collision-avoidance.c

> +++ b/gcc/config/aarch64/falkor-tag-collision-avoidance.c

> @@ -805,6 +805,7 @@ execute_tag_collision_avoidance ()

>    df_set_flags (DF_RD_PRUNE_DEAD_DEFS);

>    df_chain_add_problem (DF_UD_CHAIN);

>    df_compute_regs_ever_live (true);

> +  df_note_add_problem ();

>    df_analyze ();

>    df_set_flags (DF_DEFER_INSN_RESCAN);

>  

>
Sameera Deshpande Oct. 30, 2018, 11 a.m. UTC | #4 
On Tue, 30 Oct 2018 at 16:16, Richard Earnshaw (lists)
<Richard.Earnshaw@arm.com> wrote:
>

> On 30/10/2018 10:09, Sameera Deshpande wrote:

> > On Tue, 9 Oct 2018 at 04:08, Eric Botcazou <ebotcazou@adacore.com> wrote:

> >>

> >>> Other notes need not be changed, as they don't hold renamed register

> >>> information.

> >>>

> >>> Ok for trunk?

> >>

> >> No, REG_DEAD & REG_UNUSED note must be recomputed by passes consuming them.

> >>

> >>> 2018-10-09 Sameera Deshpande <sameera.deshpande@linaro.org

> >>>

> >>> * gcc/regrename.c (regrename_do_replace): Add condition to alter

> >>> regname if note has same register marked dead in notes.

> >>

> >> No gcc/ prefix in gcc/ChangeLog.

> >>

> >> --

> >> Eric Botcazou

> >

> > Hi Eric,

> >

> > Thanks for your comments.

> >

> > Please find attached updated patch invoking data flow for updating the

> > REG_DEAD and REG_UNUSED notes.

> >

> > As this change is made in falkor specific file, adding James and

> > Richard for review.

> >

> > Ok for trunk?

> >

> > Changelog:

> >

> > 2018-10-30 Sameera Deshpande <sameera.deshpande@linaro.org

> >

> > * gcc/config/aarch64/falkor-tag-collision-avoidance.c

> > (execute_tag_collision_avoidance): Invoke df_note_add_problem to

> > recompute REG_DEAD and REG_UNUSED notes before analysis.

> >

>

> 'Call df_note_add_problem.' is enough.

>

> OK with that change.

>

> R.

>

> >

> > bug87330.patch

> >

> > diff --git a/gcc/config/aarch64/falkor-tag-collision-avoidance.c b/gcc/config/aarch64/falkor-tag-collision-avoidance.c

> > index fb6568f..4ca9d66 100644

> > --- a/gcc/config/aarch64/falkor-tag-collision-avoidance.c

> > +++ b/gcc/config/aarch64/falkor-tag-collision-avoidance.c

> > @@ -805,6 +805,7 @@ execute_tag_collision_avoidance ()

> >    df_set_flags (DF_RD_PRUNE_DEAD_DEFS);

> >    df_chain_add_problem (DF_UD_CHAIN);

> >    df_compute_regs_ever_live (true);

> > +  df_note_add_problem ();

> >    df_analyze ();

> >    df_set_flags (DF_DEFER_INSN_RESCAN);

> >

> >

>

Thanks Richard! Patch committed at revision 265618.

-- 
- Thanks and regards,
  Sameera D.
diff mbox series

Patch

diff --git a/gcc/regrename.c b/gcc/regrename.c
index 8424093..a3446a2 100644
--- a/gcc/regrename.c
+++ b/gcc/regrename.c
@@ -970,6 +970,7 @@  regrename_do_replace (struct du_head *head, int reg)
       unsigned int regno = ORIGINAL_REGNO (*chain->loc);
       struct reg_attrs *attr = REG_ATTRS (*chain->loc);
       int reg_ptr = REG_POINTER (*chain->loc);
+      rtx note;
 
       if (DEBUG_INSN_P (chain->insn) && REGNO (*chain->loc) != base_regno)
 	validate_change (chain->insn, &(INSN_VAR_LOCATION_LOC (chain->insn)),
@@ -986,6 +987,11 @@  regrename_do_replace (struct du_head *head, int reg)
 	      last_reg = *chain->loc;
 	    }
 	  validate_change (chain->insn, chain->loc, last_repl, true);
+	  note = find_regno_note (chain->insn, REG_DEAD, base_regno);
+	  if (note != 0)
+	    {
+	      validate_change (chain->insn, &XEXP (note, 0), last_repl, true);
+	    }
 	}
     }