On Thu, 2023-10-05 at 15:33 -0400, Vladimir Makarov wrote:
> EXTERNAL EMAIL: Do not click links or open attachments unless you know the
> content is safe
>
> On 9/7/23 07:21, senthilkumar.selva...@microchip.com wrote:
> > Hi,
> >
> > One more execution failure for the avr target, this time from
> > gcc.c-torture/execute/bitfld-3.c.
> >
> > Steps to reproduce
> >
> > Enable LRA in avr.cc by removing TARGET_LRA_P hook, build with
> >
> > $ make all-host && make install-host
> >
> > and then
> >
> > $ avr-gcc gcc/testsuite/gcc.c-torture/execute/bitfld-3.c -S -Os -mmcu=avr51
> > -fdump-rtl-all
> >
> > When lra_update_fp2sp_elimination runs and pseudos assigned to the
> > FP have to be spilled to stack slots, they sometimes end up in a
> > slot that already has a reg with an overlapping live range. This is
> > because lra_reg_info[regno].live_ranges is NULL for such spilled
> > pseudos, and therefore when assign_stack_slot_num_and_sort_pseduos
> > checks if lra_intersected_live_ranges_p, it always returns false.
> >
> > In the below reload dump, all the pseudos assigned to FP get
> > allocated to slot 0. The live ranges for some of them (r1241 for
> > e.g.) conflicts with r603 that was originally assigned to slot 0,
> > but they still end up in the same slot, causing the execution failure.
> >
> Sorry for the delay with the answer, Senthil. Avr is unusual target and
> needs some changes in LRA but the changes improves LRA portability. So
> thank you for your work on porting LRA to AVR.
>
> The patch is ok for me. The only comment is that making calculation of
> the set only once would be nice. Live range calculation in LRA can take
> a lot of time, code of update_pseudo_point is hot and the worst the set
> will be really used rarely but it is calculated every time.
>
> You can commit the current patch and I'll do it by myself or, if you
> want, you can modify the patch by yourself and submit it for review and
> I'll review as soon as possible. Either way works for me.
Apologies for the extreme delay in responding - had to sort out some medical
issues.
Is it ok if I commit the patch now? I have one more patch in ira.cc, after
which I'm hoping the regression results would be good enough to switch the
avr target to LRA.
Regards
Senthil
>
> > diff --git a/gcc/lra-lives.cc b/gcc/lra-lives.cc
> > index f60e564da82..e4289f13979 100644
> > --- a/gcc/lra-lives.cc
> > +++ b/gcc/lra-lives.cc
> > @@ -250,7 +250,17 @@ update_pseudo_point (int regno, int point, enum
> > point_type type)
> > if (HARD_REGISTER_NUM_P (regno))
> > return;
> >
> > - if (complete_info_p || lra_get_regno_hard_regno (regno) < 0)
> > + /* Pseudos assigned to the FP register could potentially get spilled
> > + to stack slots when lra_update_fp2sp_elimination runs, so keep
> > + their live range info up to date, even if they aren't in memory
> > + right now. */
> > + int hard_regno = lra_get_regno_hard_regno (regno);
> > + HARD_REG_SET set;
> > + CLEAR_HARD_REG_SET(set);
> > + add_to_hard_reg_set (&set, Pmode, HARD_FRAME_POINTER_REGNUM);
> > +
> > + if (complete_info_p || hard_regno < 0
> > + || overlaps_hard_reg_set_p (set, PSEUDO_REGNO_MODE (regno),
> > hard_regno))
> > {
> > if (type == DEF_POINT)
> > {
> >
> >
