Check for dynamic allocas and inline asm that clobbers sp before building

selection dag (PR19012)

In X86SelectionDagInfo::EmitTargetCodeForMemcpy we check with MachineFrameInfo
to make sure that ESI isn't used as a base pointer register before we choose to
emit rep movs (which clobbers esi).

The problem is that MachineFrameInfo wouldn't know about dynamic allocas or
inline asm that clobbers the stack pointer until SelectionDAGBuilder has
encountered them.

This patch fixes the problem by checking for such things when building the
FunctionLoweringInfo.

Differential Revision: http://llvm-reviews.chandlerc.com/D2954

git-svn-id: https://llvm.org/svn/llvm-project/llvm/trunk@202930 91177308-0d34-0410-b5e6-96231b3b80d8

diff --git a/include/llvm/CodeGen/FunctionLoweringInfo.h b/include/llvm/CodeGen/FunctionLoweringInfo.h
index 50d320f8e839d338b7f3b098bd88b8d8ad249f0d..854cac250ccdc3b666007e8524d34de3516b271f 100644 (file)
--- a/include/llvm/CodeGen/FunctionLoweringInfo.h
+++ b/include/llvm/CodeGen/FunctionLoweringInfo.h
@@ -41,6 +41,7 @@ class MachineBasicBlock;
 class MachineFunction;
 class MachineModuleInfo;
 class MachineRegisterInfo;
+class SelectionDAG;
 class TargetLowering;
 class Value;
 
@@ -125,7 +126,7 @@ public:
   /// set - Initialize this FunctionLoweringInfo with the given Function
   /// and its associated MachineFunction.
   ///
-  void set(const Function &Fn, MachineFunction &MF);
+  void set(const Function &Fn, MachineFunction &MF, SelectionDAG *DAG);
 
   /// clear - Clear out all the function-specific state. This returns this
   /// FunctionLoweringInfo to an empty state, ready to be used for a

diff --git a/include/llvm/CodeGen/MachineFrameInfo.h b/include/llvm/CodeGen/MachineFrameInfo.h
index 0699863311f97ac1534c2f7fe6f79b70f8e73b68..1dedd74492cad4ac776417861e0776e32a8a0710 100644 (file)
--- a/include/llvm/CodeGen/MachineFrameInfo.h
+++ b/include/llvm/CodeGen/MachineFrameInfo.h
@@ -249,6 +249,7 @@ public:
     LocalFrameSize = 0;
     LocalFrameMaxAlign = 0;
     UseLocalStackAllocationBlock = false;
+    HasInlineAsmWithSPAdjust = false;
   }
 
   /// hasStackObjects - Return true if there are any stack objects in this

diff --git a/lib/CodeGen/SelectionDAG/FunctionLoweringInfo.cpp b/lib/CodeGen/SelectionDAG/FunctionLoweringInfo.cpp
index d842fabae25bf747c08b4e9a4652f75ef854f4e6..3c24f95039bbcb4d40a41210d1d28a7466e7e27e 100644 (file)
--- a/lib/CodeGen/SelectionDAG/FunctionLoweringInfo.cpp
+++ b/lib/CodeGen/SelectionDAG/FunctionLoweringInfo.cpp
@@ -33,6 +33,7 @@
 #include "llvm/Support/ErrorHandling.h"
 #include "llvm/Support/MathExtras.h"
 #include "llvm/Target/TargetInstrInfo.h"
+#include "llvm/Target/TargetFrameLowering.h"
 #include "llvm/Target/TargetLowering.h"
 #include "llvm/Target/TargetOptions.h"
 #include "llvm/Target/TargetRegisterInfo.h"
@@ -55,7 +56,8 @@ static bool isUsedOutsideOfDefiningBlock(const Instruction *I) {
   return false;
 }
 
-void FunctionLoweringInfo::set(const Function &fn, MachineFunction &mf) {
+void FunctionLoweringInfo::set(const Function &fn, MachineFunction &mf,
+                               SelectionDAG *DAG) {
   const TargetLowering *TLI = TM.getTargetLowering();
 
   Fn = &fn;
@@ -98,6 +100,43 @@ void FunctionLoweringInfo::set(const Function &fn, MachineFunction &mf) {
   for (; BB != EB; ++BB)
     for (BasicBlock::const_iterator I = BB->begin(), E = BB->end();
          I != E; ++I) {
+      // Look for dynamic allocas.
+      if (const AllocaInst *AI = dyn_cast<AllocaInst>(I)) {
+        if (!AI->isStaticAlloca()) {
+          unsigned Align = std::max(
+              (unsigned)TLI->getDataLayout()->getPrefTypeAlignment(
+                AI->getAllocatedType()),
+              AI->getAlignment());
+          unsigned StackAlign = TM.getFrameLowering()->getStackAlignment();
+          if (Align <= StackAlign)
+            Align = 0;
+          // Inform the Frame Information that we have variable-sized objects.
+          MF->getFrameInfo()->CreateVariableSizedObject(Align ? Align : 1, AI);
+        }
+      }
+
+      // Look for inline asm that clobbers the SP register.
+      if (isa<CallInst>(I) || isa<InvokeInst>(I)) {
+        ImmutableCallSite CS(I);
+        if (const InlineAsm *IA = dyn_cast<InlineAsm>(CS.getCalledValue())) {
+          unsigned SP = TLI->getStackPointerRegisterToSaveRestore();
+          std::vector<TargetLowering::AsmOperandInfo> Ops =
+            TLI->ParseConstraints(CS);
+          for (size_t I = 0, E = Ops.size(); I != E; ++I) {
+            TargetLowering::AsmOperandInfo &Op = Ops[I];
+            if (Op.Type == InlineAsm::isClobber) {
+              // Clobbers don't have SDValue operands, hence SDValue().
+              TLI->ComputeConstraintToUse(Op, SDValue(), DAG);
+              std::pair<unsigned, const TargetRegisterClass*> PhysReg =
+                TLI->getRegForInlineAsmConstraint(Op.ConstraintCode,
+                                                  Op.ConstraintVT);
+              if (PhysReg.first == SP)
+                MF->getFrameInfo()->setHasInlineAsmWithSPAdjust(true);
+            }
+          }
+        }
+      }
+
       // Mark values used outside their block as exported, by allocating
       // a virtual register for them.
       if (isUsedOutsideOfDefiningBlock(I))

diff --git a/lib/CodeGen/SelectionDAG/SelectionDAGBuilder.cpp b/lib/CodeGen/SelectionDAG/SelectionDAGBuilder.cpp
index ed66b8205f5b0a5bcfbbd4543afabe1432bd1aef..406adbd6e076a498d2beafcaa1f70470edd41997 100644 (file)
--- a/lib/CodeGen/SelectionDAG/SelectionDAGBuilder.cpp
+++ b/lib/CodeGen/SelectionDAG/SelectionDAGBuilder.cpp
@@ -860,10 +860,10 @@ void RegsForValue::AddInlineAsmOperands(unsigned Code, bool HasMatching,
       unsigned TheReg = Regs[Reg++];
       Ops.push_back(DAG.getRegister(TheReg, RegisterVT));
 
-      // Notice if we clobbered the stack pointer.  Yes, inline asm can do this.
       if (TheReg == SP && Code == InlineAsm::Kind_Clobber) {
-        MachineFrameInfo *MFI = DAG.getMachineFunction().getFrameInfo();
-        MFI->setHasInlineAsmWithSPAdjust(true);
+        // If we clobbered the stack pointer, MFI should know about it.
+        assert(DAG.getMachineFunction().getFrameInfo()->
+            hasInlineAsmWithSPAdjust());
       }
     }
   }
@@ -3437,9 +3437,7 @@ void SelectionDAGBuilder::visitAlloca(const AllocaInst &I) {
   setValue(&I, DSA);
   DAG.setRoot(DSA.getValue(1));
 
-  // Inform the Frame Information that we have just allocated a variable-sized
-  // object.
-  FuncInfo.MF->getFrameInfo()->CreateVariableSizedObject(Align ? Align : 1, &I);
+  assert(FuncInfo.MF->getFrameInfo()->hasVarSizedObjects());
 }
 
 void SelectionDAGBuilder::visitLoad(const LoadInst &I) {

diff --git a/lib/CodeGen/SelectionDAG/SelectionDAGISel.cpp b/lib/CodeGen/SelectionDAG/SelectionDAGISel.cpp
index 01c343818bd609e8f84524f5d38c8a4403badb7b..787810f8db2b7f56b223c0f0a0eeb42186ee87b5 100644 (file)
--- a/lib/CodeGen/SelectionDAG/SelectionDAGISel.cpp
+++ b/lib/CodeGen/SelectionDAG/SelectionDAGISel.cpp
@@ -417,7 +417,7 @@ bool SelectionDAGISel::runOnMachineFunction(MachineFunction &mf) {
   SplitCriticalSideEffectEdges(const_cast<Function&>(Fn), this);
 
   CurDAG->init(*MF, TLI);
-  FuncInfo->set(Fn, *MF);
+  FuncInfo->set(Fn, *MF, CurDAG);
 
   if (UseMBPI && OptLevel != CodeGenOpt::None)
     FuncInfo->BPI = &getAnalysis<BranchProbabilityInfo>();
@@ -427,7 +427,6 @@ bool SelectionDAGISel::runOnMachineFunction(MachineFunction &mf) {
   SDB->init(GFI, *AA, LibInfo);
 
   MF->setHasInlineAsm(false);
-  MF->getFrameInfo()->setHasInlineAsmWithSPAdjust(false);
 
   SelectAllBasicBlocks(Fn);
 

diff --git a/test/CodeGen/X86/inline-asm-sp-clobber-memcpy.ll b/test/CodeGen/X86/inline-asm-sp-clobber-memcpy.ll
new file mode 100644 (file)
index 0000000..b55571b
--- /dev/null
+++ b/test/CodeGen/X86/inline-asm-sp-clobber-memcpy.ll
@@ -0,0 +1,17 @@
+; RUN: llc < %s -force-align-stack -mtriple i386-apple-darwin -mcpu=i486 | FileCheck %s
+
+%struct.foo = type { [88 x i8] }
+
+declare void @bar(i8* nocapture, %struct.foo* align 4 byval) nounwind
+
+; PR19012
+; Don't clobber %esi if we have inline asm that clobbers %esp.
+define void @test1(%struct.foo* nocapture %x, i32 %y, i8* %z) nounwind {
+  call void @bar(i8* %z, %struct.foo* align 4 byval %x)
+  call void asm sideeffect inteldialect "xor esp, esp", "=*m,~{flags},~{esp},~{esp},~{dirflag},~{fpsr},~{flags}"(i8* %z)
+  ret void
+
+; CHECK-LABEL: test1:
+; CHECK: movl %esp, %esi
+; CHECK-NOT: rep;movsl
+}

diff --git a/test/CodeGen/X86/stack-align-memcpy.ll b/test/CodeGen/X86/stack-align-memcpy.ll
index 87bb85fad83e99a48ad094eff938971dd0a24911..0cc3aa848891b4eddf440709b76fa749310e724b 100644 (file)
--- a/test/CodeGen/X86/stack-align-memcpy.ll
+++ b/test/CodeGen/X86/stack-align-memcpy.ll
@@ -2,6 +2,9 @@
 
 %struct.foo = type { [88 x i8] }
 
+declare void @bar(i8* nocapture, %struct.foo* align 4 byval) nounwind
+declare void @baz(i8*) nounwind
+
 ; PR15249
 ; We can't use rep;movsl here because it clobbers the base pointer in %esi.
 define void @test1(%struct.foo* nocapture %x, i32 %y) nounwind {
@@ -15,4 +18,26 @@ define void @test1(%struct.foo* nocapture %x, i32 %y) nounwind {
 ; CHECK-NOT: rep;movsl
 }
 
-declare void @bar(i8* nocapture, %struct.foo* align 4 byval) nounwind
+; PR19012
+; Also don't clobber %esi if the dynamic alloca comes after the memcpy.
+define void @test2(%struct.foo* nocapture %x, i32 %y, i8* %z) nounwind {
+  call void @bar(i8* %z, %struct.foo* align 4 byval %x)
+  %dynalloc = alloca i8, i32 %y, align 1
+  call void @baz(i8* %dynalloc)
+  ret void
+
+; CHECK-LABEL: test2:
+; CHECK: movl %esp, %esi
+; CHECK-NOT: rep;movsl
+}
+
+; Check that we do use rep movs if we make the alloca static.
+define void @test3(%struct.foo* nocapture %x, i32 %y, i8* %z) nounwind {
+  call void @bar(i8* %z, %struct.foo* align 4 byval %x)
+  %statalloc = alloca i8, i32 8, align 1
+  call void @baz(i8* %statalloc)
+  ret void
+
+; CHECK-LABEL: test3:
+; CHECK: rep;movsl
+}