[PATCH] D31830: Emit invariant.group.barrier when using union field

John McCall via Phabricator via cfe-commits cfe-commits at lists.llvm.org
Fri May 26 07:43:11 PDT 2017

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.


More information about the cfe-commits mailing list