TypeGraph: Calculate alignment before identifying containers

Not all containers have 8-byte alignment, so if we want to avoid lots of
manual logic for calculating container alignment on a case-by-case
basis, we must calculate alignment from the member variables before the
Class nodes have been replaced by Container nodes.
This commit is contained in:
Alastair Robertson 2023-11-02 06:22:57 -07:00 committed by Alastair Robertson
parent 2c5fb5d845
commit c207972af6
4 changed files with 14 additions and 24 deletions

View File

@ -1168,6 +1168,7 @@ void CodeGen::transform(TypeGraph& typeGraph) {
// Simplify the type graph first so there is less work for later passes // Simplify the type graph first so there is less work for later passes
pm.addPass(RemoveTopLevelPointer::createPass()); pm.addPass(RemoveTopLevelPointer::createPass());
pm.addPass(Flattener::createPass()); pm.addPass(Flattener::createPass());
pm.addPass(AlignmentCalc::createPass());
pm.addPass(IdentifyContainers::createPass(containerInfos_)); pm.addPass(IdentifyContainers::createPass(containerInfos_));
pm.addPass(TypeIdentifier::createPass(config_.passThroughTypes)); pm.addPass(TypeIdentifier::createPass(config_.passThroughTypes));
if (config_.features[Feature::PruneTypeGraph]) if (config_.features[Feature::PruneTypeGraph])
@ -1183,16 +1184,13 @@ void CodeGen::transform(TypeGraph& typeGraph) {
// Re-run passes over newly added children // Re-run passes over newly added children
pm.addPass(Flattener::createPass()); pm.addPass(Flattener::createPass());
pm.addPass(AlignmentCalc::createPass());
pm.addPass(IdentifyContainers::createPass(containerInfos_)); pm.addPass(IdentifyContainers::createPass(containerInfos_));
pm.addPass(TypeIdentifier::createPass(config_.passThroughTypes)); pm.addPass(TypeIdentifier::createPass(config_.passThroughTypes));
if (config_.features[Feature::PruneTypeGraph]) if (config_.features[Feature::PruneTypeGraph])
pm.addPass(Prune::createPass()); pm.addPass(Prune::createPass());
} }
// Calculate alignment before removing members, as those members may have an
// influence on the class' overall alignment.
pm.addPass(AlignmentCalc::createPass());
pm.addPass(RemoveMembers::createPass(config_.membersToStub)); pm.addPass(RemoveMembers::createPass(config_.membersToStub));
if (!config_.features[Feature::TreeBuilderV2]) if (!config_.features[Feature::TreeBuilderV2])
pm.addPass(EnforceCompatibility::createPass()); pm.addPass(EnforceCompatibility::createPass());

View File

@ -101,6 +101,8 @@ struct alignas(align) DummySizedOperator {
// container if an empty class is passed. // container if an empty class is passed.
template <int32_t Id> template <int32_t Id>
struct DummySizedOperator<0, 0, Id> {}; struct DummySizedOperator<0, 0, Id> {};
template <int32_t Id>
struct DummySizedOperator<0, 1, Id> {};
template <template <typename, size_t, size_t, int32_t> typename DerivedT, template <template <typename, size_t, size_t, int32_t> typename DerivedT,
typename T, typename T,
@ -130,6 +132,9 @@ struct alignas(Align) DummyAllocator
template <typename T, int32_t Id> template <typename T, int32_t Id>
struct DummyAllocator<T, 0, 0, Id> struct DummyAllocator<T, 0, 0, Id>
: DummyAllocatorBase<DummyAllocator, T, 0, 0, Id> {}; : DummyAllocatorBase<DummyAllocator, T, 0, 0, Id> {};
template <typename T, int32_t Id>
struct DummyAllocator<T, 0, 1, Id>
: DummyAllocatorBase<DummyAllocator, T, 0, 1, Id> {};
template <typename Type, size_t ExpectedSize, size_t ActualSize = 0> template <typename Type, size_t ExpectedSize, size_t ActualSize = 0>
struct validate_size { struct validate_size {

View File

@ -421,20 +421,7 @@ class Container : public Type {
} }
virtual uint64_t align() const override { virtual uint64_t align() const override {
// XXX Big hack! return align_;
// We can not assume that all containers have 8-byte alignment. However, to
// properly calculate alignment would require significant refactoring of
// the container identification code:
// - DrgnParser needs to return Class objects instead of Containers
// - They must be flattened
// - We must calculate alignment based on their members
// - Then we are able to convert them into Container objects
//
// As a quick, hopefully temporary solution, hardcode some known container
// alignments here
if (containerInfo_.ctype == THRIFT_ISSET_TYPE)
return 1;
return 8;
} }
virtual NodeId id() const override { virtual NodeId id() const override {

View File

@ -62,11 +62,11 @@ TEST(CodeGenTest, TransformContainerAllocator) {
Function: deallocate Function: deallocate
)", )",
R"( R"(
[2] Container: std::vector<int32_t, DummyAllocator<int32_t, 8, 0, 3>> (size: 24) [2] Container: std::vector<int32_t, DummyAllocator<int32_t, 8, 1, 3>> (size: 24)
Param Param
Primitive: int32_t Primitive: int32_t
Param Param
[3] DummyAllocator [MyAlloc] (size: 8) [3] DummyAllocator [MyAlloc] (size: 8, align: 1)
Primitive: int32_t Primitive: int32_t
)"); )");
} }
@ -95,13 +95,13 @@ TEST(CodeGenTest, TransformContainerAllocatorParamInParent) {
Function: deallocate Function: deallocate
)", )",
R"( R"(
[4] Container: std::map<int32_t, int32_t, DummyAllocator<std::pair<int32_t const, int32_t>, 0, 0, 6>> (size: 24) [4] Container: std::map<int32_t, int32_t, DummyAllocator<std::pair<int32_t const, int32_t>, 0, 1, 6>> (size: 24)
Param Param
Primitive: int32_t Primitive: int32_t
Param Param
Primitive: int32_t Primitive: int32_t
Param Param
[6] DummyAllocator [MyAlloc<std::pair<const int, int>>] (size: 0) [6] DummyAllocator [MyAlloc<std::pair<const int, int>>] (size: 0, align: 1)
[5] Container: std::pair<int32_t const, int32_t> (size: 8) [5] Container: std::pair<int32_t const, int32_t> (size: 8)
Param Param
Primitive: int32_t Primitive: int32_t
@ -167,11 +167,11 @@ TEST(CodeGenTest, ReplaceContainersAndDummies) {
Function: deallocate Function: deallocate
)", )",
R"( R"(
[2] Container: std::vector<uint32_t, DummyAllocator<uint32_t, 0, 0, 3>> (size: 24) [2] Container: std::vector<uint32_t, DummyAllocator<uint32_t, 0, 1, 3>> (size: 24)
Param Param
Primitive: uint32_t Primitive: uint32_t
Param Param
[3] DummyAllocator [allocator<int>] (size: 0) [3] DummyAllocator [allocator<int>] (size: 0, align: 1)
Primitive: uint32_t Primitive: uint32_t
)"); )");
} }