rjmccall added a comment. Mostly LGTM. Are you going to emit assumptions for vbptrs in a separate patch?
================ Comment at: lib/CodeGen/CGCXXABI.h:349-357 @@ -348,1 +348,11 @@ + /// Checks if ABI requires extra virtual offset for vtable field. + virtual bool + isVirtualOffsetNeededForVTableField(CodeGenFunction &CGF, + const CXXRecordDecl *NearestVBase) = 0; + + /// Checks if ABI allows to initilize vptr for given class. + virtual bool canInitializeVPtr(const CXXRecordDecl *VTableClass, + const CXXRecordDecl *Base, + const CXXRecordDecl *NearestVBase) = 0; + ---------------- In contrast, this does not need to be as general as it is, and making it this general is actively harmful; just pass the vtable class. Also, I suggest calling it "doStructorsInitializeVTables". ================ Comment at: lib/CodeGen/CGCXXABI.h:352 @@ +351,3 @@ + isVirtualOffsetNeededForVTableField(CodeGenFunction &CGF, + const CXXRecordDecl *NearestVBase) = 0; + ---------------- This method does not need to be passed a CodeGenFunction&, but it should take a complete CodeGenFunction::VPtr, not just this one random field from it. ================ Comment at: lib/CodeGen/CGClass.cpp:1862 @@ +1861,3 @@ + for (const VPtr &vptr : getVTablePointers(ClassDecl)) + if (CGM.getCXXABI().canInitializeVPtr(vptr.VTableClass, vptr.Base.getBase(), + vptr.NearestVBase)) ---------------- As mentioned elsewhere, you can skip this entire loop if doStructorsInitializeVTables returns false. ================ Comment at: lib/CodeGen/CGClass.cpp:2157 @@ +2156,3 @@ + if (CGM.getCXXABI().canInitializeVPtr(Vptr.VTableClass, Vptr.Base.getBase(), + Vptr.NearestVBase)) + InitializeVTablePointer(Vptr); ---------------- Please call doStructorsInitializeVTables for RD above the call to getVTablePointers and then just skip the rest of the function if it returns false. http://reviews.llvm.org/D11859 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits