[PATCH] D22296: CodeGen: New vtable group representation: struct of vtable arrays.
This revision was automatically updated to reflect the committed changes.
pcc marked an inline comment as done.
Closed by commit rL289584: CodeGen: New vtable group representation: struct of
vtable arrays. (authored by pcc).
Changed prior to commit:
https://reviews.llvm.org/D22296?vs=81264&id=81288#toc
Repository:
rL LLVM
https://reviews.llvm.org/D22296
Files:
cfe/trunk/include/clang/AST/VTableBuilder.h
cfe/trunk/include/clang/Basic/LLVM.h
cfe/trunk/lib/AST/VTableBuilder.cpp
cfe/trunk/lib/CodeGen/CGCXX.cpp
cfe/trunk/lib/CodeGen/CGVTT.cpp
cfe/trunk/lib/CodeGen/CGVTables.cpp
cfe/trunk/lib/CodeGen/CGVTables.h
cfe/trunk/lib/CodeGen/ItaniumCXXABI.cpp
cfe/trunk/lib/CodeGen/MicrosoftCXXABI.cpp
cfe/trunk/test/CodeGenCXX/PR26569.cpp
cfe/trunk/test/CodeGenCXX/apple-kext-indirect-call-2.cpp
cfe/trunk/test/CodeGenCXX/apple-kext-indirect-call.cpp
cfe/trunk/test/CodeGenCXX/apple-kext-indirect-virtual-dtor-call.cpp
cfe/trunk/test/CodeGenCXX/cfi-cross-dso.cpp
cfe/trunk/test/CodeGenCXX/const-init-cxx11.cpp
cfe/trunk/test/CodeGenCXX/constructor-init.cpp
cfe/trunk/test/CodeGenCXX/copy-constructor-synthesis-2.cpp
cfe/trunk/test/CodeGenCXX/copy-constructor-synthesis.cpp
cfe/trunk/test/CodeGenCXX/ctor-dtor-alias.cpp
cfe/trunk/test/CodeGenCXX/dllexport.cpp
cfe/trunk/test/CodeGenCXX/dllimport-rtti.cpp
cfe/trunk/test/CodeGenCXX/dllimport.cpp
cfe/trunk/test/CodeGenCXX/invariant.group-for-vptrs.cpp
cfe/trunk/test/CodeGenCXX/key-function-vtable.cpp
cfe/trunk/test/CodeGenCXX/microsoft-abi-constexpr-vs-inheritance.cpp
cfe/trunk/test/CodeGenCXX/microsoft-abi-extern-template.cpp
cfe/trunk/test/CodeGenCXX/microsoft-abi-multiple-nonvirtual-inheritance.cpp
cfe/trunk/test/CodeGenCXX/microsoft-abi-structors.cpp
cfe/trunk/test/CodeGenCXX/microsoft-abi-vftables.cpp
cfe/trunk/test/CodeGenCXX/microsoft-abi-virtual-inheritance.cpp
cfe/trunk/test/CodeGenCXX/microsoft-abi-vtables-return-thunks.cpp
cfe/trunk/test/CodeGenCXX/microsoft-abi-vtables-single-inheritance.cpp
cfe/trunk/test/CodeGenCXX/microsoft-abi-vtables-virtual-inheritance.cpp
cfe/trunk/test/CodeGenCXX/microsoft-interface.cpp
cfe/trunk/test/CodeGenCXX/microsoft-no-rtti-data.cpp
cfe/trunk/test/CodeGenCXX/skip-vtable-pointer-initialization.cpp
cfe/trunk/test/CodeGenCXX/strict-vtable-pointers.cpp
cfe/trunk/test/CodeGenCXX/vtable-align.cpp
cfe/trunk/test/CodeGenCXX/vtable-assume-load.cpp
cfe/trunk/test/CodeGenCXX/vtable-pointer-initialization.cpp
cfe/trunk/test/CodeGenCXX/vtt-layout.cpp
Index: cfe/trunk/include/clang/AST/VTableBuilder.h
===
--- cfe/trunk/include/clang/AST/VTableBuilder.h
+++ cfe/trunk/include/clang/AST/VTableBuilder.h
@@ -218,52 +218,77 @@
class VTableLayout {
public:
typedef std::pair VTableThunkTy;
- typedef llvm::DenseMap AddressPointsMapTy;
+ struct AddressPointLocation {
+unsigned VTableIndex, AddressPointIndex;
+ };
+ typedef llvm::DenseMap
+ AddressPointsMapTy;
private:
- uint64_t NumVTableComponents;
- std::unique_ptr VTableComponents;
+ // Stores the component indices of the first component of each virtual table in
+ // the virtual table group. To save a little memory in the common case where
+ // the vtable group contains a single vtable, an empty vector here represents
+ // the vector {0}.
+ OwningArrayRef VTableIndices;
+
+ OwningArrayRef VTableComponents;
/// \brief Contains thunks needed by vtables, sorted by indices.
- uint64_t NumVTableThunks;
- std::unique_ptr VTableThunks;
+ OwningArrayRef VTableThunks;
/// \brief Address points for all vtables.
AddressPointsMapTy AddressPoints;
- bool IsMicrosoftABI;
-
public:
- VTableLayout(uint64_t NumVTableComponents,
- const VTableComponent *VTableComponents,
- uint64_t NumVTableThunks,
- const VTableThunkTy *VTableThunks,
- const AddressPointsMapTy &AddressPoints,
- bool IsMicrosoftABI);
+ VTableLayout(ArrayRef VTableIndices,
+ ArrayRef VTableComponents,
+ ArrayRef VTableThunks,
+ const AddressPointsMapTy &AddressPoints);
~VTableLayout();
ArrayRef vtable_components() const {
-return {VTableComponents.get(), size_t(NumVTableComponents)};
+return VTableComponents;
}
ArrayRef vtable_thunks() const {
-return {VTableThunks.get(), size_t(NumVTableThunks)};
+return VTableThunks;
}
- uint64_t getAddressPoint(BaseSubobject Base) const {
-assert(AddressPoints.count(Base) &&
- "Did not find address point!");
-
-uint64_t AddressPoint = AddressPoints.lookup(Base);
-assert(AddressPoint != 0 || IsMicrosoftABI);
-(void)IsMicrosoftABI;
-
-return AddressPoint;
+ AddressPointLocation getAddressPoint(BaseSubobject Base) const {
+assert(AddressPoints.count(Base) && "Did not find address point!");
+return AddressPoints.find(Base)->second;
}
con
[PATCH] D22296: CodeGen: New vtable group representation: struct of vtable arrays.
rjmccall accepted this revision. rjmccall added a comment. This revision is now accepted and ready to land. LGTM, thanks! https://reviews.llvm.org/D22296 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D22296: CodeGen: New vtable group representation: struct of vtable arrays.
pcc marked 3 inline comments as done.
pcc added inline comments.
Comment at: clang/include/clang/AST/VTableBuilder.h:255
+operator ArrayRef() const { return {data(), size()}; };
+ };
+
rjmccall wrote:
> Maybe this ought to be in LLVM as OwnedArrayRef? And the more minimal
> implementation approach would be to inherit from MutableArrayRef and just
> add a destructor and a move constructor.
>
> The implicit conversion to ArrayRef is dangerous, but only in ways that
> ArrayRef is already dangerous.
Good suggestions -- sent out D27723.
https://reviews.llvm.org/D22296
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D22296: CodeGen: New vtable group representation: struct of vtable arrays.
pcc updated this revision to Diff 81264.
pcc added a comment.
- Address review comments
https://reviews.llvm.org/D22296
Files:
clang/include/clang/AST/VTableBuilder.h
clang/include/clang/Basic/LLVM.h
clang/lib/AST/VTableBuilder.cpp
clang/lib/CodeGen/CGCXX.cpp
clang/lib/CodeGen/CGVTT.cpp
clang/lib/CodeGen/CGVTables.cpp
clang/lib/CodeGen/CGVTables.h
clang/lib/CodeGen/ItaniumCXXABI.cpp
clang/lib/CodeGen/MicrosoftCXXABI.cpp
clang/test/CodeGenCXX/PR26569.cpp
clang/test/CodeGenCXX/apple-kext-indirect-call-2.cpp
clang/test/CodeGenCXX/apple-kext-indirect-call.cpp
clang/test/CodeGenCXX/apple-kext-indirect-virtual-dtor-call.cpp
clang/test/CodeGenCXX/cfi-cross-dso.cpp
clang/test/CodeGenCXX/const-init-cxx11.cpp
clang/test/CodeGenCXX/constructor-init.cpp
clang/test/CodeGenCXX/copy-constructor-synthesis-2.cpp
clang/test/CodeGenCXX/copy-constructor-synthesis.cpp
clang/test/CodeGenCXX/ctor-dtor-alias.cpp
clang/test/CodeGenCXX/dllexport.cpp
clang/test/CodeGenCXX/dllimport-rtti.cpp
clang/test/CodeGenCXX/dllimport.cpp
clang/test/CodeGenCXX/invariant.group-for-vptrs.cpp
clang/test/CodeGenCXX/key-function-vtable.cpp
clang/test/CodeGenCXX/microsoft-abi-constexpr-vs-inheritance.cpp
clang/test/CodeGenCXX/microsoft-abi-extern-template.cpp
clang/test/CodeGenCXX/microsoft-abi-multiple-nonvirtual-inheritance.cpp
clang/test/CodeGenCXX/microsoft-abi-structors.cpp
clang/test/CodeGenCXX/microsoft-abi-vftables.cpp
clang/test/CodeGenCXX/microsoft-abi-virtual-inheritance.cpp
clang/test/CodeGenCXX/microsoft-abi-vtables-return-thunks.cpp
clang/test/CodeGenCXX/microsoft-abi-vtables-single-inheritance.cpp
clang/test/CodeGenCXX/microsoft-abi-vtables-virtual-inheritance.cpp
clang/test/CodeGenCXX/microsoft-interface.cpp
clang/test/CodeGenCXX/microsoft-no-rtti-data.cpp
clang/test/CodeGenCXX/skip-vtable-pointer-initialization.cpp
clang/test/CodeGenCXX/strict-vtable-pointers.cpp
clang/test/CodeGenCXX/vtable-align.cpp
clang/test/CodeGenCXX/vtable-assume-load.cpp
clang/test/CodeGenCXX/vtable-pointer-initialization.cpp
clang/test/CodeGenCXX/vtt-layout.cpp
Index: clang/test/CodeGenCXX/vtt-layout.cpp
===
--- clang/test/CodeGenCXX/vtt-layout.cpp
+++ clang/test/CodeGenCXX/vtt-layout.cpp
@@ -78,11 +78,12 @@
}
}
-// CHECK: @_ZTTN5Test11BE = unnamed_addr constant [1 x i8*] [i8* bitcast (i8** getelementptr inbounds ([4 x i8*], [4 x i8*]* @_ZTVN5Test11BE, i32 0, i32 3) to i8*)]
-// CHECK: @_ZTVN5Test51AE = unnamed_addr constant [4 x i8*] [i8* null, i8* bitcast ({ i8*, i8* }* @_ZTIN5Test51AE to i8*), i8* bitcast (void ()* @__cxa_pure_virtual to i8*), i8* bitcast (void (%"struct.Test5::A"*)* @_ZN5Test51A6anchorEv to i8*)]
-// CHECK: @_ZTVN5Test61AE = unnamed_addr constant [4 x i8*] [i8* null, i8* bitcast ({ i8*, i8* }* @_ZTIN5Test61AE to i8*), i8* bitcast (void ()* @__cxa_deleted_virtual to i8*), i8* bitcast (void (%"struct.Test6::A"*)* @_ZN5Test61A6anchorEv to i8*)]
-// CHECK: @_ZTTN5Test21CE = linkonce_odr unnamed_addr constant [2 x i8*] [i8* bitcast (i8** getelementptr inbounds ([5 x i8*], [5 x i8*]* @_ZTVN5Test21CE, i32 0, i32 4) to i8*), i8* bitcast (i8** getelementptr inbounds ([5 x i8*], [5 x i8*]* @_ZTVN5Test21CE, i32 0, i32 4) to i8*)]
-// CHECK: @_ZTTN5Test31DE = linkonce_odr unnamed_addr constant [13 x i8*] [i8* bitcast (i8** getelementptr inbounds ([19 x i8*], [19 x i8*]* @_ZTVN5Test31DE, i32 0, i32 5) to i8*), i8* bitcast (i8** getelementptr inbounds ([7 x i8*], [7 x i8*]* @_ZTCN5Test31DE0_NS_2C1E, i32 0, i32 3) to i8*), i8* bitcast (i8** getelementptr inbounds ([7 x i8*], [7 x i8*]* @_ZTCN5Test31DE0_NS_2C1E, i32 0, i32 6) to i8*), i8* bitcast (i8** getelementptr inbounds ([14 x i8*], [14 x i8*]* @_ZTCN5Test31DE16_NS_2C2E, i32 0, i32 6) to i8*), i8* bitcast (i8** getelementptr inbounds ([14 x i8*], [14 x i8*]* @_ZTCN5Test31DE16_NS_2C2E, i32 0, i32 6) to i8*), i8* bitcast (i8** getelementptr inbounds ([14 x i8*], [14 x i8*]* @_ZTCN5Test31DE16_NS_2C2E, i32 0, i32 10) to i8*), i8* bitcast (i8** getelementptr inbounds ([14 x i8*], [14 x i8*]* @_ZTCN5Test31DE16_NS_2C2E, i32 0, i32 13) to i8*), i8* bitcast (i8** getelementptr inbounds ([19 x i8*], [19 x i8*]* @_ZTVN5Test31DE, i32 0, i32 15) to i8*), i8* bitcast (i8** getelementptr inbounds ([19 x i8*], [19 x i8*]* @_ZTVN5Test31DE, i32 0, i32 11) to i8*), i8* bitcast (i8** getelementptr inbounds ([19 x i8*], [19 x i8*]* @_ZTVN5Test31DE, i32 0, i32 11) to i8*), i8* bitcast (i8** getelementptr inbounds ([19 x i8*], [19 x i8*]* @_ZTVN5Test31DE, i64 1, i32 0) to i8*), i8* bitcast (i8** getelementptr inbounds ([7 x i8*], [7 x i8*]* @_ZTCN5Test31DE64_NS_2V2E, i32 0, i32 3) to i8*), i8* bitcast (i8** getelementptr inbounds ([7 x i8*], [7 x i8*]* @_ZTCN5Test31DE64_NS_2V2E, i32 0, i32 6) to i8*)]
-// CHECK: @_ZTTN5Test41DE = linkonce_odr unnamed_addr constant [19 x i8*] [i8* bitcast (i8** getelementptr inbounds ([25 x i8*], [25 x i8*]* @_ZTVN5Test41DE, i32 0, i
[PATCH] D22296: CodeGen: New vtable group representation: struct of vtable arrays.
rjmccall added a comment.
A couple minor suggestions, then LGTM.
Comment at: clang/include/clang/AST/VTableBuilder.h:255
+operator ArrayRef() const { return {data(), size()}; };
+ };
+
Maybe this ought to be in LLVM as OwnedArrayRef? And the more minimal
implementation approach would be to inherit from MutableArrayRef and just
add a destructor and a move constructor.
The implicit conversion to ArrayRef is dangerous, but only in ways that
ArrayRef is already dangerous.
Comment at: clang/include/clang/AST/VTableBuilder.h:260
+ // the vtable group contains a single vtable, an empty vector here represents
+ // the vector {0}.
+ FixedSizeVector VTableIndices;
Ah. Yes, that's a reasonable solution.
Comment at: clang/lib/AST/VTableBuilder.cpp:986
+ /// vtable group.
+ SmallVector VTableIndices;
+
Same comment here. Might as well give this enough capacity for any reasonably
foreseeable case.
Comment at: clang/lib/CodeGen/CGVTables.cpp:641
+llvm::Type *CodeGenVTables::getVTableType(const VTableLayout &layout) {
+ SmallVector tys;
+ for (unsigned i = 0, e = layout.getNumVTables(); i != e; ++i) {
While 1 is definitely the most likely size, adding small amounts of extra
inline capacity to a temporary SmallVector has basically zero cost, and of
course those cases do happen. 4 sounds reasonable.
https://reviews.llvm.org/D22296
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D22296: CodeGen: New vtable group representation: struct of vtable arrays.
pcc updated this revision to Diff 80844.
pcc marked 6 inline comments as done.
pcc added a comment.
- Address review comments
https://reviews.llvm.org/D22296
Files:
clang/include/clang/AST/VTableBuilder.h
clang/lib/AST/VTableBuilder.cpp
clang/lib/CodeGen/CGCXX.cpp
clang/lib/CodeGen/CGVTT.cpp
clang/lib/CodeGen/CGVTables.cpp
clang/lib/CodeGen/CGVTables.h
clang/lib/CodeGen/ItaniumCXXABI.cpp
clang/lib/CodeGen/MicrosoftCXXABI.cpp
clang/test/CodeGenCXX/PR26569.cpp
clang/test/CodeGenCXX/apple-kext-indirect-call-2.cpp
clang/test/CodeGenCXX/apple-kext-indirect-call.cpp
clang/test/CodeGenCXX/apple-kext-indirect-virtual-dtor-call.cpp
clang/test/CodeGenCXX/cfi-cross-dso.cpp
clang/test/CodeGenCXX/const-init-cxx11.cpp
clang/test/CodeGenCXX/constructor-init.cpp
clang/test/CodeGenCXX/copy-constructor-synthesis-2.cpp
clang/test/CodeGenCXX/copy-constructor-synthesis.cpp
clang/test/CodeGenCXX/ctor-dtor-alias.cpp
clang/test/CodeGenCXX/dllexport.cpp
clang/test/CodeGenCXX/dllimport-rtti.cpp
clang/test/CodeGenCXX/dllimport.cpp
clang/test/CodeGenCXX/invariant.group-for-vptrs.cpp
clang/test/CodeGenCXX/key-function-vtable.cpp
clang/test/CodeGenCXX/microsoft-abi-constexpr-vs-inheritance.cpp
clang/test/CodeGenCXX/microsoft-abi-extern-template.cpp
clang/test/CodeGenCXX/microsoft-abi-multiple-nonvirtual-inheritance.cpp
clang/test/CodeGenCXX/microsoft-abi-structors.cpp
clang/test/CodeGenCXX/microsoft-abi-vftables.cpp
clang/test/CodeGenCXX/microsoft-abi-virtual-inheritance.cpp
clang/test/CodeGenCXX/microsoft-abi-vtables-return-thunks.cpp
clang/test/CodeGenCXX/microsoft-abi-vtables-single-inheritance.cpp
clang/test/CodeGenCXX/microsoft-abi-vtables-virtual-inheritance.cpp
clang/test/CodeGenCXX/microsoft-interface.cpp
clang/test/CodeGenCXX/microsoft-no-rtti-data.cpp
clang/test/CodeGenCXX/skip-vtable-pointer-initialization.cpp
clang/test/CodeGenCXX/strict-vtable-pointers.cpp
clang/test/CodeGenCXX/vtable-align.cpp
clang/test/CodeGenCXX/vtable-assume-load.cpp
clang/test/CodeGenCXX/vtable-pointer-initialization.cpp
clang/test/CodeGenCXX/vtt-layout.cpp
Index: clang/test/CodeGenCXX/vtt-layout.cpp
===
--- clang/test/CodeGenCXX/vtt-layout.cpp
+++ clang/test/CodeGenCXX/vtt-layout.cpp
@@ -78,11 +78,12 @@
}
}
-// CHECK: @_ZTTN5Test11BE = unnamed_addr constant [1 x i8*] [i8* bitcast (i8** getelementptr inbounds ([4 x i8*], [4 x i8*]* @_ZTVN5Test11BE, i32 0, i32 3) to i8*)]
-// CHECK: @_ZTVN5Test51AE = unnamed_addr constant [4 x i8*] [i8* null, i8* bitcast ({ i8*, i8* }* @_ZTIN5Test51AE to i8*), i8* bitcast (void ()* @__cxa_pure_virtual to i8*), i8* bitcast (void (%"struct.Test5::A"*)* @_ZN5Test51A6anchorEv to i8*)]
-// CHECK: @_ZTVN5Test61AE = unnamed_addr constant [4 x i8*] [i8* null, i8* bitcast ({ i8*, i8* }* @_ZTIN5Test61AE to i8*), i8* bitcast (void ()* @__cxa_deleted_virtual to i8*), i8* bitcast (void (%"struct.Test6::A"*)* @_ZN5Test61A6anchorEv to i8*)]
-// CHECK: @_ZTTN5Test21CE = linkonce_odr unnamed_addr constant [2 x i8*] [i8* bitcast (i8** getelementptr inbounds ([5 x i8*], [5 x i8*]* @_ZTVN5Test21CE, i32 0, i32 4) to i8*), i8* bitcast (i8** getelementptr inbounds ([5 x i8*], [5 x i8*]* @_ZTVN5Test21CE, i32 0, i32 4) to i8*)]
-// CHECK: @_ZTTN5Test31DE = linkonce_odr unnamed_addr constant [13 x i8*] [i8* bitcast (i8** getelementptr inbounds ([19 x i8*], [19 x i8*]* @_ZTVN5Test31DE, i32 0, i32 5) to i8*), i8* bitcast (i8** getelementptr inbounds ([7 x i8*], [7 x i8*]* @_ZTCN5Test31DE0_NS_2C1E, i32 0, i32 3) to i8*), i8* bitcast (i8** getelementptr inbounds ([7 x i8*], [7 x i8*]* @_ZTCN5Test31DE0_NS_2C1E, i32 0, i32 6) to i8*), i8* bitcast (i8** getelementptr inbounds ([14 x i8*], [14 x i8*]* @_ZTCN5Test31DE16_NS_2C2E, i32 0, i32 6) to i8*), i8* bitcast (i8** getelementptr inbounds ([14 x i8*], [14 x i8*]* @_ZTCN5Test31DE16_NS_2C2E, i32 0, i32 6) to i8*), i8* bitcast (i8** getelementptr inbounds ([14 x i8*], [14 x i8*]* @_ZTCN5Test31DE16_NS_2C2E, i32 0, i32 10) to i8*), i8* bitcast (i8** getelementptr inbounds ([14 x i8*], [14 x i8*]* @_ZTCN5Test31DE16_NS_2C2E, i32 0, i32 13) to i8*), i8* bitcast (i8** getelementptr inbounds ([19 x i8*], [19 x i8*]* @_ZTVN5Test31DE, i32 0, i32 15) to i8*), i8* bitcast (i8** getelementptr inbounds ([19 x i8*], [19 x i8*]* @_ZTVN5Test31DE, i32 0, i32 11) to i8*), i8* bitcast (i8** getelementptr inbounds ([19 x i8*], [19 x i8*]* @_ZTVN5Test31DE, i32 0, i32 11) to i8*), i8* bitcast (i8** getelementptr inbounds ([19 x i8*], [19 x i8*]* @_ZTVN5Test31DE, i64 1, i32 0) to i8*), i8* bitcast (i8** getelementptr inbounds ([7 x i8*], [7 x i8*]* @_ZTCN5Test31DE64_NS_2V2E, i32 0, i32 3) to i8*), i8* bitcast (i8** getelementptr inbounds ([7 x i8*], [7 x i8*]* @_ZTCN5Test31DE64_NS_2V2E, i32 0, i32 6) to i8*)]
-// CHECK: @_ZTTN5Test41DE = linkonce_odr unnamed_addr constant [19 x i8*] [i8* bitcast (i8** getelementptr inbounds ([25 x i8*], [25 x i8*]* @_ZTVN5Test41DE, i32 0
[PATCH] D22296: CodeGen: New vtable group representation: struct of vtable arrays.
pcc added inline comments. Comment at: clang/include/clang/AST/VTableBuilder.h:225 private: + std::vector VTableIndices; + rjmccall wrote: > Does this actually grow, or should you use a representation more like > VTableComponents? Or something custom that exploits the fact that, in the > vast majority of cases, this vector will have size 1? I wrapped up the code for VTableComponents and VTableThunks into a class, and used it here with some special handling for the size == 1 case. Comment at: clang/include/clang/AST/VTableBuilder.h:260 +assert(AddressPoint.second != 0 || IsMicrosoftABI); (void)IsMicrosoftABI; rjmccall wrote: > I know you didn't add this line, but could you remove this cast? > IsMicrosoftABI is not a local variable. > > Also, the assert above suggests that this can be: > ... = AddressPoint.find(Base)->second; > which should be slightly more efficient than lookup(). Also, I think the > assertion becomes unnecessary at that point. > Also, I think the assertion becomes unnecessary at that point. Yes, and so does IsMicrosoftABI. Also removed a similar assertion from one of the callers in CGVTT.cpp. https://reviews.llvm.org/D22296 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D22296: CodeGen: New vtable group representation: struct of vtable arrays.
rjmccall added a comment.
This seems reasonable to me. Just a few representational / API requests.
Comment at: clang/include/clang/AST/VTableBuilder.h:222
+ typedef llvm::DenseMap>
+ AddressPointsMapTy;
Please use a struct instead of std::pair.
Comment at: clang/include/clang/AST/VTableBuilder.h:225
private:
+ std::vector VTableIndices;
+
Does this actually grow, or should you use a representation more like
VTableComponents? Or something custom that exploits the fact that, in the vast
majority of cases, this vector will have size 1?
Comment at: clang/include/clang/AST/VTableBuilder.h:243
+ uint64_t NumVTableThunks, const VTableThunkTy *VTableThunks,
+ const AddressPointsMapTy &AddressPoints, bool IsMicrosoftABI);
~VTableLayout();
Since you're changing this interface anyway, would you mind fixing it to use
ArrayRef for VTableComponents and VTableThunks, too?
Comment at: clang/include/clang/AST/VTableBuilder.h:260
+assert(AddressPoint.second != 0 || IsMicrosoftABI);
(void)IsMicrosoftABI;
I know you didn't add this line, but could you remove this cast?
IsMicrosoftABI is not a local variable.
Also, the assert above suggests that this can be:
... = AddressPoint.find(Base)->second;
which should be slightly more efficient than lookup(). Also, I think the
assertion becomes unnecessary at that point.
Comment at: clang/lib/AST/VTableBuilder.cpp:986
+ /// vtable group.
+ std::vector VTableIndices;
+
Assuming you stop storing a std::vector in the VTableLayout, you should make
this a SmallVector.
Comment at: clang/lib/CodeGen/CGVTables.cpp:644
+
+ std::vector tys;
+ for (unsigned i = 0, e = idxs.size(); i != e; ++i) {
SmallVector, please.
Comment at: clang/lib/CodeGen/CGVTables.cpp:648
+size_t nextIndex =
+(i + 1 == e) ? layout.vtable_components().size() : idxs[i + 1];
+tys.push_back(llvm::ArrayType::get(CGM.Int8PtrTy, nextIndex - thisIndex));
This seems like a completely reasonable query to add to VTableLayout:
unsigned getVTableSize(unsigned vtableIndex) const;
or maybe:
ArrayRef getVTableComponents(unsigned vtableIndex) const;
I guess you'll also need a getNumVTables().
https://reviews.llvm.org/D22296
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D22296: CodeGen: New vtable group representation: struct of vtable arrays.
pcc added a reviewer: rjmccall. pcc added a comment. John, may I ask you to take a look? https://reviews.llvm.org/D22296 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D22296: CodeGen: New vtable group representation: struct of vtable arrays.
pcc updated this revision to Diff 80709.
pcc added a comment.
Refresh
https://reviews.llvm.org/D22296
Files:
clang/include/clang/AST/VTableBuilder.h
clang/lib/AST/VTableBuilder.cpp
clang/lib/CodeGen/CGCXX.cpp
clang/lib/CodeGen/CGVTT.cpp
clang/lib/CodeGen/CGVTables.cpp
clang/lib/CodeGen/CGVTables.h
clang/lib/CodeGen/ItaniumCXXABI.cpp
clang/lib/CodeGen/MicrosoftCXXABI.cpp
clang/test/CodeGenCXX/PR26569.cpp
clang/test/CodeGenCXX/apple-kext-indirect-call-2.cpp
clang/test/CodeGenCXX/apple-kext-indirect-call.cpp
clang/test/CodeGenCXX/apple-kext-indirect-virtual-dtor-call.cpp
clang/test/CodeGenCXX/cfi-cross-dso.cpp
clang/test/CodeGenCXX/const-init-cxx11.cpp
clang/test/CodeGenCXX/constructor-init.cpp
clang/test/CodeGenCXX/copy-constructor-synthesis-2.cpp
clang/test/CodeGenCXX/copy-constructor-synthesis.cpp
clang/test/CodeGenCXX/ctor-dtor-alias.cpp
clang/test/CodeGenCXX/dllexport.cpp
clang/test/CodeGenCXX/dllimport-rtti.cpp
clang/test/CodeGenCXX/dllimport.cpp
clang/test/CodeGenCXX/invariant.group-for-vptrs.cpp
clang/test/CodeGenCXX/key-function-vtable.cpp
clang/test/CodeGenCXX/microsoft-abi-constexpr-vs-inheritance.cpp
clang/test/CodeGenCXX/microsoft-abi-extern-template.cpp
clang/test/CodeGenCXX/microsoft-abi-multiple-nonvirtual-inheritance.cpp
clang/test/CodeGenCXX/microsoft-abi-structors.cpp
clang/test/CodeGenCXX/microsoft-abi-vftables.cpp
clang/test/CodeGenCXX/microsoft-abi-virtual-inheritance.cpp
clang/test/CodeGenCXX/microsoft-abi-vtables-return-thunks.cpp
clang/test/CodeGenCXX/microsoft-abi-vtables-single-inheritance.cpp
clang/test/CodeGenCXX/microsoft-abi-vtables-virtual-inheritance.cpp
clang/test/CodeGenCXX/microsoft-interface.cpp
clang/test/CodeGenCXX/microsoft-no-rtti-data.cpp
clang/test/CodeGenCXX/skip-vtable-pointer-initialization.cpp
clang/test/CodeGenCXX/strict-vtable-pointers.cpp
clang/test/CodeGenCXX/vtable-align.cpp
clang/test/CodeGenCXX/vtable-assume-load.cpp
clang/test/CodeGenCXX/vtable-pointer-initialization.cpp
clang/test/CodeGenCXX/vtt-layout.cpp
Index: clang/test/CodeGenCXX/vtt-layout.cpp
===
--- clang/test/CodeGenCXX/vtt-layout.cpp
+++ clang/test/CodeGenCXX/vtt-layout.cpp
@@ -78,11 +78,12 @@
}
}
-// CHECK: @_ZTTN5Test11BE = unnamed_addr constant [1 x i8*] [i8* bitcast (i8** getelementptr inbounds ([4 x i8*], [4 x i8*]* @_ZTVN5Test11BE, i32 0, i32 3) to i8*)]
-// CHECK: @_ZTVN5Test51AE = unnamed_addr constant [4 x i8*] [i8* null, i8* bitcast ({ i8*, i8* }* @_ZTIN5Test51AE to i8*), i8* bitcast (void ()* @__cxa_pure_virtual to i8*), i8* bitcast (void (%"struct.Test5::A"*)* @_ZN5Test51A6anchorEv to i8*)]
-// CHECK: @_ZTVN5Test61AE = unnamed_addr constant [4 x i8*] [i8* null, i8* bitcast ({ i8*, i8* }* @_ZTIN5Test61AE to i8*), i8* bitcast (void ()* @__cxa_deleted_virtual to i8*), i8* bitcast (void (%"struct.Test6::A"*)* @_ZN5Test61A6anchorEv to i8*)]
-// CHECK: @_ZTTN5Test21CE = linkonce_odr unnamed_addr constant [2 x i8*] [i8* bitcast (i8** getelementptr inbounds ([5 x i8*], [5 x i8*]* @_ZTVN5Test21CE, i32 0, i32 4) to i8*), i8* bitcast (i8** getelementptr inbounds ([5 x i8*], [5 x i8*]* @_ZTVN5Test21CE, i32 0, i32 4) to i8*)]
-// CHECK: @_ZTTN5Test31DE = linkonce_odr unnamed_addr constant [13 x i8*] [i8* bitcast (i8** getelementptr inbounds ([19 x i8*], [19 x i8*]* @_ZTVN5Test31DE, i32 0, i32 5) to i8*), i8* bitcast (i8** getelementptr inbounds ([7 x i8*], [7 x i8*]* @_ZTCN5Test31DE0_NS_2C1E, i32 0, i32 3) to i8*), i8* bitcast (i8** getelementptr inbounds ([7 x i8*], [7 x i8*]* @_ZTCN5Test31DE0_NS_2C1E, i32 0, i32 6) to i8*), i8* bitcast (i8** getelementptr inbounds ([14 x i8*], [14 x i8*]* @_ZTCN5Test31DE16_NS_2C2E, i32 0, i32 6) to i8*), i8* bitcast (i8** getelementptr inbounds ([14 x i8*], [14 x i8*]* @_ZTCN5Test31DE16_NS_2C2E, i32 0, i32 6) to i8*), i8* bitcast (i8** getelementptr inbounds ([14 x i8*], [14 x i8*]* @_ZTCN5Test31DE16_NS_2C2E, i32 0, i32 10) to i8*), i8* bitcast (i8** getelementptr inbounds ([14 x i8*], [14 x i8*]* @_ZTCN5Test31DE16_NS_2C2E, i32 0, i32 13) to i8*), i8* bitcast (i8** getelementptr inbounds ([19 x i8*], [19 x i8*]* @_ZTVN5Test31DE, i32 0, i32 15) to i8*), i8* bitcast (i8** getelementptr inbounds ([19 x i8*], [19 x i8*]* @_ZTVN5Test31DE, i32 0, i32 11) to i8*), i8* bitcast (i8** getelementptr inbounds ([19 x i8*], [19 x i8*]* @_ZTVN5Test31DE, i32 0, i32 11) to i8*), i8* bitcast (i8** getelementptr inbounds ([19 x i8*], [19 x i8*]* @_ZTVN5Test31DE, i64 1, i32 0) to i8*), i8* bitcast (i8** getelementptr inbounds ([7 x i8*], [7 x i8*]* @_ZTCN5Test31DE64_NS_2V2E, i32 0, i32 3) to i8*), i8* bitcast (i8** getelementptr inbounds ([7 x i8*], [7 x i8*]* @_ZTCN5Test31DE64_NS_2V2E, i32 0, i32 6) to i8*)]
-// CHECK: @_ZTTN5Test41DE = linkonce_odr unnamed_addr constant [19 x i8*] [i8* bitcast (i8** getelementptr inbounds ([25 x i8*], [25 x i8*]* @_ZTVN5Test41DE, i32 0, i32 6) to i8*), i8* bitcast (i8** getelementptr inboun
[PATCH] D22296: CodeGen: New vtable group representation: struct of vtable arrays.
pcc added a comment. Ping https://reviews.llvm.org/D22296 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D22296: CodeGen: New vtable group representation: struct of vtable arrays.
pcc updated this revision to Diff 75493.
pcc added a comment.
Refresh
https://reviews.llvm.org/D22296
Files:
clang/include/clang/AST/VTableBuilder.h
clang/lib/AST/VTableBuilder.cpp
clang/lib/CodeGen/CGCXX.cpp
clang/lib/CodeGen/CGVTT.cpp
clang/lib/CodeGen/CGVTables.cpp
clang/lib/CodeGen/CGVTables.h
clang/lib/CodeGen/ItaniumCXXABI.cpp
clang/lib/CodeGen/MicrosoftCXXABI.cpp
clang/test/CodeGenCXX/PR26569.cpp
clang/test/CodeGenCXX/apple-kext-indirect-call-2.cpp
clang/test/CodeGenCXX/apple-kext-indirect-call.cpp
clang/test/CodeGenCXX/apple-kext-indirect-virtual-dtor-call.cpp
clang/test/CodeGenCXX/cfi-cross-dso.cpp
clang/test/CodeGenCXX/const-init-cxx11.cpp
clang/test/CodeGenCXX/constructor-init.cpp
clang/test/CodeGenCXX/copy-constructor-synthesis-2.cpp
clang/test/CodeGenCXX/copy-constructor-synthesis.cpp
clang/test/CodeGenCXX/ctor-dtor-alias.cpp
clang/test/CodeGenCXX/dllexport.cpp
clang/test/CodeGenCXX/dllimport-rtti.cpp
clang/test/CodeGenCXX/dllimport.cpp
clang/test/CodeGenCXX/invariant.group-for-vptrs.cpp
clang/test/CodeGenCXX/key-function-vtable.cpp
clang/test/CodeGenCXX/microsoft-abi-constexpr-vs-inheritance.cpp
clang/test/CodeGenCXX/microsoft-abi-extern-template.cpp
clang/test/CodeGenCXX/microsoft-abi-multiple-nonvirtual-inheritance.cpp
clang/test/CodeGenCXX/microsoft-abi-structors.cpp
clang/test/CodeGenCXX/microsoft-abi-vftables.cpp
clang/test/CodeGenCXX/microsoft-abi-virtual-inheritance.cpp
clang/test/CodeGenCXX/microsoft-abi-vtables-return-thunks.cpp
clang/test/CodeGenCXX/microsoft-abi-vtables-single-inheritance.cpp
clang/test/CodeGenCXX/microsoft-abi-vtables-virtual-inheritance.cpp
clang/test/CodeGenCXX/microsoft-interface.cpp
clang/test/CodeGenCXX/microsoft-no-rtti-data.cpp
clang/test/CodeGenCXX/skip-vtable-pointer-initialization.cpp
clang/test/CodeGenCXX/strict-vtable-pointers.cpp
clang/test/CodeGenCXX/vtable-align.cpp
clang/test/CodeGenCXX/vtable-assume-load.cpp
clang/test/CodeGenCXX/vtable-pointer-initialization.cpp
clang/test/CodeGenCXX/vtt-layout.cpp
Index: clang/test/CodeGenCXX/vtt-layout.cpp
===
--- clang/test/CodeGenCXX/vtt-layout.cpp
+++ clang/test/CodeGenCXX/vtt-layout.cpp
@@ -78,11 +78,12 @@
}
}
-// CHECK: @_ZTTN5Test11BE = unnamed_addr constant [1 x i8*] [i8* bitcast (i8** getelementptr inbounds ([4 x i8*], [4 x i8*]* @_ZTVN5Test11BE, i32 0, i32 3) to i8*)]
-// CHECK: @_ZTVN5Test51AE = unnamed_addr constant [4 x i8*] [i8* null, i8* bitcast ({ i8*, i8* }* @_ZTIN5Test51AE to i8*), i8* bitcast (void ()* @__cxa_pure_virtual to i8*), i8* bitcast (void (%"struct.Test5::A"*)* @_ZN5Test51A6anchorEv to i8*)]
-// CHECK: @_ZTVN5Test61AE = unnamed_addr constant [4 x i8*] [i8* null, i8* bitcast ({ i8*, i8* }* @_ZTIN5Test61AE to i8*), i8* bitcast (void ()* @__cxa_deleted_virtual to i8*), i8* bitcast (void (%"struct.Test6::A"*)* @_ZN5Test61A6anchorEv to i8*)]
-// CHECK: @_ZTTN5Test21CE = linkonce_odr unnamed_addr constant [2 x i8*] [i8* bitcast (i8** getelementptr inbounds ([5 x i8*], [5 x i8*]* @_ZTVN5Test21CE, i32 0, i32 4) to i8*), i8* bitcast (i8** getelementptr inbounds ([5 x i8*], [5 x i8*]* @_ZTVN5Test21CE, i32 0, i32 4) to i8*)]
-// CHECK: @_ZTTN5Test31DE = linkonce_odr unnamed_addr constant [13 x i8*] [i8* bitcast (i8** getelementptr inbounds ([19 x i8*], [19 x i8*]* @_ZTVN5Test31DE, i32 0, i32 5) to i8*), i8* bitcast (i8** getelementptr inbounds ([7 x i8*], [7 x i8*]* @_ZTCN5Test31DE0_NS_2C1E, i32 0, i32 3) to i8*), i8* bitcast (i8** getelementptr inbounds ([7 x i8*], [7 x i8*]* @_ZTCN5Test31DE0_NS_2C1E, i32 0, i32 6) to i8*), i8* bitcast (i8** getelementptr inbounds ([14 x i8*], [14 x i8*]* @_ZTCN5Test31DE16_NS_2C2E, i32 0, i32 6) to i8*), i8* bitcast (i8** getelementptr inbounds ([14 x i8*], [14 x i8*]* @_ZTCN5Test31DE16_NS_2C2E, i32 0, i32 6) to i8*), i8* bitcast (i8** getelementptr inbounds ([14 x i8*], [14 x i8*]* @_ZTCN5Test31DE16_NS_2C2E, i32 0, i32 10) to i8*), i8* bitcast (i8** getelementptr inbounds ([14 x i8*], [14 x i8*]* @_ZTCN5Test31DE16_NS_2C2E, i32 0, i32 13) to i8*), i8* bitcast (i8** getelementptr inbounds ([19 x i8*], [19 x i8*]* @_ZTVN5Test31DE, i32 0, i32 15) to i8*), i8* bitcast (i8** getelementptr inbounds ([19 x i8*], [19 x i8*]* @_ZTVN5Test31DE, i32 0, i32 11) to i8*), i8* bitcast (i8** getelementptr inbounds ([19 x i8*], [19 x i8*]* @_ZTVN5Test31DE, i32 0, i32 11) to i8*), i8* bitcast (i8** getelementptr inbounds ([19 x i8*], [19 x i8*]* @_ZTVN5Test31DE, i64 1, i32 0) to i8*), i8* bitcast (i8** getelementptr inbounds ([7 x i8*], [7 x i8*]* @_ZTCN5Test31DE64_NS_2V2E, i32 0, i32 3) to i8*), i8* bitcast (i8** getelementptr inbounds ([7 x i8*], [7 x i8*]* @_ZTCN5Test31DE64_NS_2V2E, i32 0, i32 6) to i8*)]
-// CHECK: @_ZTTN5Test41DE = linkonce_odr unnamed_addr constant [19 x i8*] [i8* bitcast (i8** getelementptr inbounds ([25 x i8*], [25 x i8*]* @_ZTVN5Test41DE, i32 0, i32 6) to i8*), i8* bitcast (i8** getelementptr inboun
Re: [PATCH] D22296: CodeGen: New vtable group representation: struct of vtable arrays.
pcc added a comment. This should now be ready for review and unblocked by other changes (I split out the inrange annotation to https://reviews.llvm.org/D24431). https://reviews.llvm.org/D22296 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
