[llvm] r175334 - [ms-inline asm] Do not omit the frame pointer if we have ms-inline assembly.

Benjamin Kramer benny.kra at gmail.com
Sun Feb 17 06:05:28 PST 2013


On 16.02.2013, at 02:25, Chad Rosier <mcrosier at apple.com> wrote:

> Author: mcrosier
> Date: Fri Feb 15 19:25:28 2013
> New Revision: 175334
> 
> URL: http://llvm.org/viewvc/llvm-project?rev=175334&view=rev
> Log:
> [ms-inline asm] Do not omit the frame pointer if we have ms-inline assembly.
> 
> If the frame pointer is omitted, and any stack changes occur in the inline
> assembly, e.g.: "pusha", then any C local variable or C argument references
> will be incorrect.  

Isn't this a X86-specific thing and should be handled with X86's setForceFramePointer? Feels weird to add yet another flag to MachineFunction for this.

This commit also caused a ton of valgrind failures: http://lab.llvm.org:8011/builders/llvm-x86_64-linux-vg_leak/builds/328

Backtrace looks like this:
==30816== Conditional jump or move depends on uninitialised value(s)
==30816==    at 0xCCC440: llvm::X86FrameLowering::hasFP(llvm::MachineFunction const&) const (X86FrameLowering.cpp:55)
==30816==    by 0xCBA08A: llvm::X86RegisterInfo::getRegPressureLimit(llvm::TargetRegisterClass const*, llvm::MachineFunction&) const (X86RegisterInfo.cpp:221)
==30816==    by 0xE2D715: (anonymous namespace)::RegReductionPQBase::RegReductionPQBase(llvm::MachineFunction&, bool, bool, bool, llvm::TargetInstrInfo const*, llvm::TargetRegisterInfo const*, llvm::TargetLowering const*) (ScheduleDAGRRList.cpp:1639)
==30816==    by 0xE32305: (anonymous namespace)::RegReductionPriorityQueue<(anonymous namespace)::ilp_ls_rr_sort>::RegReductionPriorityQueue(llvm::MachineFunction&, bool, bool, llvm::TargetInstrInfo const*, llvm::TargetRegisterInfo const*, llvm::TargetLowering const*) (ScheduleDAGRRList.cpp:1750)
==30816==    by 0xE3201F: llvm::createILPListDAGScheduler(llvm::SelectionDAGISel*, llvm::CodeGenOpt::Level) (ScheduleDAGRRList.cpp:3008)
==30816==    by 0xEC9C59: llvm::createDefaultScheduler(llvm::SelectionDAGISel*, llvm::CodeGenOpt::Level) (SelectionDAGISel.cpp:234)
==30816==    by 0xECF887: llvm::SelectionDAGISel::CreateScheduler() (SelectionDAGISel.cpp:1410)
==30816==    by 0xECC413: llvm::SelectionDAGISel::CodeGenAndEmitDAG() (SelectionDAGISel.cpp:683)
==30816==    by 0xECB1E5: llvm::SelectionDAGISel::SelectBasicBlock(llvm::ilist_iterator<llvm::Instruction const>, llvm::ilist_iterator<llvm::Instruction const>, bool&) (SelectionDAGISel.cpp:515)
==30816==    by 0xECE032: llvm::SelectionDAGISel::SelectAllBasicBlocks(llvm::Function const&) (SelectionDAGISel.cpp:1184)
==30816==    by 0xECA56C: llvm::SelectionDAGISel::runOnMachineFunction(llvm::MachineFunction&) (SelectionDAGISel.cpp:375)
==30816==    by 0x10A7F10: llvm::MachineFunctionPass::runOnFunction(llvm::Function&) (MachineFunctionPass.cpp:33)
==30816==


> 
> I pass no judgement on anyone who would do such a thing. ;)
> rdar://13218191
> 
> Modified:
>    llvm/trunk/include/llvm/CodeGen/MachineFunction.h
>    llvm/trunk/include/llvm/CodeGen/MachineInstr.h
>    llvm/trunk/lib/CodeGen/SelectionDAG/SelectionDAGISel.cpp
>    llvm/trunk/lib/Target/X86/X86FrameLowering.cpp
>    llvm/trunk/test/CodeGen/X86/ms-inline-asm.ll
> 
> Modified: llvm/trunk/include/llvm/CodeGen/MachineFunction.h
> URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/include/llvm/CodeGen/MachineFunction.h?rev=175334&r1=175333&r2=175334&view=diff
> ==============================================================================
> --- llvm/trunk/include/llvm/CodeGen/MachineFunction.h (original)
> +++ llvm/trunk/include/llvm/CodeGen/MachineFunction.h Fri Feb 15 19:25:28 2013
> @@ -131,6 +131,9 @@ class MachineFunction {
>   /// about the control flow of such functions.
>   bool ExposesReturnsTwice;
> 
> +  /// True if the function includes MS-style inline assembly.
> +  bool HasMSInlineAsm;
> +
>   MachineFunction(const MachineFunction &) LLVM_DELETED_FUNCTION;
>   void operator=(const MachineFunction&) LLVM_DELETED_FUNCTION;
> public:
> @@ -214,6 +217,17 @@ public:
>   void setExposesReturnsTwice(bool B) {
>     ExposesReturnsTwice = B;
>   }
> +
> +  /// Returns true if the function contains any MS-style inline assembly.
> +  bool hasMSInlineAsm() const {
> +    return HasMSInlineAsm;
> +  }
> +
> +  /// Set a flag that indicates that the function contains MS-style inline
> +  /// assembly.
> +  void setHasMSInlineAsm(bool B) {
> +    HasMSInlineAsm = B;
> +  }
> 
>   /// getInfo - Keep track of various per-function pieces of information for
>   /// backends that would like to do so.
> 
> Modified: llvm/trunk/include/llvm/CodeGen/MachineInstr.h
> URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/include/llvm/CodeGen/MachineInstr.h?rev=175334&r1=175333&r2=175334&view=diff
> ==============================================================================
> --- llvm/trunk/include/llvm/CodeGen/MachineInstr.h (original)
> +++ llvm/trunk/include/llvm/CodeGen/MachineInstr.h Fri Feb 15 19:25:28 2013
> @@ -642,6 +642,9 @@ public:
>   bool isKill() const { return getOpcode() == TargetOpcode::KILL; }
>   bool isImplicitDef() const { return getOpcode()==TargetOpcode::IMPLICIT_DEF; }
>   bool isInlineAsm() const { return getOpcode() == TargetOpcode::INLINEASM; }
> +  bool isMSInlineAsm() const { 
> +    return getOpcode() == TargetOpcode::INLINEASM && getInlineAsmDialect();
> +  }
>   bool isStackAligningInlineAsm() const;
>   InlineAsm::AsmDialect getInlineAsmDialect() const;
>   bool isInsertSubreg() const {
> 
> Modified: llvm/trunk/lib/CodeGen/SelectionDAG/SelectionDAGISel.cpp
> URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/CodeGen/SelectionDAG/SelectionDAGISel.cpp?rev=175334&r1=175333&r2=175334&view=diff
> ==============================================================================
> --- llvm/trunk/lib/CodeGen/SelectionDAG/SelectionDAGISel.cpp (original)
> +++ llvm/trunk/lib/CodeGen/SelectionDAG/SelectionDAGISel.cpp Fri Feb 15 19:25:28 2013
> @@ -441,25 +441,28 @@ bool SelectionDAGISel::runOnMachineFunct
>   }
> 
>   // Determine if there are any calls in this machine function.
> +  MF->setHasMSInlineAsm(false);
>   MachineFrameInfo *MFI = MF->getFrameInfo();
> -  if (!MFI->hasCalls()) {
> -    for (MachineFunction::const_iterator
> -           I = MF->begin(), E = MF->end(); I != E; ++I) {
> -      const MachineBasicBlock *MBB = I;
> -      for (MachineBasicBlock::const_iterator
> -             II = MBB->begin(), IE = MBB->end(); II != IE; ++II) {
> -        const MCInstrDesc &MCID = TM.getInstrInfo()->get(II->getOpcode());
> +  for (MachineFunction::const_iterator I = MF->begin(), E = MF->end(); I != E;
> +       ++I) {
> 
> -        if ((MCID.isCall() && !MCID.isReturn()) ||
> -            II->isStackAligningInlineAsm()) {
> -          MFI->setHasCalls(true);
> -          goto done;
> -        }
> +    if (MFI->hasCalls() && MF->hasMSInlineAsm())
> +      break;
> +
> +    const MachineBasicBlock *MBB = I;
> +    for (MachineBasicBlock::const_iterator II = MBB->begin(), IE = MBB->end();
> +         II != IE; ++II) {
> +      const MCInstrDesc &MCID = TM.getInstrInfo()->get(II->getOpcode());
> +      if ((MCID.isCall() && !MCID.isReturn()) ||
> +          II->isStackAligningInlineAsm()) {
> +        MFI->setHasCalls(true);
> +      }
> +      if (II->isMSInlineAsm()) {
> +        MF->setHasMSInlineAsm(true);
>       }
>     }
>   }
> 
> -  done:
>   // Determine if there is a call to setjmp in the machine function.
>   MF->setExposesReturnsTwice(Fn.callsFunctionThatReturnsTwice());
> 
> 
> Modified: llvm/trunk/lib/Target/X86/X86FrameLowering.cpp
> URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Target/X86/X86FrameLowering.cpp?rev=175334&r1=175333&r2=175334&view=diff
> ==============================================================================
> --- llvm/trunk/lib/Target/X86/X86FrameLowering.cpp (original)
> +++ llvm/trunk/lib/Target/X86/X86FrameLowering.cpp Fri Feb 15 19:25:28 2013
> @@ -50,7 +50,7 @@ bool X86FrameLowering::hasFP(const Machi
>   return (MF.getTarget().Options.DisableFramePointerElim(MF) ||
>           RegInfo->needsStackRealignment(MF) ||
>           MFI->hasVarSizedObjects() ||
> -          MFI->isFrameAddressTaken() ||
> +          MFI->isFrameAddressTaken() || MF.hasMSInlineAsm() ||
>           MF.getInfo<X86MachineFunctionInfo>()->getForceFramePointer() ||
>           MMI.callsUnwindInit() || MMI.callsEHReturn());
> }
> 
> Modified: llvm/trunk/test/CodeGen/X86/ms-inline-asm.ll
> URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/test/CodeGen/X86/ms-inline-asm.ll?rev=175334&r1=175333&r2=175334&view=diff
> ==============================================================================
> --- llvm/trunk/test/CodeGen/X86/ms-inline-asm.ll (original)
> +++ llvm/trunk/test/CodeGen/X86/ms-inline-asm.ll Fri Feb 15 19:25:28 2013
> @@ -5,6 +5,7 @@ entry:
>   %0 = tail call i32 asm sideeffect inteldialect "mov eax, $1\0A\09mov $0, eax", "=r,r,~{eax},~{dirflag},~{fpsr},~{flags}"(i32 1) nounwind
>   ret i32 %0
> ; CHECK: t1
> +; CHECK: movl %esp, %ebp
> ; CHECK: {{## InlineAsm Start|#APP}}
> ; CHECK: .intel_syntax
> ; CHECK: mov eax, ecx
> @@ -18,6 +19,7 @@ entry:
>   call void asm sideeffect inteldialect "mov eax, $$1", "~{eax},~{dirflag},~{fpsr},~{flags}"() nounwind
>   ret void
> ; CHECK: t2
> +; CHECK: movl %esp, %ebp
> ; CHECK: {{## InlineAsm Start|#APP}}
> ; CHECK: .intel_syntax
> ; CHECK: mov eax, 1
> @@ -32,6 +34,7 @@ entry:
>   call void asm sideeffect inteldialect "mov eax, DWORD PTR [$0]", "*m,~{eax},~{dirflag},~{fpsr},~{flags}"(i32* %V.addr) nounwind
>   ret void
> ; CHECK: t3
> +; CHECK: movl %esp, %ebp
> ; CHECK: {{## InlineAsm Start|#APP}}
> ; CHECK: .intel_syntax
> ; CHECK: mov eax, DWORD PTR {{[[esp]}}
> @@ -53,6 +56,7 @@ entry:
>   %0 = load i32* %b1, align 4
>   ret i32 %0
> ; CHECK: t18
> +; CHECK: movl %esp, %ebp
> ; CHECK: {{## InlineAsm Start|#APP}}
> ; CHECK: .intel_syntax
> ; CHECK: lea ebx, foo
> @@ -72,6 +76,7 @@ entry:
>   call void asm sideeffect inteldialect "call $0", "r,~{dirflag},~{fpsr},~{flags}"(void ()* @t19_helper) nounwind
>   ret void
> ; CHECK: t19:
> +; CHECK: movl %esp, %ebp
> ; CHECK: movl ${{_?}}t19_helper, %eax
> ; CHECK: {{## InlineAsm Start|#APP}}
> ; CHECK: .intel_syntax
> @@ -90,6 +95,7 @@ entry:
>   %0 = load i32** %res, align 4
>   ret i32* %0
> ; CHECK: t30:
> +; CHECK: movl %esp, %ebp
> ; CHECK: {{## InlineAsm Start|#APP}}
> ; CHECK: .intel_syntax
> ; CHECK: lea edi, dword ptr [{{_?}}results]
> @@ -97,8 +103,8 @@ entry:
> ; CHECK: {{## InlineAsm End|#NO_APP}}
> ; CHECK: {{## InlineAsm Start|#APP}}
> ; CHECK: .intel_syntax
> -; CHECK: mov dword ptr [esp], edi
> +; CHECK: mov dword ptr [ebp - 8], edi
> ; CHECK: .att_syntax
> ; CHECK: {{## InlineAsm End|#NO_APP}}
> -; CHECK: movl (%esp), %eax
> +; CHECK: movl -8(%ebp), %eax
> }
> 
> 
> _______________________________________________
> llvm-commits mailing list
> llvm-commits at cs.uiuc.edu
> http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits





More information about the llvm-commits mailing list