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