[PATCH] D33443: Fix PR33031: Cannot scavenge register without an emergency spill slot.

[Matthias Braun via Phabricator via llvm-commits]

MatzeB added a comment.

> Introducing option -reg-scavenging-always-use-spill-slot to enable writing maintainable tests for register scavenging.

That seems like a lot of machinery to test this. How about just adding a debug print and matching for that? Alternatively you should be able to match the extra spillslot in the `stack:` section of the mir file maybe?



================
Comment at: lib/Target/AArch64/AArch64FrameLowering.cpp:142
 
+/// estimateRSStackSizeLimit - Look at each instruction that references stack
+/// frames and return the stack size limit beyond which some of these
----------------
Don't repeat the function name in the doxygen comment (old code does that but we don't do it anymore in new code).


================
Comment at: lib/Target/AArch64/AArch64FrameLowering.cpp:145
+/// instructions will require a scratch register during their expansion later.
+// FIXME: Move to TII?
+static unsigned estimateRSStackSizeLimit(MachineFunction &MF) {
----------------
Just do it? (Or remove the FIXME)


================
Comment at: lib/Target/AArch64/AArch64FrameLowering.cpp:151
+  unsigned Limit = 256;
+  for (auto &MBB : MF) {
+    for (auto &MI : MBB) {
----------------
Please spell `MachineBasicBlock` instead of `auto` that is friendlier to the reader. We tend to only use `auto` in contexts where the type is immediately clear such as `auto X = dyn_cast<SomeType>(...)`.
Same with the next loop.


================
Comment at: lib/Target/AArch64/AArch64FrameLowering.cpp:164
+            AArch64FrameOffsetCannotUpdate)
+          Limit = 0;
+      }
----------------
Simply `return 0;`?


https://reviews.llvm.org/D33443