[cfe-commits] r126865 - /cfe/trunk/lib/CodeGen/CGVTables.cpp

[Tilmann Scheller]

Makes sense, reverted in r126876.

Regards,

Tilmann

On Wed, Mar 2, 2011 at 9:34 PM, Douglas Gregor <dgregor at apple.com> wrote:

>
> On Mar 2, 2011, at 11:38 AM, Tilmann Scheller wrote:
>
> > Author: tilmann
> > Date: Wed Mar  2 13:38:28 2011
> > New Revision: 126865
> >
> > URL: http://llvm.org/viewvc/llvm-project?rev=126865&view=rev
> > Log:
> > Add preliminary support for MSVC-style vtables.
>
> This is the wrong way to introduce support for MSVC vtables. Having a large
> number of "if (Win64)" branches within the (currently Itanium) vtable
> builder makes it a mess to read.
>
> Instead, you should first introduce an abstraction layer, so that the
> Itanium vtable builder will be cleanly separated from  from MSVC vtable
> builder, just as we've done with the Objective-C runtimes and which Chip
> Davis already started work on. Then, you can fill in the MSVC-style vtable
> builder with all of the Win64-specific code.
>
> Please revert this commit.
>
>  - Doug
>
> > Modified:
> >    cfe/trunk/lib/CodeGen/CGVTables.cpp
> >
> > Modified: cfe/trunk/lib/CodeGen/CGVTables.cpp
> > URL:
> http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/CodeGen/CGVTables.cpp?rev=126865&r1=126864&r2=126865&view=diff
> >
> ==============================================================================
> > --- cfe/trunk/lib/CodeGen/CGVTables.cpp (original)
> > +++ cfe/trunk/lib/CodeGen/CGVTables.cpp Wed Mar  2 13:38:28 2011
> > @@ -2266,6 +2266,32 @@
> >
> >   int64_t CurrentIndex = 0;
> >
> > +  // The MSVC Win64 ABI uses a different ordering of the
> > +  // virtual function pointers:
> > +  //
> > +  // The order of the virtual function pointers in a virtual table is
> > +  // the reverse order of declaration of the corresponding member
> functions
> > +  // in the class except for overloaded member functions. Overloaded
> member
> > +  // functions are moved below the first declaration of the
> > +  // respective overloaded member function. All declarations for one
> > +  // overloaded member function are added in reverse order of
> declaration.
> > +  //
> > +  // E.g. for the following class:
> > +  // class A {
> > +  //   virtual b();
> > +  //   virtual a(int a);
> > +  //   virtual c();
> > +  //   virtual a(float a);
> > +  // };
> > +  //
> > +  // The vtable layout looks like this:
> > +  // c()
> > +  // a(float b)
> > +  // a(int a)
> > +  // b()
> > +  //
> > +  const bool IsWin64 = CGM.getContext().Target.isWin64();
> > +
> >   const ASTRecordLayout &Layout =
> CGM.getContext().getASTRecordLayout(RD);
> >   const CXXRecordDecl *PrimaryBase = Layout.getPrimaryBase();
> >
> > @@ -2285,6 +2311,9 @@
> >
> >   const CXXDestructorDecl *ImplicitVirtualDtor = 0;
> >
> > +  llvm::StringMap<llvm::SmallVector<const CXXMethodDecl *, 16> *>
> Methods;
> > +  llvm::SmallVector<llvm::SmallVector<const CXXMethodDecl *, 16> *, 16>
> Order;
> > +
> >   for (CXXRecordDecl::method_iterator i = RD->method_begin(),
> >        e = RD->method_end(); i != e; ++i) {
> >     const CXXMethodDecl *MD = *i;
> > @@ -2293,6 +2322,20 @@
> >     if (!MD->isVirtual())
> >       continue;
> >
> > +    if (IsWin64) {
> > +      if (Methods.count(MD->getNameAsString()) == 0) {
> > +        // No entry yet.
> > +        llvm::SmallVector<const CXXMethodDecl *, 16> *entry = new
> llvm::SmallVector<const CXXMethodDecl *, 16>();
> > +        entry->push_back(MD);
> > +        Methods[MD->getNameAsString()] = entry;
> > +        Order.push_back(entry);
> > +      } else {
> > +        // This is an overloaded method, add to already existing entry.
> > +        llvm::SmallVector<const CXXMethodDecl *, 16> *entry =
> Methods[MD->getNameAsString()];
> > +        entry->push_back(MD);
> > +      }
> > +    }
> > +
> >     // Check if this method overrides a method in the primary base.
> >     if (const CXXMethodDecl *OverriddenMD =
> >           FindNearestOverriddenMethod(MD, PrimaryBases)) {
> > @@ -2327,7 +2370,7 @@
> >         ImplicitVirtualDtor = DD;
> >         continue;
> >       }
> > -
> > +      assert(!IsWin64 && "Need support for dtor!");
> >       // Add the complete dtor.
> >       MethodVTableIndices[GlobalDecl(DD, Dtor_Complete)] =
> CurrentIndex++;
> >
> > @@ -2335,11 +2378,14 @@
> >       MethodVTableIndices[GlobalDecl(DD, Dtor_Deleting)] =
> CurrentIndex++;
> >     } else {
> >       // Add the entry.
> > -      MethodVTableIndices[MD] = CurrentIndex++;
> > +      if (!IsWin64) {
> > +        MethodVTableIndices[MD] = CurrentIndex++;
> > +      }
> >     }
> >   }
> >
> >   if (ImplicitVirtualDtor) {
> > +    assert(!IsWin64 && "Need support for implicit virtual dtor!");
> >     // Itanium C++ ABI 2.5.2:
> >     //   If a class has an implicitly-defined virtual destructor,
> >     //   its entries come after the declared virtual function pointers.
> > @@ -2353,6 +2399,19 @@
> >       CurrentIndex++;
> >   }
> >
> > +  if (IsWin64) {
> > +    for (llvm::SmallVector<llvm::SmallVector<const CXXMethodDecl *, 16>
> *, 16>::iterator i = Order.begin(),
> > +         e = Order.end(); i != e; ++i) {
> > +        llvm::SmallVector<const CXXMethodDecl *, 16> *elem = *i;
> > +
> > +        while (elem->size() > 0) {
> > +            const CXXMethodDecl *method = elem->pop_back_val();
> > +            // Add the entry.
> > +            MethodVTableIndices[method] = CurrentIndex++;
> > +        }
> > +    }
> > +  }
> > +
> >   NumVirtualFunctionPointers[RD] = CurrentIndex;
> > }
> >
> >
> >
> > _______________________________________________
> > cfe-commits mailing list
> > cfe-commits at cs.uiuc.edu
> > http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20110302/1acbe459/attachment.html>