[PATCH] D92341: [NFC][MC] TargetRegisterInfo::getSubReg is a MCRegister.

Gaurav Jain via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Wed Dec 2 10:19:52 PST 2020


gjain added inline comments.


================
Comment at: llvm/lib/CodeGen/CalcSpillWeights.cpp:67
   const TargetRegisterClass *rc = MRI.getRegClass(Reg);
-  Register CopiedPReg = (HSub ? TRI.getSubReg(HReg, HSub) : HReg);
+  Register CopiedPReg = (HSub ? Register(TRI.getSubReg(HReg, HSub)) : HReg);
   if (rc->contains(CopiedPReg))
----------------
CopiedPReg should be a MCRegister instead. Looking at the documentation of contains it seems there is a desire that it only takes MCRegisters


================
Comment at: llvm/lib/Target/Hexagon/BitTracker.cpp:344
   Register PhysR =
-      (RR.Sub == 0) ? Register(RR.Reg) : TRI.getSubReg(RR.Reg, RR.Sub);
+      (RR.Sub == 0) ? RR.Reg : Register(TRI.getSubReg(RR.Reg, RR.Sub));
   return getPhysRegBitWidth(PhysR);
----------------
Seems like PhyR should be a MCRegister and getPhysRegBitWidth should only take MCRegisters.


================
Comment at: llvm/lib/Target/Hexagon/HexagonExpandCondsets.cpp:596
+    Register PhysS =
+        (RS.Sub == 0) ? PhysR : Register(TRI->getSubReg(PhysR, RS.Sub));
     const TargetRegisterClass *RC = TRI->getMinimalPhysRegClass(PhysS);
----------------
getMinimalPhysRegClass takes an MCRegister so I think we should not be converting it to a Register


================
Comment at: llvm/lib/Target/Mips/MipsExpandPseudo.cpp:739
     // On Mips64 result of slt is GPR32.
     Register Scratch2_32 =
+        (Size == 8) ? Register(STI->getRegisterInfo()->getSubReg(Scratch2,
----------------
Seems like this should be a MCRegister


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D92341



More information about the llvm-commits mailing list