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

[Quentin Colombet via llvm-commits]

qcolombet added a comment.

Thanks Geoff, the update looks good, just one missing thing.
(See my inlined comments.)

FWIW, the AArch64 changes looks good to me as well.

Q.


================
Comment at: lib/CodeGen/PrologEpilogInserter.cpp:599
@@ +598,3 @@
+    auto ObjOffset = MFI->getObjectOffset(i);
+    auto ObjSize = MFI->getObjectSize(i);
+    int ObjStart, ObjEnd;
----------------
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.

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

================
Comment at: lib/CodeGen/PrologEpilogInserter.cpp:628
@@ +627,3 @@
+  if (StackBytesFree.none()) {
+    StackBytesFree.clear();
+    return false;
----------------
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.)


http://reviews.llvm.org/D20220