Fixed "use POP for small post-call stack clean-up" code size optimization

[aaboud]

Summary

Fixed Bug 24649.
Now the optimization will search for the right GPR type depends on the architecture size 32bit or 64bit.

[mkuper]

Thanks, Amjad!
Can you also fix pop-stack-cleanup.ll to also check the 64-bit case?

lib/Target/X86/X86FrameLowering.cpp

1853 ↗

(On Diff #33975)

I think this should not be strictly necessary, because if it's negative, NumPops will also be negative, so it will never be 1 or 2. But if you think it makes the intent clearer, that's ok.

1877 ↗

(On Diff #33975)

Argh, right, thanks. I wonder how this manages to work correctly with integrated-as...

[aaboud]

lib/Target/X86/X86FrameLowering.cpp

1853 ↗

(On Diff #33975)

The % of negative numbers are not well defined, especially, as offset is an "int" and "SlotSize" is an "unsigned int".
So, I do not want to depend on that result if I know that we should not handle negative numbers.

[aaboud]

Added a test case to cover this chang.

[mkuper]

lib/Target/X86/X86FrameLowering.cpp

1853 ↗

(On Diff #34202)

Ohhh, right, forgot the sign is implementation-defined!
Thanks a lot.

llvm/trunk/lib/Target/X86/X86FrameLowering.cpp

 #ifdef XDEBUG
   MF.verify();
 #endif
 }
 
 bool X86FrameLowering::adjustStackWithPops(MachineBasicBlock &MBB,
     MachineBasicBlock::iterator MBBI, DebugLoc DL, int Offset) const {
 
+  if (Offset <= 0)
+    return false;
+
   if (Offset % SlotSize)
     return false;
 
   int NumPops = Offset / SlotSize;
   // This is only worth it if we have at most 2 pops.
   if (NumPops != 1 && NumPops != 2)
     return false;
 
   // Handle only the trivial case where the adjustment directly follows
   // a call. This is the most common one, anyway.
   if (MBBI == MBB.begin())
     return false;
   MachineBasicBlock::iterator Prev = std::prev(MBBI);
   if (!Prev->isCall() || !Prev->getOperand(1).isRegMask())
     return false;
 
   unsigned Regs[2];
   unsigned FoundRegs = 0;
 
   auto RegMask = Prev->getOperand(1);
-  
-  // Try to find up to NumPops free registers. 
-  for (auto Candidate : X86::GR32_NOREX_NOSPRegClass) {
+  auto &RegClass =
+      Is64Bit ? X86::GR64_NOREX_NOSPRegClass : X86::GR32_NOREX_NOSPRegClass;
+  // Try to find up to NumPops free registers.
+  for (auto Candidate : RegClass) {
 
     // Poor man's liveness:
     // Since we're immediately after a call, any register that is clobbered
     // by the call and not defined by it can be considered dead.
     if (!RegMask.clobbersPhysReg(Candidate))
       continue;
 
     bool IsDef = false;

llvm/trunk/test/CodeGen/X86/pop-stack-cleanup.ll

-; RUN: llc < %s -mtriple=i686-windows | FileCheck %s -check-prefix=CHECK -check-prefix=NORMAL
+; RUN: llc < %s -mtriple=i686-windows | FileCheck %s -check-prefix=CHECK 
+; RUN: llc < %s -mtriple=x86_64-linux | FileCheck %s -check-prefix=LINUX64
 
 declare void @param1(i32 %a)
 declare i32 @param2_ret(i32 %a, i32 %b)
 declare i64 @param2_ret64(i32 %a, i32 %b)
 declare void @param2(i32 %a, i32 %b)
 declare void @param3(i32 %a, i32 %b, i32 %c)
+declare void @param8(i64, i64, i64, i64, i64, i64, i64, i64)
+
 
 define void @test() minsize {
 ; CHECK-LABEL: test:
 ; CHECK: calll _param1
 ; CHECK-NEXT: popl %eax
 ; CHECK: calll _param2
 ; CHECK-NEXT: popl %eax
 ; CHECK-NEXT: popl %ecx

 ; CHECK-NEXT: popl %edx
 ; CHECK-DAG: movl {{.*}}, %ecx
 ; CHECK-DAG: movl {{.*}}, %edx
 ; CHECK: calll _spill
   %i = call i32 @param2_ret(i32 1, i32 2)
   call void @spill(i32 %a, i32 %b, i32 %c)
   ret void
 }
+
+define void @test_linux64(i32 %size) minsize {
+; LINUX64-LABEL: test_linux64:
+; LINUX64: pushq %rbp
+; LINUX64: callq param8
+; LINUX64-NEXT: popq %rax
+; LINUX64-NEXT: popq %rcx
+
+  %a = alloca i64, i32 %size, align 8
+  call void @param8(i64 1, i64 2, i64 3, i64 4, i64 5, i64 6, i64 7, i64 8)
+  ret void
+}