<html><head></head><body style="word-wrap: break-word; -webkit-nbsp-mode: space; -webkit-line-break: after-white-space; "><br><div><div>On Mar 2, 2011, at 12:46 PM, Tilmann Scheller wrote:</div><br class="Apple-interchange-newline"><blockquote type="cite">Makes sense, reverted in r126876.<br></blockquote><div><br></div>Thanks!</div><div><br><blockquote type="cite">Regards,<br><br>Tilmann<br><br><div class="gmail_quote">On Wed, Mar 2, 2011 at 9:34 PM, Douglas Gregor <span dir="ltr"><<a href="mailto:dgregor@apple.com">dgregor@apple.com</a>></span> wrote:<br>
<blockquote class="gmail_quote" style="margin: 0pt 0pt 0pt 0.8ex; border-left: 1px solid rgb(204, 204, 204); padding-left: 1ex;"><div class="im"><br>
On Mar 2, 2011, at 11:38 AM, Tilmann Scheller wrote:<br>
<br>
> Author: tilmann<br>
> Date: Wed Mar 2 13:38:28 2011<br>
> New Revision: 126865<br>
><br>
> URL: <a href="http://llvm.org/viewvc/llvm-project?rev=126865&view=rev" target="_blank">http://llvm.org/viewvc/llvm-project?rev=126865&view=rev</a><br>
> Log:<br>
> Add preliminary support for MSVC-style vtables.<br>
<br>
</div>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.<br>
<br>
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.<br>
<br>
Please revert this commit.<br>
<br>
- Doug<br>
<div><div></div><div class="h5"><br>
> Modified:<br>
> cfe/trunk/lib/CodeGen/CGVTables.cpp<br>
><br>
> Modified: cfe/trunk/lib/CodeGen/CGVTables.cpp<br>
> URL: <a href="http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/CodeGen/CGVTables.cpp?rev=126865&r1=126864&r2=126865&view=diff" target="_blank">http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/CodeGen/CGVTables.cpp?rev=126865&r1=126864&r2=126865&view=diff</a><br>
> ==============================================================================<br>
> --- cfe/trunk/lib/CodeGen/CGVTables.cpp (original)<br>
> +++ cfe/trunk/lib/CodeGen/CGVTables.cpp Wed Mar 2 13:38:28 2011<br>
> @@ -2266,6 +2266,32 @@<br>
><br>
> int64_t CurrentIndex = 0;<br>
><br>
> + // The MSVC Win64 ABI uses a different ordering of the<br>
> + // virtual function pointers:<br>
> + //<br>
> + // The order of the virtual function pointers in a virtual table is<br>
> + // the reverse order of declaration of the corresponding member functions<br>
> + // in the class except for overloaded member functions. Overloaded member<br>
> + // functions are moved below the first declaration of the<br>
> + // respective overloaded member function. All declarations for one<br>
> + // overloaded member function are added in reverse order of declaration.<br>
> + //<br>
> + // E.g. for the following class:<br>
> + // class A {<br>
> + // virtual b();<br>
> + // virtual a(int a);<br>
> + // virtual c();<br>
> + // virtual a(float a);<br>
> + // };<br>
> + //<br>
> + // The vtable layout looks like this:<br>
> + // c()<br>
> + // a(float b)<br>
> + // a(int a)<br>
> + // b()<br>
> + //<br>
> + const bool IsWin64 = CGM.getContext().Target.isWin64();<br>
> +<br>
> const ASTRecordLayout &Layout = CGM.getContext().getASTRecordLayout(RD);<br>
> const CXXRecordDecl *PrimaryBase = Layout.getPrimaryBase();<br>
><br>
> @@ -2285,6 +2311,9 @@<br>
><br>
> const CXXDestructorDecl *ImplicitVirtualDtor = 0;<br>
><br>
> + llvm::StringMap<llvm::SmallVector<const CXXMethodDecl *, 16> *> Methods;<br>
> + llvm::SmallVector<llvm::SmallVector<const CXXMethodDecl *, 16> *, 16> Order;<br>
> +<br>
> for (CXXRecordDecl::method_iterator i = RD->method_begin(),<br>
> e = RD->method_end(); i != e; ++i) {<br>
> const CXXMethodDecl *MD = *i;<br>
> @@ -2293,6 +2322,20 @@<br>
> if (!MD->isVirtual())<br>
> continue;<br>
><br>
> + if (IsWin64) {<br>
> + if (Methods.count(MD->getNameAsString()) == 0) {<br>
> + // No entry yet.<br>
> + llvm::SmallVector<const CXXMethodDecl *, 16> *entry = new llvm::SmallVector<const CXXMethodDecl *, 16>();<br>
> + entry->push_back(MD);<br>
> + Methods[MD->getNameAsString()] = entry;<br>
> + Order.push_back(entry);<br>
> + } else {<br>
> + // This is an overloaded method, add to already existing entry.<br>
> + llvm::SmallVector<const CXXMethodDecl *, 16> *entry = Methods[MD->getNameAsString()];<br>
> + entry->push_back(MD);<br>
> + }<br>
> + }<br>
> +<br>
> // Check if this method overrides a method in the primary base.<br>
> if (const CXXMethodDecl *OverriddenMD =<br>
> FindNearestOverriddenMethod(MD, PrimaryBases)) {<br>
> @@ -2327,7 +2370,7 @@<br>
> ImplicitVirtualDtor = DD;<br>
> continue;<br>
> }<br>
> -<br>
> + assert(!IsWin64 && "Need support for dtor!");<br>
> // Add the complete dtor.<br>
> MethodVTableIndices[GlobalDecl(DD, Dtor_Complete)] = CurrentIndex++;<br>
><br>
> @@ -2335,11 +2378,14 @@<br>
> MethodVTableIndices[GlobalDecl(DD, Dtor_Deleting)] = CurrentIndex++;<br>
> } else {<br>
> // Add the entry.<br>
> - MethodVTableIndices[MD] = CurrentIndex++;<br>
> + if (!IsWin64) {<br>
> + MethodVTableIndices[MD] = CurrentIndex++;<br>
> + }<br>
> }<br>
> }<br>
><br>
> if (ImplicitVirtualDtor) {<br>
> + assert(!IsWin64 && "Need support for implicit virtual dtor!");<br>
> // Itanium C++ ABI 2.5.2:<br>
> // If a class has an implicitly-defined virtual destructor,<br>
> // its entries come after the declared virtual function pointers.<br>
> @@ -2353,6 +2399,19 @@<br>
> CurrentIndex++;<br>
> }<br>
><br>
> + if (IsWin64) {<br>
> + for (llvm::SmallVector<llvm::SmallVector<const CXXMethodDecl *, 16> *, 16>::iterator i = Order.begin(),<br>
> + e = Order.end(); i != e; ++i) {<br>
> + llvm::SmallVector<const CXXMethodDecl *, 16> *elem = *i;<br>
> +<br>
> + while (elem->size() > 0) {<br>
> + const CXXMethodDecl *method = elem->pop_back_val();<br>
> + // Add the entry.<br>
> + MethodVTableIndices[method] = CurrentIndex++;<br>
> + }<br>
> + }<br>
> + }<br>
> +<br>
> NumVirtualFunctionPointers[RD] = CurrentIndex;<br>
> }<br>
><br>
><br>
><br>
> _______________________________________________<br>
> cfe-commits mailing list<br>
> <a href="mailto:cfe-commits@cs.uiuc.edu">cfe-commits@cs.uiuc.edu</a><br>
> <a href="http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits" target="_blank">http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits</a><br>
<br>
</div></div></blockquote></div><br>
</blockquote></div><br></body></html>