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

[Cullen Rhodes via Phabricator via llvm-commits]

c-rhodes marked an inline comment as done.
c-rhodes added inline comments.


================
Comment at: llvm/lib/Target/AArch64/AArch64LoadStoreOptimizer.cpp:813
+        if (TRI->getMinimalPhysRegClass(OriginalReg)
+                ->hasSubClassEq(TRI->getMinimalPhysRegClass(SubOrSuper)))
           return SubOrSuper;
----------------
efriedma wrote:
> I don't think this does what you want.  Suppose we had a separate register class for every single AArch64 register.  Then this would fail unless OriginalReg and SubOrSuper are equal, I think.
> 
> Maybe instead of `TRI->getMinimalPhysRegClass(OriginalReg)`, we can use MachineInstr::getRegClassConstraint?
> Maybe instead of TRI->getMinimalPhysRegClass(OriginalReg), we can use MachineInstr::getRegClassConstraint?

Took me a while to figure out how to use it but I've updated it. An issue I found was for some machine instrs the number of operands was different to the `MCInstrDesc` so it couldn't find a regclass when `OpIdx` >= `MCID.getNumOperands()`. This caused a difference in `test8` of `llvm/test/CodeGen/AArch64/stp-opt-with-renaming.mir` since the `implicit-def` at the end of the ORR no longer uses the rename register.


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

https://reviews.llvm.org/D88663