Make X86CallFrameOptimization work with code generated by fast isel.

[thakis]

Summary

My motivation is that I want to enable fast isel for thiscall and other callee-pop calls, but that currently means codegen stops creating pushls, which means test/CodeGen/X86/win32_sret.ll starts failing (harmlesly) with the fast isel change, and updating it to have to different expectations is harder than just teaching this pass about fast isel code -- and this fixes a TODO too.

[mkuper]

The change itself LGTM.

However, I don't think we should be running this pass at all with CodeGenOpt::None.
It was originally enabled only for -Os, and was later also enabled (by Hans, IIRC) for other opt level - but I think enabling it for -O0 was unintentional.

[thakis]

That makes sense, I was surprised the pass was running at -O0 to be honest. I'll still land this, since it fixes a TODO. Thanks for the review!

[thakis]

Landed in r275320.

[labath]

The optimized frames this change produces are confusing lldb's unwinder. We'll try to make it more resilient as that is a generally useful thing, but either way, I do recommend switching this off for -O0.

[thakis]

https://reviews.llvm.org/D22362 removes this pass from -O0 (which means this change here won't have an observable effect anymore).

[mkuper]

This was committed, but the review was never closed. Cleaning up.

lib/Target/X86/X86CallFrameOptimization.cpp

void X86CallFrameOptimization::collectCallInfo(MachineFunction &MF,
   }
 
   // For globals in PIC mode, we can have some LEAs here.
   // Ignore them, they don't bother us.
   // TODO: Extend this to something that covers more cases.
   while (I->getOpcode() == X86::LEA32r)
     ++I;
 
-  // We expect a copy instruction here.
-  // TODO: The copy instruction is a lowering artifact.
-  //       We should also support a copy-less version, where the stack
-  //       pointer is used directly.
-  if (!I->isCopy() || !I->getOperand(0).isReg())
-    return;
+  unsigned StackPtr = RegInfo.getStackRegister();
+  // SelectionDAG (but not FastISel) inserts a copy of ESP into a virtual
+  // register here.  If it's there, use that virtual register as stack pointer
+  // instead.
+  if (I->isCopy() && I->getOperand(0).isReg() && I->getOperand(1).isReg() &&
+      I->getOperand(1).getReg() == StackPtr) {
     Context.SPCopy = &*I++;
-
-  unsigned StackPtr = Context.SPCopy->getOperand(0).getReg();
+    StackPtr = Context.SPCopy->getOperand(0).getReg();
+  }
 
   // Scan the call setup sequence for the pattern we're looking for.
   // We only handle a simple case - a sequence of store instructions that
   // push a sequence of stack-slot-aligned values onto the stack, with
   // no gaps between them.
   if (MaxAdjust > 4)
     Context.MovVector.resize(MaxAdjust, nullptr);
 
if (!TFL->hasFP(MF))
           MBB, std::next(Push), DL,
           MCCFIInstruction::createAdjustCfaOffset(nullptr, SlotSize));
 
     MBB.erase(MOV);
   }
 
   // The stack-pointer copy is no longer used in the call sequences.
   // There should not be any other users, but we can't commit to that, so:
-  if (MRI->use_empty(Context.SPCopy->getOperand(0).getReg()))
+  if (Context.SPCopy && MRI->use_empty(Context.SPCopy->getOperand(0).getReg()))
     Context.SPCopy->eraseFromParent();
 
   // Once we've done this, we need to make sure PEI doesn't assume a reserved
   // frame.
   X86MachineFunctionInfo *FuncInfo = MF.getInfo<X86MachineFunctionInfo>();
   FuncInfo->setHasPushSequences(true);
 }
 

test/CodeGen/X86/fast-isel-call.ll

 declare zeroext i1 @foo()  nounwind
 
 declare void @foo2(%struct.s* byval)
 
 define void @test2(%struct.s* %d) nounwind {
   call void @foo2(%struct.s* byval %d )
   ret void
 ; CHECK-LABEL: test2:
-; CHECK: movl (%eax)
-; CHECK: movl {{.*}}, (%esp)
-; CHECK: movl 4(%eax)
-; CHECK: movl {{.*}}, 4(%esp)
-; CHECK: movl 8(%eax)
-; CHECK: movl {{.*}}, 8(%esp)
+; CHECK: movl (%eax), %[[reg1:e[a-d]x]]
+; CHECK: movl 4(%eax), %[[reg2:e[a-d]x]]
+; CHECK: movl 8(%eax), %[[reg3:e[a-d]x]]
+; CHECK: pushl %[[reg3]]
+; CHECK: pushl %[[reg2]]
+; CHECK: pushl %[[reg1]]
 }
 
 declare void @llvm.memset.p0i8.i32(i8* nocapture, i8, i32, i32, i1) nounwind
 
 define void @test3(i8* %a) {
   call void @llvm.memset.p0i8.i32(i8* %a, i8 0, i32 100, i32 1, i1 false)
   ret void
 ; CHECK-LABEL: test3: