[PATCH] D20220: [PEI, AArch64] Use empty spaces in stack area for local stack slot allocation.

[Geoff Berry via llvm-commits]

gberry added a comment.

Thanks Quentin,

I'll address the issues you raised shortly.


================
Comment at: lib/CodeGen/PrologEpilogInserter.cpp:599
@@ +598,3 @@
+    auto ObjOffset = MFI->getObjectOffset(i);
+    auto ObjSize = MFI->getObjectSize(i);
+    int ObjStart, ObjEnd;
----------------
qcolombet wrote:
> Putting back my comment from the other day: having the actual type (instead of auto) would be nice. At first glance, I have no idea if those are unsigned, int, etc.
Sorry, I didn't ignore you :)  I think your original comment was on a different part of the original change and I didn't notice these needed fixing too.  I'll double check all of the autos just in case.

================
Comment at: lib/CodeGen/PrologEpilogInserter.cpp:622
@@ +621,3 @@
+  if (StackBytesFree.empty())
+    return false;
+
----------------
qcolombet wrote:
> The empty check is now also checked as part of the none check.
> I would suggest to get rid of the empty check.
Yeah, this is probably overkill.

================
Comment at: lib/CodeGen/PrologEpilogInserter.cpp:628
@@ +627,3 @@
+  if (StackBytesFree.none()) {
+    StackBytesFree.clear();
+    return false;
----------------
qcolombet wrote:
> I guess we can get rid of the call to clear.
> Maybe that was an optimization when StackBytesFree is not empty to have none being faster?
> (I am fine keeping it.)
Yes, my thinking was that later calls to empty()/none() would complete faster.


http://reviews.llvm.org/D20220