[PATCH] D12312: Emiting invariant.group.barrier and adding -fstrict-vptrs

[John McCall via cfe-commits]

rjmccall added inline comments.

================
Comment at: include/clang/Driver/Options.td:990
@@ -988,2 +989,3 @@
            "value range">;
+def fstrict_vptrs: Flag<["-"], "fstrict-vptrs">, Group<f_Group>, Flags<[CC1Option]>;
 def fstrict_overflow : Flag<["-"], "fstrict-overflow">, Group<f_Group>;
----------------
This needs documentation for the --help output, something like "Enable optimizations based on the strict rules for overwriting polymorphic C++ objects".

This option should eventually be promoted to be a driver option, so we might as well figure out the name now.  I'd rather not introduce "vptr" to the user lexicon.  I suggest -fstrict-vtable-pointers.

================
Comment at: lib/CodeGen/CGClass.cpp:1279
@@ +1278,3 @@
+  if (CGM.getCodeGenOpts().StrictVPtrs && BaseVPtrsInitialized)
+    CXXThisValue = Builder.CreateInvariantGroupBarrier(LoadCXXThis());
+
----------------
Prazek wrote:
> rjmccall wrote:
> > Should this just be in InitializeVTablePointers?
> I want to add invariant.group.barrier only if it's needed. F.e. I don't want to put before I initialize vptrs for base, or when my class doesn't inherit frome anything. I want emit barrier after I will initialize some other vptrs.
> 
> InitializeVptrs is called in EmitBaseInitializer, and also I woudnt want to put some extra flag if it must produce barrier or not (because it is hard to distinguish it from inside)
Fair enough.

Do we need to emit these barriers in unoptimized builds?

================
Comment at: lib/CodeGen/CGClass.cpp:1487
@@ +1486,3 @@
+    if (!CanSkipVTablePointerInitialization(getContext(), Dtor)) {
+      // Inserting llvm.invariant.group.barrier intrinsic before base vptrs
+      // initialization to stop propagating previous vptr value.
----------------
Grammar: "Insert the llvm.invariant.group.barrier intrinsic before initializing the vptrs to cancel any previous assumptions we might have made."


http://reviews.llvm.org/D12312