Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

TypeGraph: Add "--tree-builder-v2" flag #210

Merged
merged 1 commit into from
Jul 6, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion oi/CodeGen.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -778,7 +778,7 @@ void CodeGen::transform(type_graph::TypeGraph& typeGraph) {
type_graph::TypeIdentifier::createPass(config_.passThroughTypes));
}
pm.addPass(type_graph::RemoveIgnored::createPass(config_.membersToStub));
pm.addPass(type_graph::AddPadding::createPass());
pm.addPass(type_graph::AddPadding::createPass(config_.features));
pm.addPass(type_graph::NameGen::createPass());
pm.addPass(type_graph::AlignmentCalc::createPass());
pm.addPass(type_graph::RemoveTopLevelPointer::createPass());
Expand Down
4 changes: 3 additions & 1 deletion oi/Features.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -33,9 +33,11 @@ std::string_view featureHelp(Feature f) {
case Feature::CaptureThriftIsset:
return "Capture isset data for Thrift object.";
case Feature::TypeGraph:
return "Use Type Graph for code generation (CodeGen V2).";
return "Use Type Graph for code generation (CodeGen v2).";
case Feature::TypedDataSegment:
return "Use Typed Data Segment in generated code.";
case Feature::TreeBuilderV2:
return "Use Tree Builder v2 for reading the data segment";
case Feature::GenJitDebug:
return "Generate debug information for the JIT object.";
case Feature::JitLogging:
Expand Down
1 change: 1 addition & 0 deletions oi/Features.h
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@
X(CaptureThriftIsset, "capture-thrift-isset") \
X(TypeGraph, "type-graph") \
X(TypedDataSegment, "typed-data-segment") \
X(TreeBuilderV2, "tree-builder-v2") \
X(GenJitDebug, "gen-jit-debug") \
X(JitLogging, "jit-logging") \
X(JitTiming, "jit-timing") \
Expand Down
23 changes: 20 additions & 3 deletions oi/type_graph/AddPadding.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -17,16 +17,20 @@

#include <cassert>

#include "Flattener.h"
#include "TypeGraph.h"

template <typename T>
using ref = std::reference_wrapper<T>;

using ObjectIntrospection::Feature;
using ObjectIntrospection::FeatureSet;

namespace type_graph {

Pass AddPadding::createPass() {
auto fn = [](TypeGraph& typeGraph) {
AddPadding pass(typeGraph);
Pass AddPadding::createPass(FeatureSet features) {
auto fn = [features](TypeGraph& typeGraph) {
AddPadding pass(typeGraph, features);
for (auto& type : typeGraph.rootTypes()) {
pass.visit(type);
}
Expand Down Expand Up @@ -67,6 +71,19 @@ void AddPadding::visit(Class& c) {
if (i >= 1) {
addPadding(c.members[i - 1], c.members[i].bitOffset, paddedMembers);
}

if (!features_[Feature::TreeBuilderV2] &&
c.members[i].name.starts_with(Flattener::ParentPrefix)) {
// CodeGen v1 can't handle parent containers. It replaces them with
// padding.
auto& primitive = typeGraph_.makeType<Primitive>(Primitive::Kind::Int8);
auto& paddingArray =
typeGraph_.makeType<Array>(primitive, c.members[i].type().size());
paddedMembers.emplace_back(paddingArray, MemberPrefix,
c.members[i].bitOffset);
continue;
}

paddedMembers.push_back(c.members[i]);
}

Expand Down
10 changes: 7 additions & 3 deletions oi/type_graph/AddPadding.h
Original file line number Diff line number Diff line change
Expand Up @@ -15,12 +15,13 @@
*/
#pragma once

#include <functional>
#include <string>
#include <unordered_set>

#include "PassManager.h"
#include "Types.h"
#include "Visitor.h"
#include "oi/Features.h"

namespace type_graph {

Expand All @@ -35,9 +36,11 @@ class TypeGraph;
*/
class AddPadding final : public RecursiveVisitor {
public:
static Pass createPass();
static Pass createPass(ObjectIntrospection::FeatureSet features);

explicit AddPadding(TypeGraph& typeGraph) : typeGraph_(typeGraph) {
explicit AddPadding(TypeGraph& typeGraph,
ObjectIntrospection::FeatureSet features)
: typeGraph_(typeGraph), features_(features) {
}

using RecursiveVisitor::visit;
Expand All @@ -50,6 +53,7 @@ class AddPadding final : public RecursiveVisitor {
private:
std::unordered_set<Type*> visited_;
TypeGraph& typeGraph_;
ObjectIntrospection::FeatureSet features_;

void addPadding(const Member& prevMember,
uint64_t paddingEndBits,
Expand Down
2 changes: 1 addition & 1 deletion oi/type_graph/Flattener.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -71,7 +71,7 @@ void flattenParent(const Parent& parent,
} else if (Container* parentContainer =
dynamic_cast<Container*>(&parentType)) {
// Create a new member to represent this parent container
flattenedMembers.emplace_back(*parentContainer, "__parent",
flattenedMembers.emplace_back(*parentContainer, Flattener::ParentPrefix,
parent.bitOffset);
} else {
throw std::runtime_error("Invalid type for parent");
Expand Down
3 changes: 3 additions & 0 deletions oi/type_graph/Flattener.h
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@
*/
#pragma once

#include <string>
#include <unordered_set>
#include <vector>

Expand Down Expand Up @@ -42,6 +43,8 @@ class Flattener : public RecursiveVisitor {
void visit(Class& c) override;
void visit(Container& c) override;

static const inline std::string ParentPrefix = "__oi_parent";

private:
std::unordered_set<Type*> visited_;
std::vector<Member> flattened_members_;
Expand Down
44 changes: 40 additions & 4 deletions test/test_add_padding.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,17 @@
#include "test/type_graph_utils.h"

using namespace type_graph;
using ObjectIntrospection::Feature;
using ObjectIntrospection::FeatureSet;

namespace {
void test(std::vector<std::reference_wrapper<type_graph::Type>> rootTypes,
std::string_view expectedAfter) {
FeatureSet features;
features[Feature::TreeBuilderV2] = true;
::test(AddPadding::createPass({}), rootTypes, expectedAfter);
}
} // namespace

TEST(AddPaddingTest, BetweenMembers) {
auto myclass = Class{0, Class::Kind::Class, "MyClass", 16};
Expand All @@ -13,7 +24,7 @@ TEST(AddPaddingTest, BetweenMembers) {
myclass.members.push_back(Member{myint8, "n1", 0});
myclass.members.push_back(Member{myint64, "n2", 8 * 8});

test(AddPadding::createPass(), {myclass}, R"(
test({myclass}, R"(
[0] Class: MyClass (size: 16)
Member: n1 (offset: 0)
Primitive: int8_t
Expand All @@ -32,7 +43,7 @@ TEST(AddPaddingTest, AtEnd) {
myclass.members.push_back(Member{myint64, "n1", 0});
myclass.members.push_back(Member{myint8, "n2", 8 * 8});

test(AddPadding::createPass(), {myclass}, R"(
test({myclass}, R"(
[0] Struct: MyStruct (size: 16)
Member: n1 (offset: 0)
Primitive: int64_t
Expand All @@ -51,7 +62,7 @@ TEST(AddPaddingTest, UnionNotPadded) {
myclass.members.push_back(Member{myint64, "n1", 0});
myclass.members.push_back(Member{myint8, "n2", 0});

test(AddPadding::createPass(), {myclass}, R"(
test({myclass}, R"(
[0] Union: MyUnion (size: 8)
Member: n1 (offset: 0)
Primitive: int64_t
Expand Down Expand Up @@ -86,7 +97,7 @@ TEST(AddPaddingTest, Bitfields) {
myclass.members.push_back(b4);
myclass.members.push_back(n);

test(AddPadding::createPass(), {myclass}, R"(
test({myclass}, R"(
[0] Class: MyClass (size: 16)
Member: b1 (offset: 0, bitsize: 3)
Primitive: int64_t
Expand All @@ -111,4 +122,29 @@ TEST(AddPaddingTest, Bitfields) {
)");
}

TEST(AddPaddingTest, CodeGenCompatibility) {
auto myint = Primitive{Primitive::Kind::Int32};
auto vector = getVector(1);
vector.templateParams.push_back(TemplateParam{myint});

auto myclass = Class{0, Class::Kind::Class, "MyClass", 24};
myclass.members.push_back(Member{vector, "__oi_parent", 0});

FeatureSet features;
features[Feature::TreeBuilderV2] = false;
test(AddPadding::createPass(features), {myclass}, R"(
[0] Class: MyClass (size: 24)
Member: __oi_parent (offset: 0)
[1] Container: std::vector (size: 24)
Param
Primitive: int32_t
)",
R"(
[0] Class: MyClass (size: 24)
Member: __oi_padding (offset: 0)
[1] Array: (length: 24)
Primitive: int8_t
)");
}

// TODO test we follow class members, templates, children
8 changes: 4 additions & 4 deletions test/test_flattener.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -705,7 +705,7 @@ TEST(FlattenerTest, ParentContainer) {
)",
R"(
[0] Class: ClassA (size: 32)
Member: __parent (offset: 0)
Member: __oi_parent (offset: 0)
[1] Container: std::vector (size: 24)
Param
Primitive: int32_t
Expand Down Expand Up @@ -735,11 +735,11 @@ TEST(FlattenerTest, ParentTwoContainers) {
)",
R"(
[0] Class: ClassA (size: 48)
Member: __parent (offset: 0)
Member: __oi_parent (offset: 0)
[1] Container: std::vector (size: 24)
Param
Primitive: int32_t
Member: __parent (offset: 24)
Member: __oi_parent (offset: 24)
[1]
)");
}
Expand Down Expand Up @@ -772,7 +772,7 @@ TEST(FlattenerTest, ParentClassAndContainer) {
[0] Class: ClassA (size: 32)
Member: b (offset: 0)
Primitive: int32_t
Member: __parent (offset: 8)
Member: __oi_parent (offset: 8)
[1] Container: std::vector (size: 24)
Param
Primitive: int32_t
Expand Down