<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 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 class="im">  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 class="im">    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 class="im">+   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 class="HOEnZb"><div class="h5">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></div></blockquote></div><br></div>