[PATCH] D11859: Generating vptr assume loads

Prazek created this revision. Prazek added reviewers: rsmith, majnemer. Prazek added a subscriber: cfe-commits. Generating vptr assumption loads for better devirtualization. More here: http://lists.llvm.org/pipermail/cfe-dev/2015-July/044227.html http://reviews.llvm.org/D11859 Files: include/

Re: [PATCH] D11859: Generating vptr assume loads

majnemer added a comment. Some first round review comments. Comment at: lib/CodeGen/CGClass.cpp:1831-1832 @@ +1830,4 @@ + + // generate vtable assumptions if we are not in another ctor and + // if we calling dynamic class ctor + if (CGM.getCodeGenOpts().OptimizationLevel > 0

Re: [PATCH] D11859: Generating vptr assume loads

Prazek added inline comments. Comment at: lib/CodeGen/CGClass.cpp:1842 @@ +1841,3 @@ + CGM.getCXXABI().getVTableAddressPoint(vptr.Base, vptr.VTableClass); + if (!VTableGlobal) return; + majnemer wrote: > The return should be on it's own line, please clang-fo

Re: [PATCH] D11859: Generating vptr assume loads

rsmith added inline comments. Comment at: include/clang/AST/VTableBuilder.h:387-391 @@ -386,7 +386,3 @@ - // Copy constructor. - // FIXME: Uncomment when we've moved to C++11. - // VPtrInfo(const VPtrInfo &) = default; - /// The vtable will hold all of the virtual bases or

Re: [PATCH] D11859: Generating vptr assume loads

rsmith added inline comments. Comment at: lib/CodeGen/CGClass.cpp:2098 @@ -2063,3 +2097,3 @@ // Initialize the vtable pointer for this base. -InitializeVTablePointer(Base, NearestVBase, OffsetFromNearestVBase, -VTableClass); +VPtr vptr{Base

Re: [PATCH] D11859: Generating vptr assume loads

djasper added a subscriber: djasper. Comment at: lib/CodeGen/CGClass.cpp:1842 @@ +1841,3 @@ + CGM.getCXXABI().getVTableAddressPoint(vptr.Base, vptr.VTableClass); + if (!VTableGlobal) return; + Prazek wrote: > majnemer wrote: > > The return should be on it's

Re: [PATCH] D11859: Generating vptr assume loads

Prazek marked 9 inline comments as done. Prazek added a comment. http://reviews.llvm.org/D11859 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Re: [PATCH] D11859: Generating vptr assume loads

Prazek added inline comments. Comment at: lib/CodeGen/CGClass.cpp:1842 @@ +1841,3 @@ + CGM.getCXXABI().getVTableAddressPoint(vptr.Base, vptr.VTableClass); + if (!VTableGlobal) return; + djasper wrote: > Prazek wrote: > > majnemer wrote: > > > The return shou

Re: [PATCH] D11859: Generating vptr assume loads

Prazek added inline comments. Comment at: lib/CodeGen/CodeGenFunction.h:1328-1334 @@ -1320,8 +1327,9 @@ typedef llvm::SmallPtrSet VisitedVirtualBasesSetTy; - void InitializeVTablePointers(BaseSubobject Base, -const CXXRecordDecl *NearestVBase, -

Re: [PATCH] D11859: Generating vptr assume loads

Prazek updated this revision to Diff 31766. http://reviews.llvm.org/D11859 Files: lib/CodeGen/CGCXXABI.h lib/CodeGen/CGClass.cpp lib/CodeGen/CodeGenFunction.h lib/CodeGen/ItaniumCXXABI.cpp lib/CodeGen/MicrosoftCXXABI.cpp test/CodeGen/available-externally-hidden.cpp test/CodeGenCXX/c

Re: [PATCH] D11859: Generating vptr assume loads

Prazek updated this revision to Diff 31768. http://reviews.llvm.org/D11859 Files: lib/CodeGen/CGCXXABI.h lib/CodeGen/CGClass.cpp lib/CodeGen/CodeGenFunction.h lib/CodeGen/ItaniumCXXABI.cpp lib/CodeGen/MicrosoftCXXABI.cpp test/CodeGen/available-externally-hidden.cpp test/CodeGenCXX/c

Re: [PATCH] D11859: Generating vptr assume loads

nwilson added a subscriber: nwilson. Comment at: lib/CodeGen/CGClass.cpp:1862 @@ +1861,3 @@ +if (CGM.getCXXABI().canInitializeVPtr(vptr.VTableClass, vptr.Base.getBase(), + vptr.NearestVBase)) + EmitVTableAssumptionLoad(vptr, This);

Re: [PATCH] D11859: Generating vptr assume loads

djasper added inline comments. Comment at: lib/CodeGen/CGClass.cpp:2155 @@ +2154,3 @@ +if (CGM.getCXXABI().canInitializeVPtr(Vptr.VTableClass, Vptr.Base.getBase(), + Vptr.NearestVBase)) + InitializeVTablePointer(Vptr);

Re: [PATCH] D11859: Generating vptr assume loads

nwilson added inline comments. Comment at: lib/CodeGen/CGClass.cpp:2155 @@ +2154,3 @@ +if (CGM.getCXXABI().canInitializeVPtr(Vptr.VTableClass, Vptr.Base.getBase(), + Vptr.NearestVBase)) + InitializeVTablePointer(Vptr);

Re: [PATCH] D11859: Generating vptr assume loads

Prazek marked 4 inline comments as done. Comment at: lib/CodeGen/ItaniumCXXABI.cpp:193-194 @@ -192,1 +192,4 @@ + bool isVirtualOffsetNeeded(CodeGenFunction &CGF, + const CXXRecordDecl *NearestVBase) override; + isVirtualOffsetNeededF

Re: [PATCH] D11859: Generating vptr assume loads

Prazek marked 3 inline comments as done. Prazek added a comment. http://reviews.llvm.org/D11859 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Re: [PATCH] D11859: Generating vptr assume loads

Prazek updated this revision to Diff 31846. http://reviews.llvm.org/D11859 Files: lib/CodeGen/CGCXXABI.h lib/CodeGen/CGClass.cpp lib/CodeGen/CodeGenFunction.h lib/CodeGen/ItaniumCXXABI.cpp lib/CodeGen/MicrosoftCXXABI.cpp test/CodeGen/available-externally-hidden.cpp test/CodeGenCXX/c

Re: [PATCH] D11859: Generating vptr assume loads

Prazek marked 4 inline comments as done. Prazek added a comment. http://reviews.llvm.org/D11859 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Re: [PATCH] D11859: Generating vptr assume loads

rsmith added inline comments. Comment at: lib/CodeGen/CGCXXABI.h:349-357 @@ -348,1 +348,11 @@ + virtual bool + isVirtualOffsetNeededForVTableField(CodeGenFunction &CGF, + const CXXRecordDecl *NearestVBase) = 0; + + virtual bool canInitiali

Re: [PATCH] D11859: Generating vptr assume loads

hfinkel added a subscriber: hfinkel. Comment at: lib/CodeGen/CGClass.cpp:1832 @@ +1831,3 @@ + // Generate vtable assumptions if we are calling dynamic class ctor + // and we are not in another ctor. + if (CGM.getCodeGenOpts().OptimizationLevel > 0 && I think th

Re: [PATCH] D11859: Generating vptr assume loads

Prazek added inline comments. Comment at: lib/CodeGen/CGClass.cpp:1832 @@ +1831,3 @@ + // Generate vtable assumptions if we are calling dynamic class ctor + // and we are not in another ctor. + if (CGM.getCodeGenOpts().OptimizationLevel > 0 && hfinkel wrote: >

Re: [PATCH] D11859: Generating vptr assume loads

rsmith added inline comments. Comment at: lib/CodeGen/CGClass.cpp:1832 @@ +1831,3 @@ + // Generate vtable assumptions if we are calling dynamic class ctor + // and we are not in another ctor. + if (CGM.getCodeGenOpts().OptimizationLevel > 0 && Prazek wrote: > h

Re: [PATCH] D11859: Generating vptr assume loads

Prazek marked 5 inline comments as done. Prazek added a comment. http://reviews.llvm.org/D11859 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Re: [PATCH] D11859: Generating vptr assume loads

Prazek marked an inline comment as done. Comment at: lib/CodeGen/CGClass.cpp:1832 @@ +1831,3 @@ + // Generate vtable assumptions if we are calling dynamic class ctor + // and we are not in another ctor. + if (CGM.getCodeGenOpts().OptimizationLevel > 0 && rsmith

Re: [PATCH] D11859: Generating vptr assume loads

rsmith added inline comments. Comment at: lib/CodeGen/CGClass.cpp:1832 @@ +1831,3 @@ + // Generate vtable assumptions if we are calling dynamic class ctor + // and we are not in another ctor. + if (CGM.getCodeGenOpts().OptimizationLevel > 0 && Prazek wrote: > r

Re: [PATCH] D11859: Generating vptr assume loads

Prazek marked 3 inline comments as done. Comment at: lib/CodeGen/CGClass.cpp:1832 @@ +1831,3 @@ + // Generate vtable assumptions if we are calling dynamic class ctor + // and we are not in another ctor. + if (CGM.getCodeGenOpts().OptimizationLevel > 0 && rsmith

Re: [PATCH] D11859: Generating vptr assume loads

Prazek updated this revision to Diff 32122. Prazek marked an inline comment as done. http://reviews.llvm.org/D11859 Files: lib/CodeGen/CGCXXABI.h lib/CodeGen/CGClass.cpp lib/CodeGen/CodeGenFunction.h lib/CodeGen/ItaniumCXXABI.cpp lib/CodeGen/MicrosoftCXXABI.cpp test/CodeGen/available-

Re: [PATCH] D11859: Generating vptr assume loads

rjmccall added a comment. Mostly LGTM. Are you going to emit assumptions for vbptrs in a separate patch? Comment at: lib/CodeGen/CGCXXABI.h:349-357 @@ -348,1 +348,11 @@ + /// Checks if ABI requires extra virtual offset for vtable field. + virtual bool + isVirtualOffsetNeed

Re: [PATCH] D11859: Generating vptr assume loads

majnemer added inline comments. Comment at: test/CodeGenCXX/vtable-assume-load.cpp:40 @@ +39,3 @@ +// CHECK1: call void @llvm.assume(i1 %cmp.vtables) +// CHECK1-LABLE: } + This check line looks misspelled. http://reviews.llvm.org/D11859 __

Re: [PATCH] D11859: Generating vptr assume loads

Prazek added a comment. In http://reviews.llvm.org/D11859#225025, @rjmccall wrote: > Mostly LGTM. Are you going to emit assumptions for vbptrs in a separate > patch? I wasn't planning to. I am focusing now on upgrading GVN for using new invariant.barrier metadata. http://reviews.llvm.org/D

Re: [PATCH] D11859: Generating vptr assume loads

rjmccall added a comment. In http://reviews.llvm.org/D11859#225384, @Prazek wrote: > In http://reviews.llvm.org/D11859#225025, @rjmccall wrote: > > > Mostly LGTM. Are you going to emit assumptions for vbptrs in a separate > > patch? > > > I wasn't planning to. I am focusing now on upgrading GVN

Re: [PATCH] D11859: Generating vptr assume loads

Prazek marked an inline comment as done. Comment at: lib/CodeGen/CGCXXABI.h:352 @@ +351,3 @@ + isVirtualOffsetNeededForVTableField(CodeGenFunction &CGF, + const CXXRecordDecl *NearestVBase) = 0; + rjmccall wrote: > This method

Re: [PATCH] D11859: Generating vptr assume loads

Prazek marked an inline comment as done. Prazek added a comment. In http://reviews.llvm.org/D11859#225404, @rjmccall wrote: > In http://reviews.llvm.org/D11859#225384, @Prazek wrote: > > > In http://reviews.llvm.org/D11859#225025, @rjmccall wrote: > > > > > Mostly LGTM. Are you going to emit ass

Re: [PATCH] D11859: Generating vptr assume loads

Prazek updated this revision to Diff 32262. http://reviews.llvm.org/D11859 Files: lib/CodeGen/CGCXXABI.h lib/CodeGen/CGClass.cpp lib/CodeGen/CodeGenFunction.h lib/CodeGen/ItaniumCXXABI.cpp lib/CodeGen/MicrosoftCXXABI.cpp test/CodeGen/available-externally-hidden.cpp test/CodeGenCXX/c

Re: [PATCH] D11859: Generating vptr assume loads

Prazek updated this revision to Diff 32263. http://reviews.llvm.org/D11859 Files: lib/CodeGen/CGCXXABI.h lib/CodeGen/CGClass.cpp lib/CodeGen/CodeGenFunction.h lib/CodeGen/ItaniumCXXABI.cpp lib/CodeGen/MicrosoftCXXABI.cpp test/CodeGen/available-externally-hidden.cpp test/CodeGenCXX/c

Re: [PATCH] D11859: Generating vptr assume loads

rjmccall added inline comments. Comment at: lib/CodeGen/CGClass.cpp:1862 @@ +1861,3 @@ + for (const VPtr &Vptr : getVTablePointers(ClassDecl)) +if (CGM.getCXXABI().requiresVPtrInitialization(Vptr)) + EmitVTableAssumptionLoad(Vptr, This); No, it only chec

Re: [PATCH] D11859: Generating vptr assume loads

Prazek marked 3 inline comments as done. Comment at: lib/CodeGen/CGClass.cpp:1862 @@ +1861,3 @@ + for (const VPtr &Vptr : getVTablePointers(ClassDecl)) +if (CGM.getCXXABI().requiresVPtrInitialization(Vptr)) + EmitVTableAssumptionLoad(Vptr, This); rjmccal

Re: [PATCH] D11859: Generating vptr assume loads

Prazek updated this revision to Diff 32343. Prazek marked an inline comment as done. http://reviews.llvm.org/D11859 Files: lib/CodeGen/CGCXXABI.h lib/CodeGen/CGClass.cpp lib/CodeGen/CodeGenFunction.h lib/CodeGen/ItaniumCXXABI.cpp lib/CodeGen/MicrosoftCXXABI.cpp test/CodeGen/available-

Re: [PATCH] D11859: Generating vptr assume loads

Prazek marked 3 inline comments as done. Prazek added a comment. http://reviews.llvm.org/D11859 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Re: [PATCH] D11859: Generating vptr assume loads

rjmccall added a comment. Just a couple tweaks and then LGTM. Comment at: lib/CodeGen/CGClass.cpp:1833 @@ +1832,3 @@ + // unless we are calling base constructor - we don't want to generating + // assumption loads for not completed because vptr may still change. + if (CGM.getC

Re: [PATCH] D11859: Generating vptr assume loads

Prazek updated this revision to Diff 32355. Prazek marked 2 inline comments as done. http://reviews.llvm.org/D11859 Files: lib/CodeGen/CGCXXABI.h lib/CodeGen/CGClass.cpp lib/CodeGen/CodeGenFunction.h lib/CodeGen/ItaniumCXXABI.cpp lib/CodeGen/MicrosoftCXXABI.cpp test/CodeGen/available-

Re: [PATCH] D11859: Generating vptr assume loads

Prazek updated this revision to Diff 32363. Prazek added a comment. Fix for the PR24479 http://reviews.llvm.org/D11859 Files: lib/CodeGen/CGCXXABI.h lib/CodeGen/CGCall.cpp lib/CodeGen/CGClass.cpp lib/CodeGen/CodeGenFunction.h lib/CodeGen/ItaniumCXXABI.cpp lib/CodeGen/MicrosoftCXXABI

Re: [PATCH] D11859: Generating vptr assume loads

rsmith accepted this revision. rsmith added a comment. This revision is now accepted and ready to land. LGTM Comment at: test/CodeGenCXX/vtable-assume-load.cpp:24 @@ +23,3 @@ +// CHECK1-LABEL: define void @_ZN5test14fooAEv() +// CHECK1: call void @_ZN5test11AC1Ev(%"struct.test1: