> -----Original Message----- > From: Richard Sandiford [mailto:richard.sandif...@arm.com] > Sent: Tuesday, July 28, 2020 1:33 AM > To: Zhongyunde <zhongyu...@huawei.com> > Cc: gcc-patches@gcc.gnu.org; Yangfei (Felix) <felix.y...@huawei.com> > Subject: Re: [PATCH PR95696] regrename creates overlapping register > allocations for vliw > > Zhongyunde <zhongyu...@huawei.com> writes: > > I reconsider the issue and update patch attached. > > Yes, If the kernel loop bb's information doesn't use in regrename, it > > also need not be collected to save compile time. > > Thanks. The point about other callers was a good one though. > I think it would be OK to add a new parameter to regrename_analyze that > says whether it should process all blocks, ignoring BB_DISABLE_SCHEDULE. > The default value could be true, with only regrename itself passing false.
Thanks for your detail advice, and I add a new parameter to regrename_analyze.
> Why do you need the BB_MODIFIED test? The point in time that that flag
> is measured from is somewhat arbitrary. Also, the modification that
> caused the flag to be set might not have invalidated the schedule.
Yes, in fact the BB_MODIFIED test is not need.
> Richard
>
> >
> >> -----Original Message-----
> >> From: Zhongyunde
> >> Sent: Sunday, July 26, 2020 3:29 PM
> >> To: 'Richard Sandiford' <richard.sandif...@arm.com>
> >> Cc: gcc-patches@gcc.gnu.org; Yangfei (Felix) <felix.y...@huawei.com>
> >> Subject: RE: [PATCH PR95696] regrename creates overlapping register
> >> allocations for vliw
> >>
> >>
> >> > >> It's interesting that this is for a testcase using SMS. One of
> >> > >> the traditional problems with the GCC implementation of SMS has
> >> > >> been ensuring that later passes don't mess up the scheduled
> >> > >> loop. So in your testcase, does register allocation succeed for
> >> > >> the SMS loop without invalidating the bundling decisions?
> >> > >
> >> > > Yes.
> >> > >
> >> > >> If so, then it's probably better to avoid running regrename on it at
> all.
> >> > >> It mostly exists to help the second scheduling pass, but the
> >> > >> second scheduling pass shouldn't be messing with an SMS loop
> anyway.
> >> > >> Also, although the patch deals with one case in which regrename
> >> > >> could disrupt the bundling, there are others too.
> >> > >>
> >> > >> So maybe one option would be to make regrename ignore blocks
> >> > >> that have BB_DISABLE_SCHEDULE set. (Sorry if that's been
> >> > >> discussed and discounted
> >> > >> already.)
> >> > >
> >> > > ok, according your advice, I make a new patch attached.
> >> >
> >> > Thanks. I think we should treat the SMS and the REG_UNUSED stuff
> >> > as separate patches though.
> >> >
> >> > For the SMS part, I think a better place to enforce the rule is in
> >> > build_def_use. If that function returns false early for
> >> > BB_DISABLE_SCHEDULE, we will avoid disrupting the schedule for the
> >> > block without wasting too much compile time on it, and we'll still
> >> > keep the pass structures internally correct. (It would also be
> >> > good to have a dump_file message to say that that's what we're
> >> > doing.)
> >>
> >> > Do you still need the REG_UNUSED stuff with the SMS patch? If so,
> >> > could you describe the (presumably non-SMS) cases that are
> affected?
> >>
> >> Yes, the non-SMS basic block should not be affected.
> >> An alternate method attached can avoid use REG_UNUSED stuff for BB
> >> with BB_DISABLE_SCHEDUL.
> >>
> >> I don't change build_def_use to return false early as I find some
> >> other optimization reuse the function regrename_analyze to creat
> >> def/use chain info of the kernel loop body in our target.
> >>
> >> > TBH, given that the bundling information is so uncertain at this
> >> > stage, I think it would be better to have a mode in which regrename
> >> > ignores REG_UNUSED notes altogether. Perhaps we could put it
> under
> >> a
> >> > --param, which targets could then set to whichever default they
> prefer.
> >> > The default should be the current behaviour though.
> >>
> >>
> >> > Thanks,
> >> > Richard
> >
> > diff --git a/gcc/regrename.c b/gcc/regrename.c index
> > c38173a77..1df58e52d 100644
> > --- a/gcc/regrename.c
> > +++ b/gcc/regrename.c
> > @@ -737,6 +737,14 @@ regrename_analyze (bitmap bb_mask)
> > if (dump_file)
> > fprintf (dump_file, "\nprocessing block %d:\n", bb1->index);
> >
> > + if ((bb1->flags & BB_DISABLE_SCHEDULE) != 0
> > + && (bb1->flags & BB_MODIFIED) == 0)
> > + {
> > + if (dump_file)
> > + fprintf (dump_file, "skip to avoid disrupting the sms schedule\n");
> > + continue;
> > + }
> > +
> > init_rename_info (this_info, bb1);
> >
> > success = build_def_use (bb1);
PR95696_5.patch
Description: PR95696_5.patch
