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
