[PATCH] D11859: Generating vptr assume loads

[John McCall via cfe-commits]

rjmccall added a comment.

Just a couple tweaks and then LGTM.


================
Comment at: lib/CodeGen/CGClass.cpp:1833
@@ +1832,3 @@
+  // unless we are calling base constructor - we don't want to generating
+  // assumption loads for not completed because vptr may still change.
+  if (CGM.getCodeGenOpts().OptimizationLevel > 0 &&
----------------
Please use this comment:

  // Generate vtable assumptions if we're constructing a complete object
  // with a vtable.  We don't do this for base subobjects for two reasons:
  // first, it's incorrect for classes with virtual bases, and second, we're
  // about to overwrite the vptrs anyway.

================
Comment at: lib/CodeGen/CGClass.cpp:2155
@@ -2122,1 +2154,3 @@
+    for (const VPtr &Vptr : Vptrs)
+      InitializeVTablePointer(Vptr);
 
----------------
Please also skip the call to getVTablePointers when doStructorsInitializeVPtrs, thanks.

================
Comment at: lib/CodeGen/ItaniumCXXABI.cpp:196
@@ +195,3 @@
+
+  bool doStructorsInitilizeVPtrs(const CXXRecordDecl *VTableClass) override {
+    return true;
----------------
Typo: "Initialize".


http://reviews.llvm.org/D11859