On Mon, 2020-08-10 at 11:11 -0500, Peter Bergner wrote: > On 7/23/20 3:29 PM, Jeff Law wrote: > > > > What's driving this change? > > > > > > Peter noticed IRA allocates stuff to volatile registers while it is life > > > through a call, and then LRA has to correct that, not optimal. > > I can't recall if IRA or LRA handles the need to save them to the stack, > > but the > > whole point is you can do much better sometimes by saving into the stack > > with the > > caller-saves algorithm vs just giving up and spilling. > > > > > > IRA will do a cost/benefit analysis to see using call clobbered > > > > registers like > > > > this is profitable or not. You're just turning the whole thing off. > > Sorry for taking so long to reply. I'm the guilty party for asking Pat > to submit the patch. :-) I was not aware IRA did that and just assumed > it was a bug. For now, consider the patch withdrawn. That said, I > will have to look at that cost computation, especially since when I > last looked, IRA does not count potential prologue/epilogue save/restore > insns if it were to assign a non-volatile register when computing reg > costs. That would seem to be an important consideration here. No worries. Yea, you want to count the prologue/epilogue, as well as the saves/restores at the call points (which need frequency scaling and accounting for saves which don't need to happen because a prior save is sufficient to cover more than one call), etc.
I think much of this code is still in caller-save.c. It's been eons since I worked on it, but I can probably get reacquainted with the implementation if you have questions > I'll note this all came about when I was looking at PR96017, which is > due to not shrink-wrapping a pseudo. That was due to it being live > across a call. I first I thought (for the 2nd test case, not the original > one) split_live_ranges_for_shrink_wrap() was nicely splitting the pseudo > for us, but it must have been the caller-saves algorithm you mention above. > However, that code doesn't handle the original test case, which I think > it should. > > My thought for that bug was to introduce some extra splitting before RA > (maybe as part of split_live_ranges_for_shrink_wrap?) where we split > pseudos that are live across a call, but that have at least one path > to the exit that doesn't cross a call. However, if we can beef up > the caller-saves cost computation, maybe that would work too? I've gone back and forth on pre allocation splitting as well as post-allocating splitting and re-allocation. I could argue either side of that discussion -- given we've got a bit of special code for splitting to help shrink wrapping, maybe that's the best place if we need to do splitting before RA since this was triggered by digging into a shrink-wrapping problem. I'd probably start by making sure the cost computation is sane though. Jeff
