teemperor created this revision.
teemperor added reviewers: dblaikie, rjmccall, rsmith, v.g.vassilev.
teemperor added a project: clang.
Herald added a subscriber: dexonsmith.
teemperor requested review of this revision.
Herald added subscribers: llvm-commits, cfe-commits.
Herald added a project: LLVM.
This is a WIP patch that tries to avoid creating a RecordLayout in Clang and
instead just emit an opaque structure type
as if we only had a forward declarations. The main motivation for this patch is
actually just supporting a use case in LLDB
where laying out types can be very expensive as it usually triggers parsing of
debug information.
The changes in this patch can be summarized as:
- `CodeGenTypes::ConvertRecordDeclType` (and related funcs) have a new
parameter that tells us if we need the definition. It's currently only set to
false for Clang pointer types.
- There are a few new places where I added (temporary) calls to
`ConvertTypeForMem()` on some pointee types. The reason is that the code after
is usually creating GEP instructions where we need a non-opaque source type. We
can't do this automatically from the GEP factory methods as they would need to
know the clang::Type to automatically do this (and they only have the
llvm::Type that can't be mapped back to a clang::Type from what I can see, but
that might be incorrect).
- A few test that needed to be adjusted as they relied on e.g. `Foo *x` to be
enough to force `Foo` to be laid out/emitted.
There are still about a dozen more tests failing but from what I can see they
all just need to be adjusted to force specific types to be emitted. I'll fix
those up once there is consensus that this patch is going in the right
direction.
Some benchmarks: I did a stage2 build of LLVM+Clang with my patch and those are
the stats:
current ToT Clang:
2232421 - total amount of struct types created
94911 - of which are opaque struct types
with this patch:
1715074 - total amount of struct types created (-23%)
173127 - of which are opaque struct types (+82%)
I built a part of Clang (the last 300 source files in the
compile_commands.json) and the average time on my 64 core machine changes like
this (as per hyperfine):
Benchmark #1: parallel --progress -j63 -a ToT-clang
Time (mean ± σ): 27.703 s ± 0.168 s[User: 1434.619 s, System:
66.687 s]
Range (min … max): 27.459 s … 27.891 s10 runs
Benchmark #2: parallel --progress -j63 -a with-patch
Time (mean ± σ): 27.439 s ± 0.111 s[User: 1427.739 s, System:
66.220 s]
Range (min … max): 27.300 s … 27.625 s10 runs
Summary
'parallel --progress -j63 -a with-patch' ran
1.01 ± 0.01 times faster than 'parallel --progress -j63 -a ToT-clang'
Repository:
rG LLVM Github Monorepo
https://reviews.llvm.org/D108407
Files:
clang/include/clang/AST/Type.h
clang/lib/CodeGen/CGExpr.cpp
clang/lib/CodeGen/CGExprCXX.cpp
clang/lib/CodeGen/CGExprScalar.cpp
clang/lib/CodeGen/CGOpenMPRuntime.cpp
clang/lib/CodeGen/CodeGenTypes.cpp
clang/lib/CodeGen/CodeGenTypes.h
clang/test/CodeGen/c11atomics.c
clang/test/CodeGenCXX/class-layout.cpp
clang/test/CodeGenCXX/pr18962.cpp
clang/test/CodeGenCXX/warn-padded-packed.cpp
llvm/include/llvm/IR/Instructions.h
Index: llvm/include/llvm/IR/Instructions.h
===
--- llvm/include/llvm/IR/Instructions.h
+++ llvm/include/llvm/IR/Instructions.h
@@ -1173,6 +1173,7 @@
ResultElementType(getIndexedType(PointeeType, IdxList)) {
assert(cast(getType()->getScalarType())
->isOpaqueOrPointeeTypeMatches(ResultElementType));
+ assert(PointeeType->isSized());
init(Ptr, IdxList, NameStr);
}
@@ -1187,6 +1188,7 @@
ResultElementType(getIndexedType(PointeeType, IdxList)) {
assert(cast(getType()->getScalarType())
->isOpaqueOrPointeeTypeMatches(ResultElementType));
+ assert(PointeeType->isSized());
init(Ptr, IdxList, NameStr);
}
Index: clang/test/CodeGenCXX/warn-padded-packed.cpp
===
--- clang/test/CodeGenCXX/warn-padded-packed.cpp
+++ clang/test/CodeGenCXX/warn-padded-packed.cpp
@@ -148,6 +148,6 @@
// The warnings are emitted when the layout of the structs is computed, so we have to use them.
-void f(S1*, S2*, S3*, S4*, S5*, S6*, S7*, S8*, S9*, S10*, S11*, S12*, S13*,
- S14*, S15*, S16*, S17*, S18*, S19*, S20*, S21*, S22*, S23*, S24*, S25*,
- S26*, S27*){}
+void f(S1, S2, S3, S4, S5, S6, S7, S8, S9, S10, S11, S12, S13,
+ S14, S15, S16, S17, S18, S19, S20, S21, S22, S23, S24, S25,
+ S26, S27){}
Index: clang/test/CodeGenCXX/pr18962.cpp
===
--- clang/test/CodeGenCXX/pr18962.cpp
+++ clang/test/CodeGenCXX/pr18962.cpp
@@ -27,6 +27,5 @@
// We end up using an opaque type for 'append' to avoid circular references.
// CHECK: %class.A =
