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:
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
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-
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
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
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-
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
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
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
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
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
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
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
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
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
__
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
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-
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
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
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
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
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
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:
>
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
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
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
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
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
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
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);
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);
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);
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
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
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,
-
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
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
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
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
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
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
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
42 matches
Mail list logo