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.

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.
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.
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.
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);
+	    }
 	}
     }