MS ABI x64: Treat member pointer types as records, not large ints

[rnk]

Summary

The Win64 ABI docs say that arguments bigger than 8 bytes are passed
indirectly, and we have to apply that logic to member pointers.

[rnk]

• This affects mingw64 as well as win64.

[majnemer]

LGTM

[rnk]

Closed by commit rL207817 (authored by @rnk).

lib/CodeGen/TargetInfo.cpp

ABIArgInfo WinX86_64ABIInfo::classify(QualType Ty, bool IsReturnType) const {
   if (Ty->isVoidType())
     return ABIArgInfo::getIgnore();
 
   if (const EnumType *EnumTy = Ty->getAs<EnumType>())
     Ty = EnumTy->getDecl()->getIntegerType();
 
   uint64_t Size = getContext().getTypeSize(Ty);
 
-  if (const RecordType *RT = Ty->getAs<RecordType>()) {
+  const RecordType *RT = Ty->getAs<RecordType>();
+  if (RT) {
     if (IsReturnType) {
       if (isRecordReturnIndirect(RT, getCXXABI()))
         return ABIArgInfo::getIndirect(0, false);
     } else {
       if (CGCXXABI::RecordArgABI RAA = getRecordArgABI(RT, getCXXABI()))
         return ABIArgInfo::getIndirect(0, RAA == CGCXXABI::RAA_DirectInMemory);
     }
 
     if (RT->getDecl()->hasFlexibleArrayMember())
       return ABIArgInfo::getIndirect(0, /*ByVal=*/false);
 
     // FIXME: mingw-w64-gcc emits 128-bit struct as i128
     if (Size == 128 && getTarget().getTriple().isWindowsGNUEnvironment())
       return ABIArgInfo::getDirect(llvm::IntegerType::get(getVMContext(),
                                                           Size));
+  }
+
+  if (const auto *MPT = Ty->getAs<MemberPointerType>()) {
+    // If the member pointer is not an aggregate, pass it directly.
+    if (getTarget().getCXXABI().isMicrosoft()) {
+      // For Microsoft, check with the inheritance model.
+      const CXXRecordDecl *RD = MPT->getClass()->getAsCXXRecordDecl();
+      if (MSInheritanceAttr::hasOnlyOneField(MPT->isMemberFunctionPointer(),
+                                             RD->getMSInheritanceModel()))
+        return ABIArgInfo::getDirect();
+    } else {
+      // For Itanium, data pointers are simple and function pointers are big.
+      if (MPT->isMemberDataPointer())
+        return ABIArgInfo::getDirect();
+    }
+  }
 
+  if (RT || Ty->isMemberPointerType()) {
     // MS x64 ABI requirement: "Any argument that doesn't fit in 8 bytes, or is
     // not 1, 2, 4, or 8 bytes, must be passed by reference."
-    if (Size <= 64 &&
-        (Size & (Size - 1)) == 0)
-      return ABIArgInfo::getDirect(llvm::IntegerType::get(getVMContext(),
-                                                          Size));
-
+    if (Size > 64 || !llvm::isPowerOf2_64(Size))
       return ABIArgInfo::getIndirect(0, /*ByVal=*/false);
+
+    // Otherwise, coerce it to a small integer.
+    return ABIArgInfo::getDirect(llvm::IntegerType::get(getVMContext(), Size));
   }
 
   if (Ty->isPromotableIntegerType())
     return ABIArgInfo::getExtend();
 
   return ABIArgInfo::getDirect();
 }
 

test/CodeGenCXX/member-function-pointer-calls.cpp

 // RUN: %clang_cc1 %s -triple=x86_64-apple-darwin10 -emit-llvm -O3 -o - | FileCheck %s
+// RUN: %clang_cc1 %s -triple=x86_64-windows-gnu -emit-llvm -o - | FileCheck %s -check-prefix MINGW64
 struct A {
   virtual int vf1() { return 1; }
   virtual int vf2() { return 2; }
 };
 
 int f(A* a, int (A::*fp)()) {
   return (a->*fp)();
 }
 
 // CHECK-LABEL: define i32 @_Z2g1v()
 // CHECK: ret i32 1
+// MINGW64-LABEL: define i32 @_Z2g1v()
+// MINGW64: call i32 @_Z1fP1AMS_FivE(%struct.A* %{{.*}}, { i64, i64 }* %{{.*}})
 int g1() {
   A a;
   return f(&a, &A::vf1);
 }
 
 // CHECK-LABEL: define i32 @_Z2g2v()
 // CHECK: ret i32 2
+// MINGW64-LABEL: define i32 @_Z2g2v()
+// MINGW64: call i32 @_Z1fP1AMS_FivE(%struct.A* %{{.*}}, { i64, i64 }* %{{.*}})
 int g2() {
   A a;
   return f(&a, &A::vf2);
 }

test/CodeGenCXX/microsoft-abi-member-pointers.cpp

 // RUN: %clang_cc1 -fno-rtti -emit-llvm %s -o - -triple=i386-pc-win32 | FileCheck %s
+// RUN: %clang_cc1 -fno-rtti -emit-llvm %s -o - -triple=x86_64-pc-win32 | FileCheck %s -check-prefix=X64
 // RUN: %clang_cc1 -fno-rtti -emit-llvm %s -o - -triple=i386-pc-win32 -DINCOMPLETE_VIRTUAL -fms-extensions -verify
 // RUN: %clang_cc1 -fno-rtti -emit-llvm %s -o - -triple=i386-pc-win32 -DINCOMPLETE_VIRTUAL -DMEMFUN -fms-extensions -verify
 // FIXME: Test x86_64 member pointers when codegen no longer asserts on records
 // with virtual bases.
 
 #ifndef INCOMPLETE_VIRTUAL
 struct B1 {
   void foo();

 // CHECK:   %[[mp1:.*]] = extractvalue { i32, i32, i32 } %[[mp]], 1
 // CHECK:   %[[cmp1:.*]] = icmp ne i32 %[[mp1]], 0
 // CHECK:   %[[and0:.*]] = and i1 %[[cmp0]], %[[cmp1]]
 // CHECK:   %[[mp2:.*]] = extractvalue { i32, i32, i32 } %[[mp]], 2
 // CHECK:   %[[cmp2:.*]] = icmp ne i32 %[[mp2]], -1
 // CHECK:   %[[and1:.*]] = and i1 %[[and0]], %[[cmp2]]
 // CHECK:   ret i1 %[[and1]]
 // CHECK: }
+
+// Pass this large type indirectly.
+// X64-LABEL: define zeroext i1 @"\01?nullTestDataUnspecified@@
+// X64:             ({ i32, i32, i32 }*)
 }
 
 bool nullTestFunctionUnspecified(void (Unspecified::*mp)()) {
   return mp;
 // CHECK: define zeroext i1 @"\01?nullTestFunctionUnspecified@@YA_NP8Unspecified@@AEXXZ@Z"{{.*}} {
 // CHECK:   %{{.*}} = load { i8*, i32, i32, i32 }* %{{.*}}, align 8
 // CHECK:   store { i8*, i32, i32, i32 } {{.*}} align 8
 // CHECK:   %[[mp:.*]] = load { i8*, i32, i32, i32 }* %{{.*}}, align 8

 // CHECK:   %[[v8:.*]] = bitcast i8* %[[v7]] to i32*
 // CHECK:   %[[vbase_offs:.*]] = load i32* %[[v8]]
 // CHECK:   %[[v10:.*]] = getelementptr inbounds i8* %[[vbptr]], i32 %[[vbase_offs]]
 // CHECK:   %[[offset:.*]] = getelementptr inbounds i8* %[[v10]], i32 %[[memptr0]]
 // CHECK:   %[[v11:.*]] = bitcast i8* %[[offset]] to i32*
 // CHECK:   %[[v12:.*]] = load i32* %[[v11]]
 // CHECK:   ret i32 %[[v12]]
 // CHECK: }
+
+// A two-field data memptr on x64 gets coerced to i64 and is passed in a
+// register or memory.
+// X64-LABEL: define i32 @"\01?loadDataMemberPointerVirtual@@YAHPEAUVirtual@@PEQ1@H@Z"
+// X64:             (%struct.Virtual* %o, i64 %memptr.coerce)
 }
 
 int loadDataMemberPointerUnspecified(Unspecified *o, int Unspecified::*memptr) {
   return o->*memptr;
 // Test that we can unpack this aggregate member pointer and load the member
 // data pointer.
 // CHECK: define i32 @"\01?loadDataMemberPointerUnspecified@@YAHPAUUnspecified@@PQ1@H@Z"{{.*}} {
 // CHECK:   %[[o:.*]] = load %{{.*}}** %{{.*}}, align 4

 
 void callMemberPointerSingle(Single *o, void (Single::*memptr)()) {
   (o->*memptr)();
 // Just look for an indirect thiscall.
 // CHECK: define void @"\01?callMemberPointerSingle@@{{.*}} {{.*}} {
 // CHECK:   call x86_thiscallcc void %{{.*}}(%{{.*}} %{{.*}})
 // CHECK:   ret void
 // CHECK: }
+
+// X64-LABEL: define void @"\01?callMemberPointerSingle@@
+// X64:           (%struct.Single* %o, i8* %memptr)
+// X64:   bitcast i8* %{{[^ ]*}} to void (%struct.Single*)*
+// X64:   ret void
 }
 
 void callMemberPointerMultiple(Multiple *o, void (Multiple::*memptr)()) {
   (o->*memptr)();
 // CHECK: define void @"\01?callMemberPointerMultiple@@{{.*}} {
 // CHECK:   %[[memptr0:.*]] = extractvalue { i8*, i32 } %{{.*}}, 0
 // CHECK:   %[[memptr1:.*]] = extractvalue { i8*, i32 } %{{.*}}, 1
 // CHECK:   %[[this_adjusted:.*]] = getelementptr inbounds i8* %{{.*}}, i32 %[[memptr1]]
bool compareSingleFunctionMemptr(void (Single::*l)(), void (Single::*r)()) {
   return l == r;
 // Should only be one comparison here.
 // CHECK: define zeroext i1 @"\01?compareSingleFunctionMemptr@@YA_NP8Single@@AEXXZ0@Z"{{.*}} {
 // CHECK-NOT: icmp
 // CHECK:   %[[r:.*]] = icmp eq
 // CHECK-NOT: icmp
 // CHECK:   ret i1 %[[r]]
 // CHECK: }
+
+// X64-LABEL: define zeroext i1 @"\01?compareSingleFunctionMemptr@@
+// X64:             (i8* %{{[^,]*}}, i8* %{{[^)]*}})
 }
 
 bool compareNeqSingleFunctionMemptr(void (Single::*l)(), void (Single::*r)()) {
   return l != r;
 // Should only be one comparison here.
 // CHECK: define zeroext i1 @"\01?compareNeqSingleFunctionMemptr@@YA_NP8Single@@AEXXZ0@Z"{{.*}} {
 // CHECK-NOT: icmp
 // CHECK:   %[[r:.*]] = icmp ne

 // CHECK:   %{{.*}} = extractvalue { i8*, i32, i32, i32 } %[[r]], 3
 // CHECK:   %[[cmp3:.*]] = icmp eq i32
 // CHECK:   %[[res123:.*]] = and i1 %[[res12]], %[[cmp3]]
 // CHECK:   %[[iszero:.*]] = icmp eq i8* %[[lhs0]], null
 // CHECK:   %[[bits_or_null:.*]] = or i1 %[[res123]], %[[iszero]]
 // CHECK:   %{{.*}} = and i1 %[[bits_or_null]], %[[cmp0]]
 // CHECK:   ret i1 %{{.*}}
 // CHECK: }
+
+// X64-LABEL: define zeroext i1 @"\01?unspecFuncMemptrEq@@
+// X64:             ({ i8*, i32, i32, i32 }*, { i8*, i32, i32, i32 }*)
 }
 
 bool unspecFuncMemptrNeq(void (Unspecified::*l)(), void (Unspecified::*r)()) {
   return l != r;
 // CHECK: define zeroext i1 @"\01?unspecFuncMemptrNeq@@YA_NP8Unspecified@@AEXXZ0@Z"{{.*}} {
 // CHECK:   %[[lhs0:.*]] = extractvalue { i8*, i32, i32, i32 } %[[l:.*]], 0
 // CHECK:   %{{.*}} = extractvalue { i8*, i32, i32, i32 } %[[r:.*]], 0
 // CHECK:   %[[cmp0:.*]] = icmp ne i8* %[[lhs0]], %{{.*}}

 // CHECK:   icmp eq i32
 // CHECK:   extractvalue { i32, i32, i32 } %{{.*}}, 2
 // CHECK:   extractvalue { i32, i32, i32 } %{{.*}}, 2
 // CHECK:   icmp eq i32
 // CHECK:   and i1
 // CHECK:   and i1
 // CHECK:   ret i1
 // CHECK: }
+
+// X64-LABEL: define zeroext i1 @"\01?unspecDataMemptrEq@@
+// X64:             ({ i32, i32, i32 }*, { i32, i32, i32 }*)
 }
 
 void (Multiple::*convertB2FuncToMultiple(void (B2::*mp)()))() {
   return mp;
 // CHECK: define i64 @"\01?convertB2FuncToMultiple@@YAP8Multiple@@AEXXZP8B2@@AEXXZ@Z"{{.*}} {
 // CHECK:   store
 // CHECK:   %[[mp:.*]] = load i8** %{{.*}}, align 4
 // CHECK:   icmp ne i8* %[[mp]], null