[llvm] r175334 - [ms-inline asm] Do not omit the frame pointer if we have ms-inline assembly.
Chad Rosier
mcrosier at apple.com
Mon Feb 18 11:25:49 PST 2013
On Feb 17, 2013, at 6:05 AM, Benjamin Kramer <benny.kra at gmail.com> wrote:
>
> 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?
It is now, but the long-term plan is to support multiple ISAs, so I decided to use a target independent representation.
> Feels weird to add yet another flag to MachineFunction for this.
I don't disagree, so..
If I were to change this to using the setForceFramePointer function, do you have a suggestion where I to call it?
Lower formal arguments looks like a good place, but I don't want to have to iterate over the ISDAG. I don't believe the instruction have been lowered from ISD Nodes to MIs at that point, so I can't just iterate as was done in the original solution.
Suggestions welcome.
> This commit also caused a ton of valgrind failures: http://lab.llvm.org:8011/builders/llvm-x86_64-linux-vg_leak/builds/328
I believe this was fixed in r175422.
Chad
>
> 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