[PATCH] D89018: [NFC][MC] MCRegister API typing.

[Mircea Trofin via Phabricator via llvm-commits]

mtrofin added inline comments.


================
Comment at: llvm/lib/CodeGen/RegisterCoalescer.cpp:2396
 public:
-  JoinVals(LiveRange &LR, unsigned Reg, unsigned SubIdx, LaneBitmask LaneMask,
-           SmallVectorImpl<VNInfo*> &newVNInfo, const CoalescerPair &cp,
+  JoinVals(LiveRange &LR, Register Reg, unsigned SubIdx, LaneBitmask LaneMask,
+           SmallVectorImpl<VNInfo *> &newVNInfo, const CoalescerPair &cp,
----------------
gjain wrote:
> Should this be a reference?
No, storage-wise it's a 32 bit integer.


================
Comment at: llvm/lib/CodeGen/RegisterCoalescer.h:33
     /// virtual or physical register.
-    unsigned DstReg = 0;
+    Register DstReg = 0;
 
----------------
gjain wrote:
> Should we omit the assignment since we are already using the default constructor?
done.


================
Comment at: llvm/lib/Target/SystemZ/SystemZRegisterInfo.cpp:112
           Register Reg = MO->getReg();
-          Register PhysReg =
-            Register::isPhysicalRegister(Reg) ? Reg : VRM->getPhys(Reg);
+          Register PhysReg = Register::isPhysicalRegister(Reg)
+                                 ? Reg
----------------
gjain wrote:
> Should this be instead?
> 
> ```
> Register PhysReg(Register::isPhysicalRegister(MO->getReg()) ? MO->getReg() : VRM->getPhys(Reg))
> ```
It'll be lowered to the same thing. Plus, we still need to cast VRM->getPhys so both sides of the ternary operator have the same type.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D89018