[PATCH] D75086: [AArch64][GlobalISel] Fixup <32b heterogeneous regbanks of G_PHIs just before selection.

Amara Emerson via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Thu Feb 27 10:31:05 PST 2020


aemerson added a comment.

In D75086#1894721 <https://reviews.llvm.org/D75086#1894721>, @qcolombet wrote:

> I understand that cross copies are bad, but what I don't understand is why don't we form an extended load? That would apply nicely to your example.


I see what you mean. Essentially we would take the code size/perf hit with the specific MIR example I used, but instead try harder to form the extending load ops and ensure those are assigned to gpr banks. There might be other issues with G_EXTRACT but we'll see.

Since this patch fixes a compiler crash I'm going to leave it in, but I'll investigate this approach as a follow up.

> Put differently, (I am being pedantic just for the sake of the argument), I would argue that:
>  `s16 = G_LOAD`
>  maps to
>  `fpr16 = LDR`
> 
> not
>  `gpr32 = LDR16`
>  since we're effectively sneaking in an extension in that load.
> 
> `gpr32 = LDR16` would better match `ANYEXT(G_LOAD)`.
> 
> All in all I understand your solution and why you're doing that. I am afraid we are actually building technical debt here, but I trust your judgement on what you think is best to move forward here.
> 
> To go back on RBS, we could "fix" it to issue only PHIs with the same bank, but effectively, we would just move the copies around and basically still have the weirdly sized cross copies [1] appearing as COPY instead of PHIs and in different blocks.
> 
> Like you noted with PHIs in loops, ideally we would like to have RBS to make its decision globally. That would indeed require some work and also, I believe that would only make the weird sized copies less likely to happen but not impossible. Therefore I really think this is a legalization issue.
> 
> [1] they are only weirdly sized after selection which is why I think we are actually not modeling this right :)




Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D75086





More information about the llvm-commits mailing list