[PATCH] D30052: [X86] Emit fewer instructions to allocate >16GB stack frames

[Simon Dardis via Phabricator via llvm-commits]

sdardis accepted this revision.
sdardis added a comment.
This revision is now accepted and ready to land.

LGTM with the first inlined comment addressed. When I tested this, the new stack pointer in the epilogue case was pointing at the value above the spilled base pointer. So %rbp got filled with junk when popped, at then %rip got the value of the base pointer.



================
Comment at: lib/Target/X86/X86FrameLowering.cpp:293-311
+      unsigned PushOpc = Is64Bit ? X86::PUSH64r : X86::PUSH32r;
+      unsigned LoadOpc = Is64Bit ? X86::MOV64rm : X86::MOV32rm;
+      unsigned XchgOpc = Is64Bit ? X86::XCHG64rm : X86::XCHG32rm;
+      BuildMI(MBB, MBBI, DL, TII.get(PushOpc))
+          .addReg(Rax, RegState::Kill)
+          .setMIFlag(Flag);
+      BuildMI(MBB, MBBI, DL, TII.get(MovRIOpc), Rax)
----------------
Given the assert(Is64Bit..) above this hunk, can't PushOpc be replaced with PUSH64r directly? Likewise for LoadOpc, XchgOpc.


================
Comment at: lib/Target/X86/X86FrameLowering.cpp:300
+      BuildMI(MBB, MBBI, DL, TII.get(MovRIOpc), Rax)
+          .addImm(Offset)
+          .setMIFlag(Flag);
----------------
In the epilogue case, this needs to be Offset + SlotSize, as we need to account for the spill we just performed.


https://reviews.llvm.org/D30052