[PATCH] D87157: [GlobalISel] Add a localizer for copies from physregs and use it in AArch64

[Jessica Paquette via Phabricator via llvm-commits]

paquette added a comment.

In D87157#2257366 <https://reviews.llvm.org/D87157#2257366>, @qcolombet wrote:

> Hi Jessica,
>
> This feels to me that we are adding another level of heuristics that could well degrade the performances on other cases. The fact that this runs only on functions with only one basic block and with less than 25 instructions tells me this is really a workaround against a more general problem.
> Put differently, I don't see that this is generally useful to be worth the compile time.

I don't disagree; I'm not particularly attached to or happy with this approach. :)

> Solving that properly IMHO would require to model batch of copies as parallel copies (all the copies happen in parallel in a bundle) and we would serialize them only after register allocation. We already do something like that in SplitKit IIRC.

I think I prefer this.

> I am guessing your concern is that we regress some cases compared to SDISel (I would also expect that we improve some as well!), thus I wondering if we should try to "fix" this at all.

I found a surprising number of places where this happens. It's extremely noticeable in small functions containing tail calls.

These are some of the cases I found:

https://godbolt.org/z/xakndt
https://godbolt.org/z/pnK5xQ
https://godbolt.org/z/ziYBUs
https://godbolt.org/z/fwmRvE
https://godbolt.org/z/w5uPHH
https://godbolt.org/z/xJVMgp
https://godbolt.org/z/ZDXn-_
https://godbolt.org/z/fcDWGT
https://godbolt.org/z/-trkgD
https://godbolt.org/z/faLS0z
https://godbolt.org/z/xUdnUq

I suspect there are more cases like this. These are just the ones I found by manually clicking through code size regressions I found with a script.

> If we need to fix this, could we instead issue the arguments in the same order as SDISel in the meantime? (While we bring up the proper support for parallel copies.)

I tried doing that, and it made some cases better, and some cases worse. (Like you mentioned, we improve some things!) It didn't seem like there was a strong improvement or regression, so I don't think it's worth doing.

If there's a proper way to model what we're missing here (e.g. parallel copies), I think it would be best to just wait for that.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D87157/new/

https://reviews.llvm.org/D87157