[PATCH] D122615: Early return in constrainOperandRegClass()

[Viacheslav Nikolaev via Phabricator via llvm-commits]

wvoquine added inline comments.


================
Comment at: llvm/lib/CodeGen/MachineRegisterInfo.cpp:82
   if (NewRC->getNumRegs() < MinNumRegs)
     return nullptr;
   MRI.setRegClass(Reg, NewRC);
----------------
dsanders wrote:
> This is more of a naming nitpick but this path isn't constraining the reg class yet constraining is still needed. I believe the caller is expected to emit a copy instead and that caller should notify the observers correctly
> 
> FWIW, MinNumRegs is hardly ever used, I can only find a couple instances
Maybe I should call `needsConstrainOrCopy()` ?


================
Comment at: llvm/lib/CodeGen/RegisterBankInfo.cpp:131-132
+    Register Reg, const TargetRegisterClass &RC, MachineRegisterInfo &MRI) {
+  // If the register already has a class, there's a chance
+  // it's already constrained appropriately.
+  if (MRI.getRegClassOrNull(Reg) != nullptr)
----------------
dsanders wrote:
> There's also a chance it isn't and constrainGenericRegister() changes the class requiring that the observers be notified
You mean, it's not constrained appropriately but the Reg's class is the same?


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

https://reviews.llvm.org/D122615