[PATCH] Added a size field to the stack map record to handle subregister spills.

[Jakob Stoklund Olesen]

On Nov 15, 2013, at 10:06 AM, Andrew Trick <atrick at apple.com> wrote:

> Hi ributzka, lhames,
> 
> Implementing this on bigendian platforms could get strange. I added a
> target hook, getStackSlotRange, per Jakob's recommendation to make
> this as explicit as possible.

Hi Andy,

Just commenting on the getStackSlotRange function:

+void TargetInstrInfo::getStackSlotRange(const TargetRegisterClass *RC,
+                                        unsigned SubIdx, unsigned &Size,
+                                        int &Offset,
+                                        const TargetMachine *TM) const {

Would it be more natural to return a bool error instead of using a signed Offset?

+  if (!TM->getDataLayout()->isLittleEndian()) {
+    report_fatal_error("Big endian spill slot ranges are unimplemented on "
+                       "this platform.");
+  }

You can give the BE guys a decent default implementation with RC->getSize() - (Size+Offset).

+  if (SubIdx) {
+    Size = TM->getRegisterInfo()->getSubRegIdxSize(SubIdx);
+    // Convert bit size to byte size to be consistent with
+    // MCRegisterClass::getSize().
+    assert((Size % 8) == 0 && "subreg not byte-sized");

It's perfectly fine for a sub-register to be a non-multiple of 8. You should return an error, not assert here.

+    Size /= 8;
+    Offset = TM->getRegisterInfo()->getSubRegIdxOffset(SubIdx);
+    if (Offset > 0) {
+      assert(((unsigned)Offset % 8) == 0 && "subreg not byte-sized");

Same.

+      Offset = (unsigned)Offset / 8;
+    }
+  }
+  else {
+    Size = RC->getSize();
+    Offset = 0;

Could you make the SubIdx==0 case an early return to reduce indentation?

Thanks,
/jakob