[PATCH] D11859: Generating vptr assume loads

[Richard Smith via cfe-commits]

rsmith added inline comments.

================
Comment at: lib/CodeGen/CGClass.cpp:1832
@@ +1831,3 @@
+  // Generate vtable assumptions if we are calling dynamic class ctor
+  // and we are not in another ctor.
+  if (CGM.getCodeGenOpts().OptimizationLevel > 0 &&
----------------
Prazek wrote:
> rsmith wrote:
> > Prazek wrote:
> > > hfinkel wrote:
> > > > I think this comment should be a little more verbose. How about this:
> > > > 
> > > >   // Generate vtable assumptions if we are calling dynamic-class's ctor, except when doing so as part of a derived class's ctor's base-class initialization. Doing so in this latter case would be useless, because the vtable is about to be overwritten by the derived class's vtable.
> > > > 
> > > The main point of not calling this function, is because it is useless to have assumption loads inside constructor, when they are generated also outside of ctor. I guess You could have case when You are not overriding base vptr, and You are calling base ctor from dynamic class
> > It might also be worth including in the comment that it is not correct to call `EmitVTableAssumptionLoads` here, because it assumes the object's vptr points to the complete object vtable; during a constructor call it will probably have a construction vtable instead.
> Well,  it would be correct, but it would be useless. Just after calling base constructor, assume loads would appear, and after there would be new store to vptr.
`EmitVTableAssumptionLoads` assumes that the `This` pointer points to an object whose most-derived type is `ClassDecl`, and it assumes that the object is not under construction. Both of those assumptions can be violated for a base class constructor call, and can result in it emitting incorrect assumptions (because virtual bases may be at different offsets and because the vptr of an object under construction may be different from the corresponding vptr of a fully-constructed object).


http://reviews.llvm.org/D11859