Re: [PATCH] D13373: Emiting invariant.group.barrier for ctors bugfix

2015-10-02 Thread Richard Smith via cfe-commits
rsmith accepted this revision. rsmith added a comment. This revision is now accepted and ready to land. LGTM, but your commit message should be more descriptive than your current summary. Something like: Ensure that the vptr store in the most-derived constructor is not behind an invariant

Re: [PATCH] D13373: Emiting invariant.group.barrier for ctors bugfix

2015-10-02 Thread Piotr Padlewski via cfe-commits
Prazek updated the summary for this revision. Prazek updated this revision to Diff 36382. Prazek marked an inline comment as done. http://reviews.llvm.org/D13373 Files: lib/CodeGen/CGClass.cpp test/CodeGenCXX/invariant.group-for-vptrs.cpp test/CodeGenCXX/strict-vtable-pointers.cpp Index:

Re: [PATCH] D13373: Emiting invariant.group.barrier for ctors bugfix

2015-10-02 Thread NAKAMURA Takumi via cfe-commits
chapuni added a subscriber: chapuni. Comment at: test/CodeGenCXX/strict-vtable-pointers.cpp:145 @@ +144,3 @@ +// CHECK-CTORS: %[[THIS3:.*]] = bitcast i8* %[[THIS2]] to %[[DynamicDerived]]* +// CHECK-CTORS: %[[THIS4:.*]] = bitcast %[[DynamicDerived]]* %2 to %[[DynamicBase:.*]]*

Re: [PATCH] D13373: Emiting invariant.group.barrier for ctors bugfix

2015-10-02 Thread Piotr Padlewski via cfe-commits
Thank you! On Fri, Oct 2, 2015 at 5:54 PM, NAKAMURA Takumi wrote: > chapuni added a subscriber: chapuni. > > > Comment at: test/CodeGenCXX/strict-vtable-pointers.cpp:145 > @@ +144,3 @@ > +// CHECK-CTORS: %[[THIS3:.*]] = bitcast i8* %[[THIS2]] to >

[PATCH] D13373: Emiting invariant.group.barrier for ctors bugfix

2015-10-01 Thread Piotr Padlewski via cfe-commits
Prazek created this revision. Prazek added reviewers: rsmith, nlewycky, rjmccall, majnemer. Prazek added a subscriber: cfe-commits. Please review asap http://reviews.llvm.org/D13373 Files: lib/CodeGen/CGClass.cpp test/CodeGenCXX/invariant.group-for-vptrs.cpp

Re: [PATCH] D13373: Emiting invariant.group.barrier for ctors bugfix

2015-10-01 Thread Nick Lewycky via cfe-commits
nlewycky added a comment. I can't meaningfully review this, but I see nothing wrong here. Comment at: lib/CodeGen/CGClass.cpp:1378 @@ -1377,3 +1377,3 @@ - bool BaseVPtrsInitialized = false; + llvm::Value* const OldThis = CXXThisValue; // Virtual base initializers first.