rjmccall added inline comments.

================
Comment at: lib/CodeGen/CGExpr.cpp:3517
+        CGM.getCodeGenOpts().StrictVTablePointers &&
+        CGM.getCodeGenOpts().OptimizationLevel > 0)
+      addr = Address(Builder.CreateInvariantGroupBarrier(addr.getPointer()),
----------------
Prazek wrote:
> rjmccall wrote:
> > Prazek wrote:
> > > rjmccall wrote:
> > > > Prazek wrote:
> > > > > rjmccall wrote:
> > > > > > Checking for v-table pointers recursively isn't really that 
> > > > > > difficult, but if you really don't want to do that, please at least 
> > > > > > check for non-POD-ness or something so that this isn't kicking in 
> > > > > > for literally every struct type.
> > > > > ok, I am also planning to fix this in the later patch, because the 
> > > > > same problem arise when comparing 2 pointers to dynamic classe. 
> > > > > I would like to have a bit in CXXRecordDecl to remember if it has any 
> > > > > vptr inside that would calculate durring the construction. 
> > > > > My biggest fear is that if I won't cache it then it will be very slow.
> > > > We could repeat this work from scratch on every single union field 
> > > > access done by IRGen and it would still be way, way faster than doing 
> > > > anything extra on every record definition in the program.  The latter 
> > > > is done orders of magnitude more frequently than the former.
> > > This is a good point, and actually I don't need to check if class holds 
> > > any vptrs for the example I refered (comparing 2 pointers).
> > > Hovewer I still need to do it when someone casts pointer to class holding 
> > > vptrs to something that doesn't hold vptrs, and any pointer casts are 
> > > much more frequent than Record declarations.
> > What are you planning to do if someone casts to a pointer to incomplete 
> > type?
> > 
> > I am concerned that what these problems are really telling us is that this 
> > representation is not very good.
> In this case we also need to add the barrier. I might be wrong, but I think 
> this in general should be pretty rare and even more rare if it would limit 
> the devirtualization. 
> For example cases like: 
> 
>   Derived *d;
>   (Base*)d;
> 
> will not limit devirtualization if Derived is complete type (because base 
> will be also).
> 
> The other thing is that with my recent patches, adding more barriers will not 
> limit any memory optimizations.
You don't need the .getTypePtr() here.


================
Comment at: lib/CodeGen/CGExpr.cpp:3530
+  return false;
+}
+
----------------
You need to recurse into base classes (to check their fields), and if you write 
this to take a QualType you won't have to eagerly extract RD below.


https://reviews.llvm.org/D31830



_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to