MS ABI: Pass 'sret' as the second parameter of instance methods

[rnk]

Summary

MSVC always passes 'sret' after 'this', unlike GCC. This required
changing a number of places in Clang that assumed the sret parameter was
always first in LLVM IR.

This fixes win64 MSVC ABI compatibility for methods returning structs.

[majnemer]

LGTM

[rnk]

Closed by commit rL208458 (authored by @rnk).

cfe/trunk/include/clang/CodeGen/CGFunctionInfo.h

private:
   llvm::Type *PaddingType;
   union {
     unsigned DirectOffset;     // isDirect() || isExtend()
     unsigned IndirectAlign;    // isIndirect()
     unsigned AllocaFieldIndex; // isInAlloca()
   };
   Kind TheKind;
   bool PaddingInReg : 1;
-  bool InAllocaSRet : 1;    // isInAlloca
+  bool InAllocaSRet : 1;    // isInAlloca()
   bool IndirectByVal : 1;   // isIndirect()
   bool IndirectRealign : 1; // isIndirect()
+  bool SRetAfterThis : 1;   // isIndirect()
   bool InReg : 1;           // isDirect() || isExtend() || isIndirect()
 
   ABIArgInfo(Kind K)
       : PaddingType(nullptr), TheKind(K), PaddingInReg(false), InReg(false) {}
 
 public:
   ABIArgInfo()
       : TypeData(nullptr), PaddingType(nullptr), DirectOffset(0),
public:
   }
   static ABIArgInfo getIndirect(unsigned Alignment, bool ByVal = true,
                                 bool Realign = false,
                                 llvm::Type *Padding = nullptr) {
     auto AI = ABIArgInfo(Indirect);
     AI.setIndirectAlign(Alignment);
     AI.setIndirectByVal(ByVal);
     AI.setIndirectRealign(Realign);
+    AI.setSRetAfterThis(false);
     AI.setPaddingType(Padding);
     return AI;
   }
   static ABIArgInfo getIndirectInReg(unsigned Alignment, bool ByVal = true,
                                      bool Realign = false) {
     auto AI = getIndirect(Alignment, ByVal, Realign);
     AI.setInReg(true);
     return AI;
bool getIndirectRealign() const {
     assert(isIndirect() && "Invalid kind!");
     return IndirectRealign;
   }
   void setIndirectRealign(bool IR) {
     assert(isIndirect() && "Invalid kind!");
     IndirectRealign = IR;
   }
 
+  bool isSRetAfterThis() const {
+    assert(isIndirect() && "Invalid kind!");
+    return SRetAfterThis;
+  }
+  void setSRetAfterThis(bool AfterThis) {
+    assert(isIndirect() && "Invalid kind!");
+    SRetAfterThis = AfterThis;
+  }
+
   unsigned getInAllocaFieldIndex() const {
     assert(isInAlloca() && "Invalid kind!");
     return AllocaFieldIndex;
   }
   void setInAllocaFieldIndex(unsigned FieldIndex) {
     assert(isInAlloca() && "Invalid kind!");
     AllocaFieldIndex = FieldIndex;
   }

cfe/trunk/lib/CodeGen/CGCXXABI.h

enum RecordArgABI {
 
     /// Pass it as a pointer to temporary memory.
     RAA_Indirect
   };
 
   /// Returns how an argument of the given record type should be passed.
   virtual RecordArgABI getRecordArgABI(const CXXRecordDecl *RD) const = 0;
 
+  /// Returns true if the implicit 'sret' parameter comes after the implicit
+  /// 'this' parameter of C++ instance methods.
+  virtual bool isSRetParameterAfterThis() const { return false; }
+
   /// Find the LLVM type used to represent the given member pointer
   /// type.
   virtual llvm::Type *
   ConvertMemberPointerType(const MemberPointerType *MPT);
 
   /// Load a member function from an object and a member function
   /// pointer.  Apply the this-adjustment and set 'This' to the
   /// adjusted value.

cfe/trunk/lib/CodeGen/CGCall.cpp

 }
 
 llvm::FunctionType *
 CodeGenTypes::GetFunctionType(const CGFunctionInfo &FI) {
   
   bool Inserted = FunctionsBeingProcessed.insert(&FI); (void)Inserted;
   assert(Inserted && "Recursively being processed?");
   
+  bool SwapThisWithSRet = false;
   SmallVector<llvm::Type*, 8> argTypes;
   llvm::Type *resultType = 0;
 
   const ABIArgInfo &retAI = FI.getReturnInfo();
   switch (retAI.getKind()) {
   case ABIArgInfo::Expand:
     llvm_unreachable("Invalid ABI kind for return argument");
 
CodeGenTypes::GetFunctionType(const CGFunctionInfo &FI) {
   case ABIArgInfo::Indirect: {
     assert(!retAI.getIndirectAlign() && "Align unused on indirect return.");
     resultType = llvm::Type::getVoidTy(getLLVMContext());
 
     QualType ret = FI.getReturnType();
     llvm::Type *ty = ConvertType(ret);
     unsigned addressSpace = Context.getTargetAddressSpace(ret);
     argTypes.push_back(llvm::PointerType::get(ty, addressSpace));
+
+    SwapThisWithSRet = retAI.isSRetAfterThis();
     break;
   }
 
   case ABIArgInfo::Ignore:
     resultType = llvm::Type::getVoidTy(getLLVMContext());
     break;
   }
 
case ABIArgInfo::Expand:
       break;
     }
   }
 
   // Add the inalloca struct as the last parameter type.
   if (llvm::StructType *ArgStruct = FI.getArgStruct())
     argTypes.push_back(ArgStruct->getPointerTo());
 
+  if (SwapThisWithSRet)
+    std::swap(argTypes[0], argTypes[1]);
+
   bool Erased = FunctionsBeingProcessed.erase(&FI); (void)Erased;
   assert(Erased && "Not in set?");
   
   return llvm::FunctionType::get(resultType, argTypes, FI.isVariadic());
 }
 
 llvm::Type *CodeGenTypes::GetFunctionTypeForVTable(GlobalDecl GD) {
   const CXXMethodDecl *MD = cast<CXXMethodDecl>(GD.getDecl());
FuncAttrs.addAttribute("stack-protector-buffer-size",
                            llvm::utostr(CodeGenOpts.SSPBufferSize));
 
     if (!CodeGenOpts.StackRealignment)
       FuncAttrs.addAttribute("no-realign-stack");
   }
 
   QualType RetTy = FI.getReturnType();
   unsigned Index = 1;
+  bool SwapThisWithSRet = false;
   const ABIArgInfo &RetAI = FI.getReturnInfo();
   switch (RetAI.getKind()) {
   case ABIArgInfo::Extend:
     if (RetTy->hasSignedIntegerRepresentation())
       RetAttrs.addAttribute(llvm::Attribute::SExt);
     else if (RetTy->hasUnsignedIntegerRepresentation())
       RetAttrs.addAttribute(llvm::Attribute::ZExt);
     // FALL THROUGH
case ABIArgInfo::InAlloca: {
     break;
   }
 
   case ABIArgInfo::Indirect: {
     llvm::AttrBuilder SRETAttrs;
     SRETAttrs.addAttribute(llvm::Attribute::StructRet);
     if (RetAI.getInReg())
       SRETAttrs.addAttribute(llvm::Attribute::InReg);
-    PAL.push_back(llvm::
-                  AttributeSet::get(getLLVMContext(), Index, SRETAttrs));
+    SwapThisWithSRet = RetAI.isSRetAfterThis();
+    PAL.push_back(llvm::AttributeSet::get(
+        getLLVMContext(), SwapThisWithSRet ? 2 : Index, SRETAttrs));
 
+    if (!SwapThisWithSRet)
       ++Index;
     // sret disables readnone and readonly
     FuncAttrs.removeAttribute(llvm::Attribute::ReadOnly)
       .removeAttribute(llvm::Attribute::ReadNone);
     break;
   }
 
   case ABIArgInfo::Expand:
     llvm_unreachable("Invalid ABI kind for return argument");
   }
 
   if (RetAttrs.hasAttributes())
     PAL.push_back(llvm::
                   AttributeSet::get(getLLVMContext(),
                                     llvm::AttributeSet::ReturnIndex,
                                     RetAttrs));
 
   for (const auto &I : FI.arguments()) {
     QualType ParamType = I.type;
     const ABIArgInfo &AI = I.info;
     llvm::AttrBuilder Attrs;
 
+    // Skip over the sret parameter when it comes second.  We already handled it
+    // above.
+    if (Index == 2 && SwapThisWithSRet)
+      ++Index;
+
     if (AI.getPaddingType()) {
       if (AI.getPaddingInReg())
         PAL.push_back(llvm::AttributeSet::get(getLLVMContext(), Index,
                                               llvm::Attribute::InReg));
       // Increment Index if there is padding.
       ++Index;
     }
 
void CodeGenFunction::EmitFunctionProlog(const CGFunctionInfo &FI,
   llvm::Value *ArgStruct = 0;
   if (FI.usesInAlloca()) {
     llvm::Function::arg_iterator EI = Fn->arg_end();
     --EI;
     ArgStruct = EI;
     assert(ArgStruct->getType() == FI.getArgStruct()->getPointerTo());
   }
 
-  // Name the struct return argument.
-  if (CGM.ReturnTypeUsesSRet(FI)) {
+  // Name the struct return parameter, which can come first or second.
+  const ABIArgInfo &RetAI = FI.getReturnInfo();
+  bool SwapThisWithSRet = false;
+  if (RetAI.isIndirect()) {
+    SwapThisWithSRet = RetAI.isSRetAfterThis();
+    if (SwapThisWithSRet)
+      ++AI;
     AI->setName("agg.result");
-    AI->addAttr(llvm::AttributeSet::get(getLLVMContext(),
-                                        AI->getArgNo() + 1,
+    AI->addAttr(llvm::AttributeSet::get(getLLVMContext(), AI->getArgNo() + 1,
                                         llvm::Attribute::NoAlias));
-    ++AI;
+    if (SwapThisWithSRet)
+      --AI;  // Go back to the beginning for 'this'.
+    else
+      ++AI;  // Skip the sret parameter.
   }
 
   // Track if we received the parameter as a pointer (indirect, byval, or
   // inalloca).  If already have a pointer, EmitParmDecl doesn't need to copy it
   // into a local alloca for us.
   enum ValOrPointer { HaveValue = 0, HavePointer = 1 };
   typedef llvm::PointerIntPair<llvm::Value *, 1> ValueAndIsPtr;
   SmallVector<ValueAndIsPtr, 16> ArgVals;
case ABIArgInfo::Ignore:
         ArgVals.push_back(ValueAndIsPtr(U, HaveValue));
       }
 
       // Skip increment, no matching LLVM parameter.
       continue;
     }
 
     ++AI;
+
+    if (ArgNo == 1 && SwapThisWithSRet)
+      ++AI;  // Skip the sret parameter.
   }
 
   if (FI.usesInAlloca())
     ++AI;
   assert(AI == Fn->arg_end() && "Argument mismatch!");
 
   if (getTarget().getCXXABI().areArgsDestroyedLeftToRightInCallee()) {
     for (int I = Args.size() - 1; I >= 0; --I)
if (RetAI.getInAllocaSRet()) {
       llvm::Value *ArgStruct = EI;
       llvm::Value *SRet =
           Builder.CreateStructGEP(ArgStruct, RetAI.getInAllocaFieldIndex());
       RV = Builder.CreateLoad(SRet, "sret");
     }
     break;
 
   case ABIArgInfo::Indirect: {
+    auto AI = CurFn->arg_begin();
+    if (RetAI.isSRetAfterThis())
+      ++AI;
     switch (getEvaluationKind(RetTy)) {
     case TEK_Complex: {
       ComplexPairTy RT =
         EmitLoadOfComplex(MakeNaturalAlignAddrLValue(ReturnValue, RetTy),
                           EndLoc);
-      EmitStoreOfComplex(RT,
-                       MakeNaturalAlignAddrLValue(CurFn->arg_begin(), RetTy),
+      EmitStoreOfComplex(RT, MakeNaturalAlignAddrLValue(AI, RetTy),
                          /*isInit*/ true);
       break;
     }
     case TEK_Aggregate:
       // Do nothing; aggregrates get evaluated directly into the destination.
       break;
     case TEK_Scalar:
       EmitStoreOfScalar(Builder.CreateLoad(ReturnValue),
-                        MakeNaturalAlignAddrLValue(CurFn->arg_begin(), RetTy),
+                        MakeNaturalAlignAddrLValue(AI, RetTy),
                         /*isInit*/ true);
       break;
     }
     break;
   }
 
   case ABIArgInfo::Extend:
   case ABIArgInfo::Direct:
if (llvm::StructType *ArgStruct = CallInfo.getArgStruct()) {
     AI->setUsedWithInAlloca(true);
     assert(AI->isUsedWithInAlloca() && !AI->isStaticAlloca());
     ArgMemory = AI;
   }
 
   // If the call returns a temporary with struct return, create a temporary
   // alloca to hold the result, unless one is given to us.
   llvm::Value *SRetPtr = 0;
-  if (CGM.ReturnTypeUsesSRet(CallInfo) || RetAI.isInAlloca()) {
+  bool SwapThisWithSRet = false;
+  if (RetAI.isIndirect() || RetAI.isInAlloca()) {
     SRetPtr = ReturnValue.getValue();
     if (!SRetPtr)
       SRetPtr = CreateMemTemp(RetTy);
-    if (CGM.ReturnTypeUsesSRet(CallInfo)) {
+    if (RetAI.isIndirect()) {
       Args.push_back(SRetPtr);
+      SwapThisWithSRet = RetAI.isSRetAfterThis();
+      if (SwapThisWithSRet)
+        IRArgNo = 1;
       checkArgMatches(SRetPtr, IRArgNo, IRFuncTy);
+      if (SwapThisWithSRet)
+        IRArgNo = 0;
     } else {
       llvm::Value *Addr =
           Builder.CreateStructGEP(ArgMemory, RetAI.getInAllocaFieldIndex());
       Builder.CreateStore(SRetPtr, Addr);
     }
   }
 
   assert(CallInfo.arg_size() == CallArgs.size() &&
          "Mismatch between function signature & arguments.");
   CGFunctionInfo::const_arg_iterator info_it = CallInfo.arg_begin();
   for (CallArgList::const_iterator I = CallArgs.begin(), E = CallArgs.end();
        I != E; ++I, ++info_it) {
     const ABIArgInfo &ArgInfo = info_it->info;
     RValue RV = I->RV;
 
+    // Skip 'sret' if it came second.
+    if (IRArgNo == 1 && SwapThisWithSRet)
+      ++IRArgNo;
+
     CharUnits TypeAlign = getContext().getTypeAlignInChars(I->Ty);
 
     // Insert a padding argument to ensure proper alignment.
     if (llvm::Type *PaddingType = ArgInfo.getPaddingType()) {
       Args.push_back(llvm::UndefValue::get(PaddingType));
       ++IRArgNo;
     }
 
for (CallArgList::const_iterator I = CallArgs.begin(), E = CallArgs.end();
 
     case ABIArgInfo::Expand:
       ExpandTypeToArgs(I->Ty, RV, Args, IRFuncTy);
       IRArgNo = Args.size();
       break;
     }
   }
 
+  if (SwapThisWithSRet)
+    std::swap(Args[0], Args[1]);
+
   if (ArgMemory) {
     llvm::Value *Arg = ArgMemory;
     llvm::Type *LastParamTy =
         IRFuncTy->getParamType(IRFuncTy->getNumParams() - 1);
     if (Arg->getType() != LastParamTy) {
 #ifndef NDEBUG
       // Assert that these structs have equivalent element types.
       llvm::StructType *FullTy = CallInfo.getArgStruct();

cfe/trunk/lib/CodeGen/CodeGenFunction.cpp

if (RetTy->isVoidType()) {
 
     // Count the implicit return.
     if (!endsWithReturn(D))
       ++NumReturnExprs;
   } else if (CurFnInfo->getReturnInfo().getKind() == ABIArgInfo::Indirect &&
              !hasScalarEvaluationKind(CurFnInfo->getReturnType())) {
     // Indirect aggregate return; emit returned value directly into sret slot.
     // This reduces code size, and affects correctness in C++.
-    ReturnValue = CurFn->arg_begin();
+    auto AI = CurFn->arg_begin();
+    if (CurFnInfo->getReturnInfo().isSRetAfterThis())
+      ++AI;
+    ReturnValue = AI;
   } else if (CurFnInfo->getReturnInfo().getKind() == ABIArgInfo::InAlloca &&
              !hasScalarEvaluationKind(CurFnInfo->getReturnType())) {
     // Load the sret pointer from the argument struct and return into that.
     unsigned Idx = CurFnInfo->getReturnInfo().getInAllocaFieldIndex();
     llvm::Function::arg_iterator EI = CurFn->arg_end();
     --EI;
     llvm::Value *Addr = Builder.CreateStructGEP(EI, Idx);
     ReturnValue = Builder.CreateLoad(Addr, "agg.result");

cfe/trunk/lib/CodeGen/MicrosoftCXXABI.cpp

public:
 
   bool isReturnTypeIndirect(const CXXRecordDecl *RD) const override {
     // Structures that are not C++03 PODs are always indirect.
     return !RD->isPOD();
   }
 
   RecordArgABI getRecordArgABI(const CXXRecordDecl *RD) const override;
 
+  bool isSRetParameterAfterThis() const override { return true; }
+
   StringRef GetPureVirtualCallName() override { return "_purecall"; }
   // No known support for deleted functions in MSVC yet, so this choice is
   // arbitrary.
   StringRef GetDeletedVirtualCallName() override { return "_purecall"; }
 
   bool isInlineInitializedStaticDataMemberLinkOnce()  override{ return true; }
 
   llvm::Value *adjustToCompleteObject(CodeGenFunction &CGF,

cfe/trunk/lib/CodeGen/TargetInfo.cpp

void X86_32ABIInfo::computeInfo(CGFunctionInfo &FI) const {
   else if (FI.getHasRegParm())
     State.FreeRegs = FI.getRegParm();
   else
     State.FreeRegs = DefaultNumRegisterParameters;
 
   FI.getReturnInfo() =
       classifyReturnType(FI.getReturnType(), State, FI.isInstanceMethod());
 
-  // On win32, use the x86_cdeclmethodcc convention for cdecl methods that use
-  // sret.  This convention swaps the order of the first two parameters behind
-  // the scenes to match MSVC.
+  // On win32, swap the order of the first two parameters for instance methods
+  // which are sret behind the scenes to match MSVC.
   if (IsWin32StructABI && FI.isInstanceMethod() &&
-      FI.getCallingConvention() == llvm::CallingConv::C &&
-      FI.getReturnInfo().isIndirect())
-    FI.setEffectiveCallingConvention(llvm::CallingConv::X86_CDeclMethod);
+      FI.getReturnInfo().isIndirect()) {
+    assert(FI.arg_size() >= 1 && "instance method should have this");
+    FI.getReturnInfo().setSRetAfterThis(true);
+  }
 
   bool UsedInAlloca = false;
   for (auto &I : FI.arguments()) {
     I.info = classifyArgumentType(I.type, State);
     UsedInAlloca |= (I.info.getKind() == ABIArgInfo::InAlloca);
   }
 
   // If we needed to use inalloca for any argument, do a second pass and rewrite
ABIArgInfo WinX86_64ABIInfo::classify(QualType Ty, bool IsReturnType) const {
   return ABIArgInfo::getDirect();
 }
 
 void WinX86_64ABIInfo::computeInfo(CGFunctionInfo &FI) const {
 
   QualType RetTy = FI.getReturnType();
   FI.getReturnInfo() = classify(RetTy, true);
 
+  // On win64, swap the order of the first two parameters for instance methods
+  // which are sret behind the scenes to match MSVC.
+  if (FI.getReturnInfo().isIndirect() && FI.isInstanceMethod() &&
+      getCXXABI().isSRetParameterAfterThis()) {
+    assert(FI.arg_size() >= 1 && "instance method should have this");
+    FI.getReturnInfo().setSRetAfterThis(true);
+  }
+
   for (auto &I : FI.arguments())
     I.info = classify(I.type, false);
 }
 
 llvm::Value *WinX86_64ABIInfo::EmitVAArg(llvm::Value *VAListAddr, QualType Ty,
                                       CodeGenFunction &CGF) const {
   llvm::Type *BPP = CGF.Int8PtrPtrTy;
 

cfe/trunk/test/CodeGenCXX/microsoft-abi-cdecl-method-sret.cpp

struct C {
   S __cdecl byval_and_sret(S a);
   int c;
 };
 
 S C::variadic_sret(const char *f, ...) { return S(); }
 S C::cdecl_sret() { return S(); }
 S C::byval_and_sret(S a) { return S(); }
 
-// CHECK: define x86_cdeclmethodcc void @"\01?variadic_sret@C@@QAA?AUS@@PBDZZ"(%struct.S* noalias sret %agg.result, %struct.C* %this, i8* %f, ...)
-// CHECK: define x86_cdeclmethodcc void @"\01?cdecl_sret@C@@QAA?AUS@@XZ"(%struct.S* noalias sret %agg.result, %struct.C* %this)
-// CHECK: define x86_cdeclmethodcc void @"\01?byval_and_sret@C@@QAA?AUS@@U2@@Z"(%struct.S* noalias sret %agg.result, %struct.C* %this, %struct.S* byval align 4 %a)
+// CHECK: define void @"\01?variadic_sret@C@@QAA?AUS@@PBDZZ"(%struct.C* %this, %struct.S* noalias sret %agg.result, i8* %f, ...)
+// CHECK: define void @"\01?cdecl_sret@C@@QAA?AUS@@XZ"(%struct.C* %this, %struct.S* noalias sret %agg.result)
+// CHECK: define void @"\01?byval_and_sret@C@@QAA?AUS@@U2@@Z"(%struct.C* %this, %struct.S* noalias sret %agg.result, %struct.S* byval align 4 %a)
 
 int main() {
   C c;
   c.variadic_sret("asdf");
   c.cdecl_sret();
   c.byval_and_sret(S());
 }
 // CHECK-LABEL: define i32 @main()
-// CHECK: call x86_cdeclmethodcc void {{.*}} @"\01?variadic_sret@C@@QAA?AUS@@PBDZZ"
-// CHECK: call x86_cdeclmethodcc void @"\01?cdecl_sret@C@@QAA?AUS@@XZ"
-// CHECK: call x86_cdeclmethodcc void @"\01?byval_and_sret@C@@QAA?AUS@@U2@@Z"
+// CHECK: call void {{.*}} @"\01?variadic_sret@C@@QAA?AUS@@PBDZZ"
+// CHECK: call void @"\01?cdecl_sret@C@@QAA?AUS@@XZ"
+// CHECK: call void @"\01?byval_and_sret@C@@QAA?AUS@@U2@@Z"

cfe/trunk/test/CodeGenCXX/microsoft-abi-sret-and-byval.cpp

 // WIN32: define void @"\01?big_arg@@YAXUBig@@@Z"(%struct.Big* byval align 4 %s)
 // WIN64: define void @"\01?big_arg@@YAXUBig@@@Z"(%struct.Big* %s)
 
 // FIXME: Add WIN64 tests. Currently, even the method manglings are wrong (sic!).
 class Class {
  public:
   Small thiscall_method_small() { return Small(); }
   // LINUX: define {{.*}} void @_ZN5Class21thiscall_method_smallEv(%struct.Small* noalias sret %agg.result, %class.Class* %this)
-  // WIN32: define {{.*}} x86_thiscallcc void @"\01?thiscall_method_small@Class@@QAE?AUSmall@@XZ"(%struct.Small* noalias sret %agg.result, %class.Class* %this)
+  // WIN32: define {{.*}} x86_thiscallcc void @"\01?thiscall_method_small@Class@@QAE?AUSmall@@XZ"(%class.Class* %this, %struct.Small* noalias sret %agg.result)
 
   SmallWithCtor thiscall_method_small_with_ctor() { return SmallWithCtor(); }
   // LINUX: define {{.*}} void @_ZN5Class31thiscall_method_small_with_ctorEv(%struct.SmallWithCtor* noalias sret %agg.result, %class.Class* %this)
-  // WIN32: define {{.*}} x86_thiscallcc void @"\01?thiscall_method_small_with_ctor@Class@@QAE?AUSmallWithCtor@@XZ"(%struct.SmallWithCtor* noalias sret %agg.result, %class.Class* %this)
+  // WIN32: define {{.*}} x86_thiscallcc void @"\01?thiscall_method_small_with_ctor@Class@@QAE?AUSmallWithCtor@@XZ"(%class.Class* %this, %struct.SmallWithCtor* noalias sret %agg.result)
 
   Small __cdecl cdecl_method_small() { return Small(); }
   // LINUX: define {{.*}} void @_ZN5Class18cdecl_method_smallEv(%struct.Small* noalias sret %agg.result, %class.Class* %this)
-  // WIN32: define {{.*}} void @"\01?cdecl_method_small@Class@@QAA?AUSmall@@XZ"(%struct.Small* noalias sret %agg.result, %class.Class* %this)
+  // WIN32: define {{.*}} void @"\01?cdecl_method_small@Class@@QAA?AUSmall@@XZ"(%class.Class* %this, %struct.Small* noalias sret %agg.result)
 
   Big __cdecl cdecl_method_big() { return Big(); }
   // LINUX: define {{.*}} void @_ZN5Class16cdecl_method_bigEv(%struct.Big* noalias sret %agg.result, %class.Class* %this)
-  // WIN32: define {{.*}} void @"\01?cdecl_method_big@Class@@QAA?AUBig@@XZ"(%struct.Big* noalias sret %agg.result, %class.Class* %this)
+  // WIN32: define {{.*}} void @"\01?cdecl_method_big@Class@@QAA?AUBig@@XZ"(%class.Class* %this, %struct.Big* noalias sret %agg.result)
 
   void thiscall_method_arg(Empty s) {}
   // LINUX: define {{.*}} void @_ZN5Class19thiscall_method_argE5Empty(%class.Class* %this)
   // WIN32: define {{.*}} void @"\01?thiscall_method_arg@Class@@QAEXUEmpty@@@Z"(%class.Class* %this, %struct.Empty* byval align 4 %s)
 
   void thiscall_method_arg(EmptyWithCtor s) {}
   // LINUX: define {{.*}} void @_ZN5Class19thiscall_method_argE13EmptyWithCtor(%class.Class* %this)
   // WIN32: define {{.*}} void @"\01?thiscall_method_arg@Class@@QAEXUEmptyWithCtor@@@Z"(%class.Class* %this, %struct.EmptyWithCtor* byval align 4 %s)

cfe/trunk/test/CodeGenCXX/microsoft-abi-virtual-member-pointers.cpp

void f() {
   ptr3 = &C::baz;
 
   void (D::*ptr4)();
   ptr4 = &D::foo;
 
 // CHECK32-LABEL: define void @"\01?f@@YAXXZ"()
 // CHECK32: store i8* bitcast (void (%struct.C*)* @"\01??_9C@@$BA@AE" to i8*), i8** %ptr
 // CHECK32: store i8* bitcast (i32 (%struct.C*, i32, double)* @"\01??_9C@@$B3AE" to i8*), i8** %ptr2
-// CHECK32: store i8* bitcast (void (%struct.S*, %struct.C*, i32)* @"\01??_9C@@$B7AE" to i8*), i8** %ptr3
+// CHECK32: store i8* bitcast (void (%struct.C*, %struct.S*, i32)* @"\01??_9C@@$B7AE" to i8*), i8** %ptr3
 // CHECK32: store i8* bitcast (void (%"struct.(anonymous namespace)::D"*)* @"\01??_9D@?A@@$BA@AE" to i8*), i8** %ptr4
 // CHECK32: }
 //
 // CHECK64-LABEL: define void @"\01?f@@YAXXZ"()
 // CHECK64: store i8* bitcast (void (%struct.C*)* @"\01??_9C@@$BA@AA" to i8*), i8** %ptr
 // CHECK64: store i8* bitcast (i32 (%struct.C*, i32, double)* @"\01??_9C@@$B7AA" to i8*), i8** %ptr2
-// CHECK64: store i8* bitcast (void (%struct.S*, %struct.C*, i32)* @"\01??_9C@@$BBA@AA" to i8*), i8** %ptr3
+// CHECK64: store i8* bitcast (void (%struct.C*, %struct.S*, i32)* @"\01??_9C@@$BBA@AA" to i8*), i8** %ptr3
 // CHECK64: store i8* bitcast (void (%"struct.(anonymous namespace)::D"*)* @"\01??_9D@?A@@$BA@AA" to i8*), i8** %ptr
 // CHECK64: }
 }
 
 
 // Thunk for calling the 1st virtual function in C with no parameters.
 // CHECK32-LABEL: define linkonce_odr x86_thiscallcc void @"\01??_9C@@$BA@AE"(%struct.C* %this) unnamed_addr
 // CHECK32: [[VPTR:%.*]] = getelementptr inbounds void (%struct.C*)** %{{.*}}, i64 0

 // CHECK64-LABEL: define linkonce_odr i32 @"\01??_9C@@$B7AA"(%struct.C* %this, i32, double) unnamed_addr
 // CHECK64: [[VPTR:%.*]] = getelementptr inbounds i32 (%struct.C*, i32, double)** %{{.*}}, i64 1
 // CHECK64: [[CALLEE:%.*]] = load i32 (%struct.C*, i32, double)** [[VPTR]]
 // CHECK64: [[CALL:%.*]] = call i32 [[CALLEE]](%struct.C* %{{.*}}, i32 %{{.*}}, double %{{.*}})
 // CHECK64: ret i32 [[CALL]]
 // CHECK64: }
 
 // Thunk for calling the 3rd virtual function in C, taking an int parameter, returning a struct.
-// CHECK32-LABEL: define linkonce_odr x86_thiscallcc void @"\01??_9C@@$B7AE"(%struct.S* noalias sret %agg.result, %struct.C* %this, i32) unnamed_addr
-// CHECK32: [[VPTR:%.*]] = getelementptr inbounds void (%struct.S*, %struct.C*, i32)** %{{.*}}, i64 2
-// CHECK32: [[CALLEE:%.*]] = load void (%struct.S*, %struct.C*, i32)** [[VPTR]]
-// CHECK32: call x86_thiscallcc void [[CALLEE]](%struct.S* sret %agg.result, %struct.C* %{{.*}}, i32 %{{.*}})
+// CHECK32-LABEL: define linkonce_odr x86_thiscallcc void @"\01??_9C@@$B7AE"(%struct.C* %this, %struct.S* noalias sret %agg.result, i32) unnamed_addr
+// CHECK32: [[VPTR:%.*]] = getelementptr inbounds void (%struct.C*, %struct.S*, i32)** %{{.*}}, i64 2
+// CHECK32: [[CALLEE:%.*]] = load void (%struct.C*, %struct.S*, i32)** [[VPTR]]
+// CHECK32: call x86_thiscallcc void [[CALLEE]](%struct.C* %{{.*}}, %struct.S* sret %agg.result, i32 %{{.*}})
 // CHECK32: ret void
 // CHECK32: }
 //
-// CHECK64-LABEL: define linkonce_odr void @"\01??_9C@@$BBA@AA"(%struct.S* noalias sret %agg.result, %struct.C* %this, i32) unnamed_addr
-// CHECK64: [[VPTR:%.*]] = getelementptr inbounds void (%struct.S*, %struct.C*, i32)** %{{.*}}, i64 2
-// CHECK64: [[CALLEE:%.*]] = load void (%struct.S*, %struct.C*, i32)** [[VPTR]]
-// CHECK64: call void [[CALLEE]](%struct.S* sret %agg.result, %struct.C* %{{.*}}, i32 %{{.*}})
+// CHECK64-LABEL: define linkonce_odr void @"\01??_9C@@$BBA@AA"(%struct.C* %this, %struct.S* noalias sret %agg.result, i32) unnamed_addr
+// CHECK64: [[VPTR:%.*]] = getelementptr inbounds void (%struct.C*, %struct.S*, i32)** %{{.*}}, i64 2
+// CHECK64: [[CALLEE:%.*]] = load void (%struct.C*, %struct.S*, i32)** [[VPTR]]
+// CHECK64: call void [[CALLEE]](%struct.C* %{{.*}}, %struct.S* sret %agg.result, i32 %{{.*}})
 // CHECK64: ret void
 // CHECK64: }
 
 // Thunk for calling the virtual function in internal class D.
 // CHECK32-LABEL: define internal x86_thiscallcc void @"\01??_9D@?A@@$BA@AE"(%"struct.(anonymous namespace)::D"* %this) unnamed_addr
 // CHECK32: [[VPTR:%.*]] = getelementptr inbounds void (%"struct.(anonymous namespace)::D"*)** %{{.*}}, i64 0
 // CHECK32: [[CALLEE:%.*]] = load void (%"struct.(anonymous namespace)::D"*)** [[VPTR]]
 // CHECK32: call x86_thiscallcc void [[CALLEE]](%"struct.(anonymous namespace)::D"* %{{.*}})