[PATCH] D19896: [ARM] Fix Scavenger assert due to underestimated stack size

[Renato Golin via llvm-commits]

rengolin added a comment.

Hi Weiming,

This looks ok to me, but I have a couple of comments.

Also, the test is a bit brittle. I understand it has to be big, but it could have a comment on it of what's it trying to test, so that if it breaks on an unrelated patch, people can follow up easier.

cheers,
--renato


================
Comment at: lib/Target/ARM/ARMFrameLowering.cpp:1623
@@ +1622,3 @@
+      (RS && (MFI->estimateStackSize(MF) + 4 * (NumGPRSpills + NumFPRSpills) +
+                  (!hasFP(MF) ? ArgStackSize : 0) + 16 /* possible paddings */ +
+                  ((hasFP(MF) && AFI->hasStackFrame()) ? 4 : 0) >=
----------------
Why does ArgStackSize depends on hasFP()?

Also, what's the expected value for the stack limit here? I hope 16 is not a big fraction of it, or we'll be pessimising it on some corner cases.


Repository:
  rL LLVM

http://reviews.llvm.org/D19896