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

[Amara Emerson via Phabricator via llvm-commits]

aemerson added a comment.

I committed before I saw your message.

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

> Something just stroke me, why don't you use the FPR bank for all 16-bit values then?


Off the top of my head: we would lose the distinction between a gpr and fpr load. For example, the following:

  define i16 @bitwise(i16 *%a, i16 *%b) {
    %res = load i16, i16 *%a
    %x = xor i16 %res, 12345
    store i16 %x, i16 *%b
    ret i16 %x
  }

would end up having RegBank assignments like this:

  body:             |
    bb.1 (%ir-block.0):
      liveins: $x0, $x1
    
      %0:gpr(p0) = COPY $x0
      %1:gpr(p0) = COPY $x1
      %2:fpr(s16) = G_LOAD %0(p0) :: (load 2 from %ir.a)
      %copy:gpr(s16) = COPY %2(s16) ; Need a cross-bank copy here because of the ANYEXT+XOR.
      %6:gpr(s32) = G_ANYEXT %copy(s16)
      %7:gpr(s32) = G_CONSTANT i32 12345
      %8:gpr(s32) = G_XOR %6, %7
      %4:gpr(s16) = G_TRUNC %8(s32)
      %copy2:fpr(s16) = COPY %4(s16) ; Likewise here to get back to an fpr s16 value.
      G_STORE %copy2(s16), %1(p0) :: (store 2 into %ir.b)
      %5:gpr(s32) = COPY %8(s32)
      $w0 = COPY %5(s32)
      RET_ReallyLR implicit $w0

This results in cross-bank copies which are bad for performance. We can't eliminate s16 GPRs completely because things like trunc/extend need to be able to deal with them, as well as G_EXTRACT/INSERT. Maybe there's a way to deal with those issues too, I'm not sure.

> Put differently I feel this is a work around other issue, either legalization or reg bank select. Bottom line, I really don't like this patch, but I let you judge what is the best way to move forward.

I think this ultimately stems from the fact that we lost fp/integer type information in the translator and trying to patch this back together from defs and uses is prone to failure. For performance reasons it there's probably no good reason to have G_PHIs with separate regbanks, but RBS is difficult to change. Not only that but in this case if you have a cyclic phi you can't even tell completely whether or not you have this problem because not every user has had a RegBank assigned yet.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D75086