[PATCH] [ms-cxxabi] Implement member pointer conversions

[Reid Kleckner]

================
Comment at: lib/CodeGen/MicrosoftCXXABI.cpp:928
@@ +927,3 @@
+  if (E->getCastKind() == CK_ReinterpretMemberPointer)
+    return Builder.CreateSelect(IsNotNull, Src, DstNull);
+
----------------
Richard Smith wrote:
> Will we have rejected this in Sema if Src and Dst would have different types, or are we missing a bitcast on Src here? (What if the member pointers would have the same size but different types because one is missing a pointer member and the other is missing an int member?)
> 
> Also, it'd be nice to not emit the select if the null representation didn't change, which is presumably the common case.
Sema rejects data <-> function pointer casts and casts to member pointers of different sizes.  In practice, that guarantees the types are equal, but I'll put in an assert for it.

Added a test for avoiding the select if possible.

================
Comment at: lib/CodeGen/MicrosoftCXXABI.cpp:982-983
@@ +981,4 @@
+      Dst = Builder.CreateInsertValue(Dst, NonVirtualBaseAdjustment, Idx++);
+    // FIXME: Bug here if src had no VBPtrOffset but dst does, Idx isn't
+    // updated.
+    if (VBPtrOffset && hasVBPtrOffsetField(DstInheritance))
----------------
Richard Smith wrote:
> Right. Maybe unconditionally create an 'insertvalue' for every field in the destination, and use getConstantOrZeroInt? Also, start with an 'undef' rather than zero-initialized aggregate if you do this.
Oops, I handled this in the Constant case and forgot the Value case.  Added getValueOrZeroInt() and started from undef.

================
Comment at: lib/CodeGen/MicrosoftCXXABI.cpp:1016
@@ +1015,3 @@
+  if (E->getCastKind() == CK_ReinterpretMemberPointer)
+    return Src;
+
----------------
Richard Smith wrote:
> Do we ever need a bitcast here (if the representations are the same size but different types)?
We shouldn't, because data <-> function casts are rejected and there are no cases of different types but matching sizes in the ABI.

================
Comment at: lib/CodeGen/MicrosoftCXXABI.cpp:611-625
@@ +610,17 @@
+  // FIXME PR15713: Support virtual inheritance paths.
+  CharUnits ThisAdjustment = CharUnits::Zero();
+  ArrayRef<const CXXRecordDecl*> Path = MP.getMemberPointerPath();
+  bool DerivedMember = MP.isMemberPointerToDerivedMember();
+  const CXXRecordDecl *RD = cast<CXXRecordDecl>(MPD->getDeclContext());
+  for (unsigned I = 0, N = Path.size(); I != N; ++I) {
+    const CXXRecordDecl *Base = RD;
+    const CXXRecordDecl *Derived = Path[I];
+    if (DerivedMember)
+      std::swap(Base, Derived);
+    ThisAdjustment +=
+      getContext().getASTRecordLayout(Derived).getBaseClassOffset(Base);
+    RD = Path[I];
+  }
+  if (DerivedMember)
+    ThisAdjustment = -ThisAdjustment;
+
----------------
Richard Smith wrote:
> Please factor out the duplication between this and ItaniumCXXABI (maybe add an APValue overload for CGCXXABI::getMemberPointerAdjustment). Also, it'd be great to reuse CGM.GetNonVirtualBaseClassOffset for the actual computation here (but it looks like it expects the path in a slightly different format, so it may be more effort than it's worth to get that to happen).
Done.  I couldn't use an overload because the callers expect to get CharUnits.

Ideally getMemberPointerAdjustment(CastExpr) would also return CharUnits, since I discovered that I have to truncate it to IntTy on x86_64.


http://llvm-reviews.chandlerc.com/D741