[PATCH] D31783: Move size and alignment information of regclass to TargetRegisterInfo

[Alex Bradbury via Phabricator via llvm-commits]

asb accepted this revision.
asb added a comment.
This revision is now accepted and ready to land.

Reference to previous related discussion: https://reviews.llvm.org/D24631 and http://lists.llvm.org/pipermail/llvm-dev/2016-September/105027.html

As you expressed in the RFC mailing list thread, splitting the concept of register size and spill slot size makes sense as they are not always the same. Given the plan to move towards having this information depend on the target machine rather than being an inherent property of a register class, TargetRegisterInfo is the natural home. I've commented on a few tiny nits, but this all looks good to me.



================
Comment at: include/llvm/Target/TargetRegisterInfo.h:328
+
+  /// Return the size of the a stack slot allocated to hold a spilled
+  /// copy of this register.
----------------
"the a" -> "the"


================
Comment at: lib/Target/AArch64/AArch64InstrInfo.cpp:2728
       if (FillRC) {
-        assert(getRegClass(SrcReg)->getSize() == FillRC->getSize() &&
+        unsigned SrcSize = TRI.getRegSize(*getRegClass(SrcReg));
+        unsigned FillSize = TRI.getRegSize(*FillRC);
----------------
I'd personally be tempted to just put the TRI.getRegSize directly in the assert and avoid the need to worry about unused locals.


================
Comment at: lib/Target/Mips/MipsSEFrameLowering.cpp:892
+                                                  TRI->getSpillAlignment(RC),
+                                                  false);
     RS->addScavengingFrameIndex(FI);
----------------
Nit: second and third arguments seem to be indented an extra space, same in the call about 15 lines below.


Repository:
  rL LLVM

https://reviews.llvm.org/D31783