<html><head><meta http-equiv="Content-Type" content="text/html charset=iso-8859-1"></head><body style="word-wrap: break-word; -webkit-nbsp-mode: space; -webkit-line-break: after-white-space;">Chad is the best bet, if he has the time to look.<div><br></div><div>I vaguely recall there was a reason for being extra-conservative about the base pointer. Chad, do you remember?</div><div><br></div><div>-Jim</div><div><br><div><div>On Aug 26, 2013, at 1:20 PM, Reid Kleckner <<a href="mailto:rnk@google.com">rnk@google.com</a>> wrote:</div><br class="Apple-interchange-newline"><blockquote type="cite"><div dir="ltr">Jim, can you suggest a reviewer maybe?</div><div class="gmail_extra"><br><br><div class="gmail_quote">On Thu, Aug 22, 2013 at 9:58 AM, Reid Kleckner <span dir="ltr"><<a href="mailto:rnk@google.com" target="_blank">rnk@google.com</a>></span> wrote:<br>
<blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div dir="ltr">Ping.</div><div class="HOEnZb"><div class="h5"><div class="gmail_extra"><br><br><div class="gmail_quote">
On Fri, Aug 16, 2013 at 3:08 PM, Reid Kleckner <span dir="ltr"><<a href="mailto:rnk@google.com" target="_blank">rnk@google.com</a>></span> wrote:<br>
<blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div dir="ltr">Jakob, can you take a look at this?<div><br></div><div>Unfortunately, the fatal error I added actually comes up far more often than I expected because LLVM will try to align stack frames for code like:</div>
<div><br></div><div>void foo() {</div><div> struct { int x; int y; } p; // LLVM wants this to be 8 byte aligned. Why?</div><div> __asm { ... adjust esp and use p }<br>}</div><div><br></div><div>I'd prefer it if LLVM failed instead of miscompiling this code, though.</div>
<div><br></div><div>I wasn't able to do a complete fix to pick the base register dynamically because I can't figure out how to pull the constraints off inline asm in MI.</div><div><br></div><div>I would also reject gcc inline asm that clobbers esp and in the presence of stack realignment if I could see the constraints.</div>
</div><div><div class="gmail_extra"><br><br><div class="gmail_quote">On Tue, Aug 13, 2013 at 6:09 PM, Reid Kleckner <span dir="ltr"><<a href="mailto:rnk@google.com" target="_blank">rnk@google.com</a>></span> wrote:<br>
<blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
- Add a fatal error.<br>
<br>
<a href="http://llvm-reviews.chandlerc.com/D1317" target="_blank">http://llvm-reviews.chandlerc.com/D1317</a><br>
<br>
CHANGE SINCE LAST DIFF<br>
<a href="http://llvm-reviews.chandlerc.com/D1317?vs=3269&id=3460#toc" target="_blank">http://llvm-reviews.chandlerc.com/D1317?vs=3269&id=3460#toc</a><br>
<br>
Files:<br>
lib/Target/X86/X86RegisterInfo.cpp<br>
test/CodeGen/X86/inline-asm-stack-realign.ll<br>
<div> test/CodeGen/X86/ms-inline-asm.ll<br>
<br>
Index: lib/Target/X86/X86RegisterInfo.cpp<br>
===================================================================<br>
--- lib/Target/X86/X86RegisterInfo.cpp<br>
+++ lib/Target/X86/X86RegisterInfo.cpp<br>
</div>@@ -340,6 +340,11 @@<br>
"Stack realignment in presence of dynamic allocas is not supported with"<br>
"this calling convention.");<br>
<br>
+ if (MF.hasMSInlineAsm())<br>
+ report_fatal_error(<br>
+ "Stack realignment in the presence of stack-adjusting inline assembly "<br>
+ "is not supported");<br>
+<br>
for (MCSubRegIterator I(getBaseRegister(), this, /*IncludeSelf=*/true);<br>
I.isValid(); ++I)<br>
Reserved.set(*I);<br>
@@ -396,18 +401,16 @@<br>
<div> if (!EnableBasePointer)<br>
return false;<br>
<br>
- // When we need stack realignment and there are dynamic allocas, we can't<br>
- // reference off of the stack pointer, so we reserve a base pointer.<br>
- //<br>
- // This is also true if the function contain MS-style inline assembly. We<br>
- // do this because if any stack changes occur in the inline assembly, e.g.,<br>
- // "pusha", then any C local variable or C argument references in the<br>
- // inline assembly will be wrong because the SP is not properly tracked.<br>
- if ((needsStackRealignment(MF) && MFI->hasVarSizedObjects()) ||<br>
- MF.hasMSInlineAsm())<br>
- return true;<br>
-<br>
- return false;<br>
+ // When we need stack realignment, we can't address the stack from the frame<br>
+ // pointer. When we have dynamic allocas or MS inline asm, we can't address<br>
+ // variables from the stack pointer. MS inline asm can reference locals<br>
+ // while also adjusting the stack pointer. When we can't use both the SP and<br>
+ // the FP, we need a separate base pointer register.<br>
</div>+ // FIXME: gcc inline asm can also adjust the stack pointer if it lists esp as<br>
+ // a clobber, but LLVM does not support that.<br>
<div>+ bool CantUseFP = needsStackRealignment(MF);<br>
+ bool CantUseSP = MFI->hasVarSizedObjects() || MF.hasMSInlineAsm();<br>
+ return CantUseFP && CantUseSP;<br>
}<br>
<br>
bool X86RegisterInfo::canRealignStack(const MachineFunction &MF) const {<br>
</div>Index: test/CodeGen/X86/inline-asm-stack-realign.ll<br>
===================================================================<br>
--- /dev/null<br>
+++ test/CodeGen/X86/inline-asm-stack-realign.ll<br>
@@ -0,0 +1,16 @@<br>
+; RUN: not llc -march x86 %s 2>&1 | FileCheck %s<br>
+<br>
+; Realigning the stack and referring to local stack vars through memory in<br>
+; inline asm with this many clobbers is impossible.<br>
+; FIXME: LLVM could do better here by looking at the list of clobbers.<br>
+<br>
+; CHECK: Stack realignment in presence of MS inline asm is not supported<br>
+<br>
+define i32 @foo() {<br>
+entry:<br>
+ %r = alloca i32, align 16<br>
+ store i32 -1, i32* %r, align 16<br>
+ call void asm sideeffect inteldialect "pusha\0A\09xor eax, eax\0A\09xor ebx, ebx\0A\09xor ecx, ecx\0A\09xor edx, edx\0A\09xor esi, esi\0A\09xor edi, edi\0A\09mov dword ptr $0, esi\0A\09popa", "=*m,~{eax},~{ebx},~{ecx},~{edi},~{edx},~{esi},~{dirflag},~{fpsr},~{flags}"(i32* %r) #1<br>
+ %0 = load i32* %r, align 16<br>
+ ret i32 %0<br>
+}<br>
<div>Index: test/CodeGen/X86/ms-inline-asm.ll<br>
===================================================================<br>
--- test/CodeGen/X86/ms-inline-asm.ll<br>
+++ test/CodeGen/X86/ms-inline-asm.ll<br>
@@ -103,8 +103,8 @@<br>
; CHECK: {{## InlineAsm End|#NO_APP}}<br>
; CHECK: {{## InlineAsm Start|#APP}}<br>
; CHECK: .intel_syntax<br>
-; CHECK: mov dword ptr [esi], edi<br>
+; CHECK: mov dword ptr [ebp - 8], edi<br>
; CHECK: .att_syntax<br>
; CHECK: {{## InlineAsm End|#NO_APP}}<br>
-; CHECK: movl (%esi), %eax<br>
+; CHECK: movl -8(%ebp), %eax<br>
}<br>
</div></blockquote></div><br></div>
</div></blockquote></div><br></div>
</div></div></blockquote></div><br></div>
</blockquote></div><br></div></body></html>