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

[Cullen Rhodes via Phabricator via llvm-commits]

c-rhodes added inline comments.


================
Comment at: llvm/lib/Target/AArch64/AArch64LoadStoreOptimizer.cpp:839
+                  if (MI.getOpcode() == AArch64::ORRWrs ||
+                      MI.getOpcode() == AArch64::ADDWri)
+                    MatchingReg = MI.getOperand(0).getReg();
----------------
efriedma wrote:
> We don't want to repeat this list of opcodes in multiple places.
Makes sense, I'll create a function.


================
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
----------------
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`?


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

https://reviews.llvm.org/D88663