RE: [PATCH PR95696] regrename creates overlapping register allocations for vliw

[Zhongyunde <zhongyunde@huawei.com>]

[-- Attachment #1: Type: text/plain, Size: 2338 bytes --]


> >> 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

[-- Attachment #2: PR95696_2.patch --]
[-- Type: application/octet-stream, Size: 777 bytes --]

diff --git a/gcc/regrename.c b/gcc/regrename.c
index c38173a77..2683629db 100644
--- a/gcc/regrename.c
+++ b/gcc/regrename.c
@@ -475,6 +475,7 @@ rename_chains (void)
       int n_uses;
       HARD_REG_SET this_unavailable;
       int reg = this_head->regno;
+      basic_block bb;
 
       if (this_head->cannot_rename)
 	continue;
@@ -493,6 +494,15 @@ rename_chains (void)
       if (n_uses < 2)
 	continue;
 
+      bb = BLOCK_FOR_INSN (this_head->first->insn);
+      if ((bb->flags & BB_DISABLE_SCHEDULE) != 0
+	  && (bb->flags & BB_MODIFIED) == 0)
+	{
+	  if (dump_file)
+	    fprintf (dump_file, "Skip to avoid disrupting the sms schedule\n");
+	  continue;
+	}
+
       best_new_reg = find_rename_reg (this_head, super_class,
 				      &this_unavailable, reg, true);