[PATCH] D19896: [ARM] Fix Scavenger assert due to underestimated stack size
Renato Golin via llvm-commits
llvm-commits at lists.llvm.org
Wed May 4 03:52:03 PDT 2016
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
More information about the llvm-commits
mailing list