[PATCH] [ms-cxxabi] Implement member data pointers for non-dynamic classes

[Reid Kleckner]

New patch forthcoming, crossing fingers that it goes to the same thread.

On Wed, Mar 20, 2013 at 10:57 PM, John McCall <rjmccall at apple.com> wrote:

> On Mar 20, 2013, at 3:59 PM, Reid Kleckner <rnk at google.com> wrote:
> > CHANGE SINCE LAST DIFF
> >  http://llvm-reviews.chandlerc.com/D558?vs=1333&id=1338#toc
>
>
> +  // It's a little silly for us to cache this.
> +  llvm::IntegerType *getPtrDiffTy() {
>
> It's actually CGM.PtrDiffTy, so there was never a good reason to cache it
> separately.
>

Nice, nuking this method makes the code shorter.


> +llvm::Constant *
> +MicrosoftCXXABI::getNullMemberDataPointer(const MemberPointerType *MPT) {
> +  assert(MPT->isMemberDataPointer());
> +  llvm::Constant *Zero = llvm::ConstantInt::get(getPtrDiffTy(), 0);
> +  llvm::Constant *AllOnes =
> llvm::Constant::getAllOnesValue(getPtrDiffTy());
> +  const CXXRecordDecl *RD =
> +    cast<CXXRecordDecl>(MPT->getClass()->getAs<RecordType>()->getDecl());
>
> MPT->getClass()->getAsCXXRecordDecl()
>

Nice.


> +  if (RD->getNumVBases() == 0) {
>
> I don't think you can safely call this on a forward-declaration.
>

True.  Based on what I've learned since I wrote this patch, I'll need to go
look at those inheritance attributes.


> Why don't you make helper functions to create the all-ones and all-zero
> ptrdiff_t values?  That would make this code much more succinct.
>

Done.


> +  // FIXME: Emit a pair of {vbtable offset, field offset} when we know
> vbtable
> +  // layout.
> +  return GetBogusMemberPointer(QualType(MPT, 0));
>
> I don't think this ever actually comes up here.  If it did, you'd need a
> totally different interface anyway.
>

Yeah, we do need a different interface.  Testing this case hits assertions.


> I believe vb-table offsets are only introduced when you convert across a
> virtual base boundary.
>

I haven't looked too deeply into it yet, just enough to know how it differs
and that we don't support it yet.


> +  // For member data pointers, this is just a check against -1 or 0.
> +  if (MPT->isMemberDataPointer()) {
> +    assert(MemPtr->getType() == getPtrDiffTy());
> +    llvm::Constant *Val = getNullMemberDataPointer(MPT);
> +    return Builder.CreateICmpNE(MemPtr, Val, "memptr.tobool");
> +  }
>
> You forgot the virtual inheritance case here.
>

OK, fixed, that will issue an error instead of asserting now.


> +  if (MemPtr->getType() != getPtrDiffTy()) {
> +    // If it's not a ptrdiff_t, then it's an aggregate.
> +    ErrorUnsupportedABI(CGF, "non-scalar member pointers");
> +    return llvm::Constant::getNullValue(PType);
> +  }
> +
>
> Shouldn't this be pretty straightforward to implement?  We may not know
> how to lay out a vbtbl yet, so the conversion case will still elude us,
> but we
> do know the offset of the vb-table and how to do an offsetted load out of
> it.
>

I haven't figured out exactly how to do this with a virtual base yet, and
Timur says that Clang currently has very little support for virtual
inheritance.  I'd rather handle simple member data pointers that are just
offsets first, and then come back when we tackle virtual inheritance.
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20130321/db460bd0/attachment.html>