[PATCH] D88663: [AArch64] Use TargetRegisterClass::hasSubClassEq in tryToFindRegisterToRename

[Cullen Rhodes via Phabricator via llvm-commits]

c-rhodes added inline comments.


================
Comment at: llvm/test/CodeGen/AArch64/stp-opt-with-renaming.mir:258
 # CHECK:         renamable $x8 = MRS 58880
-# CHECK-NEXT:    $w9 = ORRWrs $wzr, killed renamable $w8, 0, implicit-def $x9
+# CHECK-NEXT:    $w2 = ORRWrs $wzr, killed renamable $w8, 0, implicit-def $w2
 # CHECK-NEXT:    renamable $x8 = MRS 55840
----------------
c-rhodes wrote:
> efriedma wrote:
> > Somehow the implicit-def is getting rewritten from an x register to a w register?
> > 
> > If the implicit-def is dead, we can just drop it from the operand list.  If it's not, rewriting like this is going to mess with liveness computation.
> > Somehow the implicit-def is getting rewritten from an x register to a w register?
> 
> Ah, that'll be because in `UpdateMIs` when `getRegClassConstraint` returns NULL for the implicit-def register it uses the result register, assuming the opcode is in the whitelist. I don't think `getRegClassConstraint` will work for this implicit-def operand, I suppose `getMinimalPhysRegClass` could be called instead if register class can't be determined by `getRegClassConstraint`?
> > Somehow the implicit-def is getting rewritten from an x register to a w register?
> 
> Ah, that'll be because in `UpdateMIs` when `getRegClassConstraint` returns NULL for the implicit-def register it uses the result register, assuming the opcode is in the whitelist. I don't think `getRegClassConstraint` will work for this implicit-def operand, I suppose `getMinimalPhysRegClass` could be called instead if register class can't be determined by `getRegClassConstraint`?

I've updated the patch to use `getMinimalPhysRegClass` for the implicit-def operand RC when `getRegClassConstraint` returns NULL, rather than copying the result register. The implicit-def is no longer getting rewritten from an x register to a w register, hopefully the fix is ok?


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

https://reviews.llvm.org/D88663