[llvm] r196879 - Revert "Fix miscompile of MS inline assembly with stack realignment"
Reid Kleckner
reid at kleckner.net
Mon Dec 9 21:31:27 PST 2013
Author: rnk
Date: Mon Dec 9 23:31:27 2013
New Revision: 196879
URL: http://llvm.org/viewvc/llvm-project?rev=196879&view=rev
Log:
Revert "Fix miscompile of MS inline assembly with stack realignment"
This reverts commit r196876. Its tests failed on the bots, so I'll
figure it out tomorrow.
Removed:
llvm/trunk/test/CodeGen/X86/inline-asm-stack-realign.ll
llvm/trunk/test/CodeGen/X86/inline-asm-stack-realign2.ll
Modified:
llvm/trunk/include/llvm/CodeGen/MachineFrameInfo.h
llvm/trunk/include/llvm/CodeGen/MachineFunction.h
llvm/trunk/lib/CodeGen/SelectionDAG/SelectionDAGBuilder.cpp
llvm/trunk/lib/CodeGen/SelectionDAG/SelectionDAGISel.cpp
llvm/trunk/lib/MC/MCParser/AsmParser.cpp
llvm/trunk/lib/Target/X86/X86FrameLowering.cpp
llvm/trunk/lib/Target/X86/X86RegisterInfo.cpp
llvm/trunk/test/CodeGen/X86/ms-inline-asm.ll
Modified: llvm/trunk/include/llvm/CodeGen/MachineFrameInfo.h
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/include/llvm/CodeGen/MachineFrameInfo.h?rev=196879&r1=196878&r2=196879&view=diff
==============================================================================
--- llvm/trunk/include/llvm/CodeGen/MachineFrameInfo.h (original)
+++ llvm/trunk/include/llvm/CodeGen/MachineFrameInfo.h Mon Dec 9 23:31:27 2013
@@ -223,10 +223,6 @@ class MachineFrameInfo {
/// Whether the "realign-stack" option is on.
bool RealignOption;
- /// True if the function includes inline assembly that adjusts the stack
- /// pointer.
- bool HasInlineAsmWithSPAdjust;
-
const TargetFrameLowering *getFrameLowering() const;
public:
explicit MachineFrameInfo(const TargetMachine &TM, bool RealignOpt)
@@ -455,10 +451,6 @@ public:
bool hasCalls() const { return HasCalls; }
void setHasCalls(bool V) { HasCalls = V; }
- /// Returns true if the function contains any stack-adjusting inline assembly.
- bool hasInlineAsmWithSPAdjust() const { return HasInlineAsmWithSPAdjust; }
- void setHasInlineAsmWithSPAdjust(bool B) { HasInlineAsmWithSPAdjust = B; }
-
/// getMaxCallFrameSize - Return the maximum size of a call frame that must be
/// allocated for an outgoing function call. This is only available if
/// CallFrameSetup/Destroy pseudo instructions are used by the target, and
Modified: llvm/trunk/include/llvm/CodeGen/MachineFunction.h
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/include/llvm/CodeGen/MachineFunction.h?rev=196879&r1=196878&r2=196879&view=diff
==============================================================================
--- llvm/trunk/include/llvm/CodeGen/MachineFunction.h (original)
+++ llvm/trunk/include/llvm/CodeGen/MachineFunction.h Mon Dec 9 23:31:27 2013
@@ -131,8 +131,8 @@ class MachineFunction {
/// about the control flow of such functions.
bool ExposesReturnsTwice;
- /// True if the function includes any inline assembly.
- bool HasInlineAsm;
+ /// True if the function includes MS-style inline assembly.
+ bool HasMSInlineAsm;
MachineFunction(const MachineFunction &) LLVM_DELETED_FUNCTION;
void operator=(const MachineFunction&) LLVM_DELETED_FUNCTION;
@@ -218,14 +218,15 @@ public:
ExposesReturnsTwice = B;
}
- /// Returns true if the function contains any inline assembly.
- bool hasInlineAsm() const {
- return HasInlineAsm;
+ /// 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 inline assembly.
- void setHasInlineAsm(bool B) {
- HasInlineAsm = B;
+ /// 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
Modified: llvm/trunk/lib/CodeGen/SelectionDAG/SelectionDAGBuilder.cpp
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/CodeGen/SelectionDAG/SelectionDAGBuilder.cpp?rev=196879&r1=196878&r2=196879&view=diff
==============================================================================
--- llvm/trunk/lib/CodeGen/SelectionDAG/SelectionDAGBuilder.cpp (original)
+++ llvm/trunk/lib/CodeGen/SelectionDAG/SelectionDAGBuilder.cpp Mon Dec 9 23:31:27 2013
@@ -851,20 +851,12 @@ void RegsForValue::AddInlineAsmOperands(
SDValue Res = DAG.getTargetConstant(Flag, MVT::i32);
Ops.push_back(Res);
- unsigned SP = TLI.getStackPointerRegisterToSaveRestore();
for (unsigned Value = 0, Reg = 0, e = ValueVTs.size(); Value != e; ++Value) {
unsigned NumRegs = TLI.getNumRegisters(*DAG.getContext(), ValueVTs[Value]);
MVT RegisterVT = RegVTs[Value];
for (unsigned i = 0; i != NumRegs; ++i) {
assert(Reg < Regs.size() && "Mismatch in # registers expected");
- 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);
- }
+ Ops.push_back(DAG.getRegister(Regs[Reg++], RegisterVT));
}
}
}
Modified: llvm/trunk/lib/CodeGen/SelectionDAG/SelectionDAGISel.cpp
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/CodeGen/SelectionDAG/SelectionDAGISel.cpp?rev=196879&r1=196878&r2=196879&view=diff
==============================================================================
--- llvm/trunk/lib/CodeGen/SelectionDAG/SelectionDAGISel.cpp (original)
+++ llvm/trunk/lib/CodeGen/SelectionDAG/SelectionDAGISel.cpp Mon Dec 9 23:31:27 2013
@@ -428,9 +428,7 @@ bool SelectionDAGISel::runOnMachineFunct
SDB->init(GFI, *AA, LibInfo);
- MF->setHasInlineAsm(false);
- MF->getFrameInfo()->setHasInlineAsmWithSPAdjust(false);
-
+ MF->setHasMSInlineAsm(false);
SelectAllBasicBlocks(Fn);
// If the first basic block in the function has live ins that need to be
@@ -514,7 +512,7 @@ bool SelectionDAGISel::runOnMachineFunct
for (MachineFunction::const_iterator I = MF->begin(), E = MF->end(); I != E;
++I) {
- if (MFI->hasCalls() && MF->hasInlineAsm())
+ if (MFI->hasCalls() && MF->hasMSInlineAsm())
break;
const MachineBasicBlock *MBB = I;
@@ -525,8 +523,8 @@ bool SelectionDAGISel::runOnMachineFunct
II->isStackAligningInlineAsm()) {
MFI->setHasCalls(true);
}
- if (II->isInlineAsm()) {
- MF->setHasInlineAsm(true);
+ if (II->isMSInlineAsm()) {
+ MF->setHasMSInlineAsm(true);
}
}
}
Modified: llvm/trunk/lib/MC/MCParser/AsmParser.cpp
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/MC/MCParser/AsmParser.cpp?rev=196879&r1=196878&r2=196879&view=diff
==============================================================================
--- llvm/trunk/lib/MC/MCParser/AsmParser.cpp (original)
+++ llvm/trunk/lib/MC/MCParser/AsmParser.cpp Mon Dec 9 23:31:27 2013
@@ -4208,11 +4208,6 @@ bool AsmParser::parseMSInlineAsm(
AsmStrRewrites.push_back(AsmRewrite(AOK_Input, Start, SymName.size()));
}
}
-
- // Consider implicit defs to be clobbers. Think of cpuid and push.
- const uint16_t *ImpDefs = Desc.getImplicitDefs();
- for (unsigned I = 0, E = Desc.getNumImplicitDefs(); I != E; ++I)
- ClobberRegs.push_back(ImpDefs[I]);
}
// Set the number of Outputs and Inputs.
Modified: llvm/trunk/lib/Target/X86/X86FrameLowering.cpp
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Target/X86/X86FrameLowering.cpp?rev=196879&r1=196878&r2=196879&view=diff
==============================================================================
--- llvm/trunk/lib/Target/X86/X86FrameLowering.cpp (original)
+++ llvm/trunk/lib/Target/X86/X86FrameLowering.cpp Mon Dec 9 23:31:27 2013
@@ -50,7 +50,7 @@ bool X86FrameLowering::hasFP(const Machi
return (MF.getTarget().Options.DisableFramePointerElim(MF) ||
RegInfo->needsStackRealignment(MF) ||
MFI->hasVarSizedObjects() ||
- MFI->isFrameAddressTaken() || MFI->hasInlineAsmWithSPAdjust() ||
+ MFI->isFrameAddressTaken() || MF.hasMSInlineAsm() ||
MF.getInfo<X86MachineFunctionInfo>()->getForceFramePointer() ||
MMI.callsUnwindInit() || MMI.callsEHReturn());
}
Modified: llvm/trunk/lib/Target/X86/X86RegisterInfo.cpp
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Target/X86/X86RegisterInfo.cpp?rev=196879&r1=196878&r2=196879&view=diff
==============================================================================
--- llvm/trunk/lib/Target/X86/X86RegisterInfo.cpp (original)
+++ llvm/trunk/lib/Target/X86/X86RegisterInfo.cpp Mon Dec 9 23:31:27 2013
@@ -347,12 +347,6 @@ BitVector X86RegisterInfo::getReservedRe
"Stack realignment in presence of dynamic allocas is not supported with"
"this calling convention.");
- // FIXME: Do a proper analysis of the inline asm to see if it actually
- // conflicts with the base register we chose.
- if (MF.hasInlineAsm())
- report_fatal_error("Stack realignment in presence of dynamic stack "
- "adjustments is not supported with inline assembly.");
-
for (MCSubRegIterator I(getBaseRegister(), this, /*IncludeSelf=*/true);
I.isValid(); ++I)
Reserved.set(*I);
@@ -409,15 +403,18 @@ bool X86RegisterInfo::hasBasePointer(con
if (!EnableBasePointer)
return false;
- // When we need stack realignment, we can't address the stack from the frame
- // pointer. When we have dynamic allocas or stack-adjusting inline asm, we
- // can't address variables from the stack pointer. MS inline asm can
- // reference locals while also adjusting the stack pointer. When we can't
- // use both the SP and the FP, we need a separate base pointer register.
- bool CantUseFP = needsStackRealignment(MF);
- bool CantUseSP =
- MFI->hasVarSizedObjects() || MFI->hasInlineAsmWithSPAdjust();
- return CantUseFP && CantUseSP;
+ // When we need stack realignment and there are dynamic allocas, we can't
+ // reference off of the stack pointer, so we reserve a base pointer.
+ //
+ // This is also true if the function contain MS-style inline assembly. We
+ // do this because if any stack changes occur in the inline assembly, e.g.,
+ // "pusha", then any C local variable or C argument references in the
+ // inline assembly will be wrong because the SP is not properly tracked.
+ if ((needsStackRealignment(MF) && MFI->hasVarSizedObjects()) ||
+ MF.hasMSInlineAsm())
+ return true;
+
+ return false;
}
bool X86RegisterInfo::canRealignStack(const MachineFunction &MF) const {
Removed: llvm/trunk/test/CodeGen/X86/inline-asm-stack-realign.ll
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/test/CodeGen/X86/inline-asm-stack-realign.ll?rev=196878&view=auto
==============================================================================
--- llvm/trunk/test/CodeGen/X86/inline-asm-stack-realign.ll (original)
+++ llvm/trunk/test/CodeGen/X86/inline-asm-stack-realign.ll (removed)
@@ -1,16 +0,0 @@
-; RUN: not llc -march x86 < %s 2>&1 | FileCheck %s
-
-; We don't currently support realigning the stack and adjusting the stack
-; pointer in inline asm. This commonly happens in MS inline assembly using
-; push and pop.
-
-; CHECK: Stack realignment in presence of dynamic stack adjustments is not supported with inline assembly
-
-define i32 @foo() {
-entry:
- %r = alloca i32, align 16
- store i32 -1, i32* %r, align 16
- call void asm sideeffect inteldialect "push esi\0A\09xor esi, esi\0A\09mov dword ptr $0, esi\0A\09pop esi", "=*m,~{flags},~{esi},~{esp},~{dirflag},~{fpsr},~{flags}"(i32* %r)
- %0 = load i32* %r, align 16
- ret i32 %0
-}
Removed: llvm/trunk/test/CodeGen/X86/inline-asm-stack-realign2.ll
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/test/CodeGen/X86/inline-asm-stack-realign2.ll?rev=196878&view=auto
==============================================================================
--- llvm/trunk/test/CodeGen/X86/inline-asm-stack-realign2.ll (original)
+++ llvm/trunk/test/CodeGen/X86/inline-asm-stack-realign2.ll (removed)
@@ -1,15 +0,0 @@
-; RUN: not llc -march x86 < %s 2>&1 | FileCheck %s
-
-; We don't currently support realigning the stack and adjusting the stack
-; pointer in inline asm. This can even happen in GNU asm.
-
-; CHECK: Stack realignment in presence of dynamic stack adjustments is not supported with inline assembly
-
-define i32 @foo() {
-entry:
- %r = alloca i32, align 16
- store i32 -1, i32* %r, align 16
- call void asm sideeffect "push %esi\0A\09xor %esi, %esi\0A\09mov %esi, $0\0A\09pop %esi", "=*m,~{flags},~{esi},~{esp},~{dirflag},~{fpsr},~{flags}"(i32* %r)
- %0 = load i32* %r, align 16
- ret i32 %0
-}
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=196879&r1=196878&r2=196879&view=diff
==============================================================================
--- llvm/trunk/test/CodeGen/X86/ms-inline-asm.ll (original)
+++ llvm/trunk/test/CodeGen/X86/ms-inline-asm.ll Mon Dec 9 23:31:27 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-LABEL: 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-LABEL: t30:
+; CHECK: movl %esp, %ebp
; CHECK: {{## InlineAsm Start|#APP}}
; CHECK: .intel_syntax
; CHECK: lea edi, dword ptr [{{_?}}results]
@@ -97,31 +103,8 @@ entry:
; CHECK: {{## InlineAsm End|#NO_APP}}
; CHECK: {{## InlineAsm Start|#APP}}
; CHECK: .intel_syntax
-; CHECK: mov dword ptr [esp], edi
-; CHECK: .att_syntax
-; CHECK: {{## InlineAsm End|#NO_APP}}
-; CHECK: movl (%esp), %eax
-}
-
-; Stack realignment plus MS inline asm that does *not* adjust the stack is no
-; longer an error.
-
-define i32 @t31() {
-entry:
- %val = alloca i32, align 16
- store i32 -1, i32* %val, align 16
- call void asm sideeffect inteldialect "mov dword ptr $0, esp", "=*m,~{dirflag},~{fpsr},~{flags}"(i32* %val) #1
- %sp = load i32* %val, align 16
- ret i32 %sp
-; CHECK-LABEL: t31:
-; CHECK: pushl %ebp
-; CHECK: movl %esp, %ebp
-; CHECK: andl $-16, %esp
-; CHECK: {{## InlineAsm Start|#APP}}
-; CHECK: .intel_syntax
-; CHECK: mov dword ptr [esp], esp
+; CHECK: mov dword ptr [esi], edi
; CHECK: .att_syntax
; CHECK: {{## InlineAsm End|#NO_APP}}
-; CHECK: movl (%esp), %eax
-; CHECK: ret
+; CHECK: movl (%esi), %eax
}
More information about the llvm-commits
mailing list