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

Ulrich Weigand via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Wed Sep 20 10:17:27 PDT 2017


uweigand added a comment.

Looks basically good to me, just a couple of cosmetic comments inline.



================
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,
----------------
No, it isn't :-)  Either the comment or the code is wrong.


================
Comment at: lib/Target/SystemZ/SystemZRegisterInfo.cpp:119
+      if ((ConstrainedRC32 =
+           getHintRC_LOCRMux(Reg, VRM, MRI, TRI, UsedByLOCRMux, Worklist)))
+        break;
----------------
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.


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


https://reviews.llvm.org/D36795





More information about the llvm-commits mailing list