[PATCH] D122615: Early return in constrainOperandRegClass()

[Viacheslav Nikolaev via Phabricator via llvm-commits]

wvoquine added inline comments.


================
Comment at: llvm/lib/CodeGen/GlobalISel/Utils.cpp:61-65
+  // Check if the register class is already as required.
+  auto *OldRegClass = MRI.getRegClassOrNull(Reg);
+  if (OldRegClass == &RegClass)
+    return Reg;
+
----------------
dsanders wrote:
> I'm not keen on duplicating internals of constrainRegToClass() behaviour (even obvious behaviours like this one). Do you get a similar perf fix if you check between the possible change and the notification with something like this?:
> ```
>   auto *OldRegClass = MRI.getRegClassOrNull(Reg);
>   Register ConstrainedReg = constrainRegToClass(MRI, TII, RBI, Reg, RegClass);
>   if (ConstrainedReg != Reg) {
>     insert copy
>     notify observers
>   } else {
>     auto *NewRegClass = MRI.getRegClassOrNull(Reg);
>     if (OldRegClass != &NewRegClass) {
>       notify observers
>     }
>   }
> ```
> 
> 
We discussed that issue with Daniel separately. It looks like a viable approach could be to add API calls `RBI.needToConstrainGenericRegister()`->`MRI. needToConstrainGenericRegister()`->...
And the implementation of those calls would be involved in the corresponding check within chain of calls of `constrainRegToClass()` - so that we would be checking precisely the same way as the function imposing the constraints.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D122615