[PATCH] D11859: Generating vptr assume loads

[John McCall via cfe-commits]

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