Zhongyunde <zhongyu...@huawei.com> writes:
>> -----Original Message-----
>> From: Richard Sandiford [mailto:richard.sandif...@arm.com]
>> Sent: Wednesday, July 22, 2020 12:12 AM
>> To: Zhongyunde <zhongyu...@huawei.com>
>> Cc: gcc-patches@gcc.gnu.org; Yangfei (A) <lancelo.y...@huawei.com>
>> Subject: Re: 答复: [PATCH PR95696] regrename creates overlapping
>> register allocations for vliw
>> 
>> Zhongyunde <zhongyu...@huawei.com> writes:
>> > Thanks for your review.
>> >
>> > First of all, this is an optimization.
>> 
>> OK, good.
>> 
>> >    gcc do sms before reload, and here each insn use pseudo-register.
>> After reload, they are allocated hard-register, then the regrename pass try
>> to adjust the register number with def/use chain created by
>> build_def_use.
>> >  As now gcc doesn't consider the VLIW bundles, so regrename pass may
>> updated a reg which may not really unused, which will bring in invalid
>> VLIW bundles.
>> >    Before the final schedule, we usually recheck the validation of VLIW
>> bundles, and reschedule the conflicted insns into two VLIW to make them
>> validation to avoid above issue, so this is not a correctness issue.
>> >  Certainly, reschedule the conflicted insns into two VLIW will destroy
>> the kernel loop's sms schedule result, and usually it will be harmful to the
>> performance.
>> 
>> Yeah.  The reason I was worried about the TI markers being stale is that, in
>> general, register allocation can introduce new spills and reloads, can add
>> and remove instructions, and can convert instructions into different forms
>> (e.g. as a result of register elimination).
>> There are then post-reload optimisers that can change the code further.
>> All these things could invalidate the VLIW bundling done by the first
>> scheduler.
>> 
>> It sounds like that's not happening in your motivating testcase, and the
>> VLIW bundling is still correct (for this loop) by the time that regrename
>> runs.  Is that right?
>
> Yes, it is right.
>
>> 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?

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

Reply via email to