[PATCH] Include intrin.h before windows.h as a workaround for the x64 self-host

Reid Kleckner rnk at google.com
Mon May 5 17:47:45 PDT 2014


Hi rafael.espindola, majnemer,

On x64, windows.h doesn't include intrin.h for intrinsics.  It just
declares them in the global namespace and uses them, expecting the
compiler to lower it as a builtin.  We basically need to do this in
clang, eventually.

Allow sret on the second parameter as well as the first

MSVC always places the implicit sret parameter after the implicit this
parameter of instance methods.  We used to handle this for
x86_thiscallcc by allocating the sret parameter on the stack and leaving
the this pointer in ecx, but that doesn't handle alternative calling
conventions like cdecl, stdcall, fastcall, or the win64 convention.

Instead, change the verifier to allow sret on the second parameter.

This also requires changing the X86 backend to return the argument with
the sret parameter, instead of assuming that the sret parameter comes
first.

http://reviews.llvm.org/D3617

Files:
  include/llvm/IR/Function.h
  lib/IR/Verifier.cpp
  lib/Support/Atomic.cpp
  lib/Target/X86/X86ISelLowering.cpp
  test/CodeGen/X86/win32_sret.ll

Index: include/llvm/IR/Function.h
===================================================================
--- include/llvm/IR/Function.h
+++ include/llvm/IR/Function.h
@@ -298,7 +298,8 @@
   /// @brief Determine if the function returns a structure through first
   /// pointer argument.
   bool hasStructRetAttr() const {
-    return AttributeSets.hasAttribute(1, Attribute::StructRet);
+    return AttributeSets.hasAttribute(1, Attribute::StructRet) ||
+           AttributeSets.hasAttribute(2, Attribute::StructRet);
   }
 
   /// @brief Determine if the parameter does not alias other parameters.
Index: lib/IR/Verifier.cpp
===================================================================
--- lib/IR/Verifier.cpp
+++ lib/IR/Verifier.cpp
@@ -853,7 +853,8 @@
     }
 
     if (Attrs.hasAttribute(Idx, Attribute::StructRet))
-      Assert1(Idx == 1, "Attribute sret is not on first parameter!", V);
+      Assert1(Idx == 1 || Idx == 2,
+              "Attribute sret is not on first or second parameter!", V);
 
     if (Attrs.hasAttribute(Idx, Attribute::InAlloca)) {
       Assert1(Idx == FT->getNumParams(),
Index: lib/Support/Atomic.cpp
===================================================================
--- lib/Support/Atomic.cpp
+++ lib/Support/Atomic.cpp
@@ -17,6 +17,7 @@
 using namespace llvm;
 
 #if defined(_MSC_VER)
+#include <Intrin.h>
 #include <windows.h>
 #undef MemoryFence
 #endif
Index: lib/Target/X86/X86ISelLowering.cpp
===================================================================
--- lib/Target/X86/X86ISelLowering.cpp
+++ lib/Target/X86/X86ISelLowering.cpp
@@ -2302,7 +2302,12 @@
       Reg = MF.getRegInfo().createVirtualRegister(getRegClassFor(PtrTy));
       FuncInfo->setSRetReturnReg(Reg);
     }
-    SDValue Copy = DAG.getCopyToReg(DAG.getEntryNode(), dl, Reg, InVals[0]);
+    size_t I = 0, E = MF.getFunction()->arg_size();
+    AttributeSet Attrs = MF.getFunction()->getAttributes();
+    for (; I != E; ++I)
+      if (Attrs.hasAttribute(I + 1, Attribute::StructRet))
+        break;
+    SDValue Copy = DAG.getCopyToReg(DAG.getEntryNode(), dl, Reg, InVals[I]);
     Chain = DAG.getNode(ISD::TokenFactor, dl, MVT::Other, Copy, Chain);
   }
 
Index: test/CodeGen/X86/win32_sret.ll
===================================================================
--- test/CodeGen/X86/win32_sret.ll
+++ test/CodeGen/X86/win32_sret.ll
@@ -181,3 +181,30 @@
   ret void
 }
 declare x86_thiscallcc void @test6_g(%struct.test6* sret, %struct.test6*)
+
+; Flipping the parameters at the IR level generates the same code.
+%struct.test7 = type { i32, i32, i32 }
+define void @test7_f(%struct.test7* %x) nounwind {
+; WIN32-LABEL: _test7_f:
+; MINGW_X86-LABEL: _test7_f:
+; CYGWIN-LABEL: _test7_f:
+; LINUX-LABEL: test7_f:
+
+; The %x argument is moved to %ecx on all OSs. It will be the this pointer.
+; WIN32:      movl    8(%ebp), %ecx
+; MINGW_X86:  movl    8(%ebp), %ecx
+; CYGWIN:     movl    8(%ebp), %ecx
+
+; The sret pointer is (%esp)
+; WIN32:          leal    8(%esp), %[[REG:e[a-d]x]]
+; WIN32-NEXT:     movl    %[[REG]], (%e{{([a-d]x)|(sp)}})
+; MINGW_X86:      leal    8(%esp), %[[REG:e[a-d]x]]
+; MINGW_X86-NEXT: movl    %[[REG]], (%e{{([a-d]x)|(sp)}})
+; CYGWIN:         leal    8(%esp), %[[REG:e[a-d]x]]
+; CYGWIN-NEXT:    movl    %[[REG]], (%e{{([a-d]x)|(sp)}})
+
+  %tmp = alloca %struct.test7, align 4
+  call x86_thiscallcc void @test7_g(%struct.test7* %x, %struct.test7* sret %tmp)
+  ret void
+}
+declare x86_thiscallcc void @test7_g(%struct.test7*, %struct.test7* sret)
-------------- next part --------------
A non-text attachment was scrubbed...
Name: D3617.9091.patch
Type: text/x-patch
Size: 3512 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20140506/eed731d6/attachment.bin>


More information about the llvm-commits mailing list