[PATCH] D36795: [SystemZ] Increase number of LOCRs emitted by passing regalloc hints

[Jonas Paulsson via Phabricator via llvm-commits]

jonpa added inline comments.


================
Comment at: lib/Target/SystemZ/SystemZRegisterInfo.cpp:29
+// that MO will eventually belong to. If the regclass is GRX32 and MO has not
+// been allocated yet, nullptr is returned.
+static const TargetRegisterClass *getRC32(MachineOperand &MO,
----------------
uweigand wrote:
> No, it isn't :-)  Either the comment or the code is wrong.
Aah, the comment was rotten.


================
Comment at: lib/Target/SystemZ/SystemZRegisterInfo.cpp:119
+      if ((ConstrainedRC32 =
+           getHintRC_LOCRMux(Reg, VRM, MRI, TRI, UsedByLOCRMux, Worklist)))
+        break;
----------------
uweigand wrote:
> I think it might simplify the code to just inline getHintRC_LOCRMux here ... it uses so many of the local variables defined here, and doesn't really serve any separate purpose, so it doesn't make much sense to have it as a separate function.
> 
> In fact, maybe even getConstrainedRC32_LOCRMux should be inlined as well.
The reason for having the LOCRMux handling separate, is that I had in mind the other Mux cases as well, that we might want to investigate.

So, it is better to have it inlined until we actually use more Mux instructions as opposed to have the code somewhat ready?

Or is it better to inline even with the other instructions handlings added?


================
Comment at: lib/Target/SystemZ/SystemZRegisterInfo.cpp:133
+  TargetRegisterInfo::getRegAllocationHints(VirtReg, Order, Hints, MF, VRM);
+  return false;
+}
----------------
uweigand wrote:
> Maybe better "return TargetRegisterInfo:: ..." in case the default is ever changed to return hard hints in some cases.
aah, yes.


https://reviews.llvm.org/D36795