majnemer added a comment.
Some first round review comments.
================
Comment at: lib/CodeGen/CGClass.cpp:1831-1832
@@ +1830,4 @@
+
+ // generate vtable assumptions if we are not in another ctor and
+ // if we calling dynamic class ctor
+ if (CGM.getCodeGenOpts().OptimizationLevel > 0 &&
----------------
Sentences should start with a capital letter and end with a period.
================
Comment at: lib/CodeGen/CGClass.cpp:1838
@@ +1837,3 @@
+
+void CodeGenFunction::EmitVTableAssumptionLoad(const VPtr &vptr,
+ llvm::Value *This) {
----------------
Variables should be capitalized.
================
Comment at: lib/CodeGen/CGClass.cpp:1842
@@ +1841,3 @@
+ CGM.getCXXABI().getVTableAddressPoint(vptr.Base, vptr.VTableClass);
+ if (!VTableGlobal) return;
+
----------------
The return should be on it's own line, please clang-format this.
Also, under which conditions is this case possible?
================
Comment at: lib/CodeGen/CGClass.cpp:2098
@@ -2063,3 +2097,3 @@
// Initialize the vtable pointer for this base.
- InitializeVTablePointer(Base, NearestVBase, OffsetFromNearestVBase,
- VTableClass);
+ VPtr vptr{Base, NearestVBase, OffsetFromNearestVBase, VTableClass};
+ vptrs.push_back(vptr);
----------------
This kind of initialization is not very common in LLVM.
================
Comment at: lib/CodeGen/CodeGenFunction.h:1312
@@ -1311,1 +1311,3 @@
+ // struct containg all informations needed to initilize vptrs
+ struct VPtr {
----------------
Capital letter and period missing here.
================
Comment at: lib/CodeGen/CodeGenFunction.h:1328-1334
@@ -1320,8 +1327,9 @@
typedef llvm::SmallPtrSet<const CXXRecordDecl *, 4> VisitedVirtualBasesSetTy;
- void InitializeVTablePointers(BaseSubobject Base,
- const CXXRecordDecl *NearestVBase,
- CharUnits OffsetFromNearestVBase,
- bool BaseIsNonVirtualPrimaryBase,
- const CXXRecordDecl *VTableClass,
- VisitedVirtualBasesSetTy& VBases);
+ VPtrsVector getVTablePointers(const CXXRecordDecl *VTableClass);
+
+ void getVTablePointers(BaseSubobject Base, const CXXRecordDecl *NearestVBase,
+ CharUnits OffsetFromNearestVBase,
+ bool BaseIsNonVirtualPrimaryBase,
+ const CXXRecordDecl *VTableClass,
+ VisitedVirtualBasesSetTy &VBases, VPtrsVector &vptrs);
----------------
Would it make more sense for these to live in `CGClass` ?
================
Comment at: lib/CodeGen/ItaniumCXXABI.cpp:193-194
@@ -192,1 +192,4 @@
+ bool isVirtualOffsetNeeded(CodeGenFunction &CGF,
+ const CXXRecordDecl *NearestVBase) override;
+
----------------
I'm not sure what this function is supposed to compute. Needed for what?
================
Comment at: lib/CodeGen/ItaniumCXXABI.cpp:1404
@@ +1403,3 @@
+ const CXXRecordDecl *NearestVBase) {
+ // TODO check it
+ if ((Base.getBase()->getNumVBases() || NearestVBase != nullptr) &&
----------------
Check what?
================
Comment at: lib/CodeGen/MicrosoftCXXABI.cpp:223-228
@@ +222,8 @@
+ // with the 'novtable' attribute.
+ bool canInitilizeVPtr(const CXXRecordDecl *VTableClass,
+ const CXXRecordDecl *Base,
+ const CXXRecordDecl *NearestVBase) override {
+ return !VTableClass->hasAttr<MSNoVTableAttr>() ||
+ (Base != VTableClass && Base != NearestVBase);
+ }
+
----------------
In the MS ABI, derived classes never share vtables with bases. Why do you need
to do anything other than check that the most derived class doesn't have the
`__declspec(novtable)` ?
http://reviews.llvm.org/D11859
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
