Skip to content

Commit

Permalink
Refactor ReferenceResolver to use native C++-enumerators in some plac…
Browse files Browse the repository at this point in the history 
…es (#4432)

Refactor ReferenceResolver to get rid of majority of unnecessary heap allocations due to obsessive `Enumerator<>` usage:
  - Use iterators and iterator ranges where possible
  - Ensure we do not allocate on heap temporary objects and exploit move semantics as much as possible returning vectors by value
  - Do some refactoring around `Enumerator<>->toVector()`: make it return result by value. It seems that the use in the tree is always about creating some temporaries. And it does not make any sense to allocate vectors on heap then
  - Few small improvements here and there, e.g. removing dependence from boost adapters here
 
Partially addresses #4412

Ideally, we'd switch from memory allocation at `getDeclarations` / `getDeclsByName`, but it's a bit long way to go for now.
  • Loading branch information
@asl
asl authored Feb 23, 2024
1 parent 7312048 commit e14d108
Show file tree
Hide file tree
Showing 16 changed files with 238 additions and 177 deletions.
11 changes: 4 additions & 7 deletions backends/p4tools/common/compiler/reachability.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -225,14 +225,11 @@ bool P4ProgramDCGCreator::preorder(const IR::P4Program *program) {
}
return pathExpr->path->name == decl->name;
};
const auto *declVector = program->getDeclarations()->where(filter)->toVector();
BUG_CHECK(!declVector->empty(), "Not a declaration instance: %1%", pathExpr);

// Convert the declaration instance into a constructor-call expression.
const auto *decl = declVector->at(0)->to<IR::Declaration_Instance>();
auto *ctorCall =
new IR::ConstructorCallExpression(decl->srcInfo, decl->type, decl->arguments);
v.push_back(ctorCall);
const auto *decl =
program->getDeclarations()->where(filter)->single()->to<IR::Declaration_Instance>();
v.push_back(
new IR::ConstructorCallExpression(decl->srcInfo, decl->type, decl->arguments));
continue;
}
}
Expand Down
18 changes: 4 additions & 14 deletions backends/p4tools/common/lib/namespace_context.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -37,13 +37,8 @@ const IR::IDeclaration *NamespaceContext::findNestedDecl(
// Handle case where current namespace is an IGeneralNamespace.
// If there is no match, we fall through.
if (const auto *ns = subNamespace->to<IR::IGeneralNamespace>()) {
const auto *decls = ns->getDeclsByName(name)->toVector();
if (!decls->empty()) {
// TODO: Figure out what to do with multiple results. Maybe return all of them and
// let the caller sort it out?
BUG_CHECK(decls->size() == 1, "Handling of overloaded names not implemented");
return decls->at(0);
}
auto *decl = ns->getDeclsByName(name)->singleOrDefault();
if (decl != nullptr) return decl;
}

// As last resort, fall through to a NestedNamespace check.
Expand Down Expand Up @@ -82,13 +77,8 @@ const IR::IDeclaration *NamespaceContext::findDecl(const IR::Path *path) const {
// Handle case where current namespace is an IGeneralNamespace.
// If there is no match, we fall through.
if (const auto *ns = curNamespace->to<IR::IGeneralNamespace>()) {
const auto *decls = ns->getDeclsByName(name)->toVector();
if (!decls->empty()) {
// TODO: Figure out what to do with multiple results. Maybe return all of them and let
// the caller sort it out?
BUG_CHECK(decls->size() == 1, "Handling of overloaded names not implemented");
return decls->at(0);
}
auto *decl = ns->getDeclsByName(name)->singleOrDefault();
if (decl != nullptr) return decl;
}
// As last resort, check if the NestedNamespace contains the declaration.
if (const auto *ns = curNamespace->to<IR::INestedNamespace>()) {
Expand Down
9 changes: 2 additions & 7 deletions backends/p4tools/common/lib/util.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -145,13 +145,8 @@ std::vector<const IR::Type_Declaration *> argumentsToTypeDeclarations(

const IR::IDeclaration *findProgramDecl(const IR::IGeneralNamespace *ns, const IR::Path *path) {
auto name = path->name.name;
const auto *decls = ns->getDeclsByName(name)->toVector();
if (!decls->empty()) {
// TODO: Figure out what to do with multiple results. Maybe return all of them and
// let the caller sort it out?
BUG_CHECK(decls->size() == 1, "Handling of overloaded names not implemented");
return decls->at(0);
}
auto *decl = ns->getDeclsByName(name)->singleOrDefault();
if (decl != nullptr) return decl;
BUG("Variable %1% not found in the available namespaces.", path);
}

Expand Down
44 changes: 21 additions & 23 deletions backends/p4tools/modules/testgen/targets/bmv2/test/small-step/reachability.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -38,15 +38,6 @@ using P4ReachabilityContext = P4CContextWithOptions<P4ReachabilityOptions>;

class P4CReachability : public ::testing::Test {};

template <class T>
std::vector<const IR::IDeclaration *> *getNodeByType(const IR::P4Program *program) {
std::function<bool(const IR::IDeclaration *)> filter = [](const IR::IDeclaration *d) {
CHECK_NULL(d);
return d->is<T>();
};
return program->getDeclarations()->where(filter)->toVector();
}

/// Loads example from a file
using ReturnedInfo = std::tuple<const IR::P4Program *, const P4Tools::NodesCallGraph *,
const P4Tools::ReachabilityHashType>;
Expand Down Expand Up @@ -83,14 +74,24 @@ ReturnedInfo loadExampleForReachability(const char *curFile) {
}

template <class T>
const IR::Node *getSpecificNode(std::vector<const IR::IDeclaration *> *v, cstring name) {
CHECK_NULL(v);
for (const auto *i : *v) {
auto element = i->to<T>();
if (element->name.name == name) {
return element;
}
}
auto getNodeByType(const IR::P4Program *program) {
auto filter = [](const IR::IDeclaration *d) {
CHECK_NULL(d);
return d->is<T>();
};
return program->getDeclarations()->where(filter);
}

template <class Node>
const Node *getSpecificNode(const IR::P4Program *program, cstring name) {
auto filter = [name](const IR::IDeclaration *d) {
CHECK_NULL(d);
if (const auto *element = d->to<Node>()) return element->name.name == name;

return false;
};
if (auto *node = program->getDeclarations()->where(filter)->singleOrDefault())
return node->template to<Node>();
return nullptr;
}

Expand All @@ -109,16 +110,13 @@ TEST_F(P4CReachability, testParserStatesAndAnnotations) {
const auto *program = std::get<0>(result);
ASSERT_TRUE(program);
const auto *dcg = std::get<1>(result);
auto *parserVector = getNodeByType<IR::P4Parser>(program);
ASSERT_TRUE(parserVector);
const auto *parser = getSpecificNode<IR::P4Parser>(parserVector, "ParserI");
const auto *parser = getSpecificNode<IR::P4Parser>(program, "ParserI");
ASSERT_TRUE(parser);
// Parser ParserI is reachable.
ASSERT_TRUE(dcg->isReachable(program, parser));
auto *controlsVector = getNodeByType<IR::P4Control>(program);
const auto *ingress = getSpecificNode<IR::P4Control>(controlsVector, "IngressI");
const auto *ingress = getSpecificNode<IR::P4Control>(program, "IngressI");
ASSERT_TRUE(ingress);
const auto *engress = getSpecificNode<IR::P4Control>(controlsVector, "EgressI");
const auto *engress = getSpecificNode<IR::P4Control>(program, "EgressI");
ASSERT_TRUE(engress);
// IngressI is reachable.
ASSERT_TRUE(dcg->isReachable(program, ingress));
Expand Down
3 changes: 1 addition & 2 deletions backends/p4tools/modules/testgen/targets/bmv2/test/transformations/saturation_arithm.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -88,8 +88,7 @@ class Z3SolverSatTests : public ::testing::Test {
}

// Extract the binary operation from the P4Program
auto *const declVector = test->getProgram().getDeclsByName("mau")->toVector();
const auto *decl = (*declVector)[0];
const auto *decl = test->getProgram().getDeclsByName("mau")->single();
const auto *control = decl->to<IR::P4Control>();
for (const auto *st : control->body->components) {
if (const auto *as = st->to<IR::IfStatement>()) {
Expand Down
3 changes: 1 addition & 2 deletions backends/p4tools/modules/testgen/targets/bmv2/test/z3-solver/asrt_model.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -93,8 +93,7 @@ class Z3SolverTest : public P4ToolsTest {
}

// Extract the binary operation from the P4Program
auto *const declVector = test->getProgram().getDeclsByName("mau")->toVector();
const auto *decl = (*declVector)[0];
const auto *decl = test->getProgram().getDeclsByName("mau")->single();
const auto *control = decl->to<IR::P4Control>();
SymbolicConverter converter;
for (const auto *st : control->body->components) {
Expand Down
3 changes: 1 addition & 2 deletions backends/p4tools/modules/testgen/targets/bmv2/test/z3-solver/expressions.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -87,8 +87,7 @@ class Z3SolverTests : public ::testing::Test {
}

// Extract the binary operation from the P4Program
auto *const declVector = test->getProgram().getDeclsByName("mau")->toVector();
const auto *decl = (*declVector)[0];
const auto *decl = test->getProgram().getDeclsByName("mau")->single();
const auto *control = decl->to<IR::P4Control>();
for (const auto *st : control->body->components) {
if (const auto *as = st->to<IR::IfStatement>()) {
Expand Down
7 changes: 2 additions & 5 deletions backends/p4tools/modules/testgen/test/small-step/util.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -55,14 +55,11 @@ std::optional<const P4ToolsTestCase> createSmallStepExprTest(const std::string &
template <class T>
const T *extractExpr(const IR::P4Program &program) {
// Get the mau declarations in the P4Program.
const auto *decls = program.getDeclsByName("mau")->toVector();
if (decls->size() != 1) {
return nullptr;
}
auto *decl = program.getDeclsByName("mau")->single();

// Convert the mau declaration to a control and ensure that
// there is a single statement in the body.
const auto *control = (*decls)[0]->to<IR::P4Control>();
const auto *control = decl->checkedTo<IR::P4Control>();
if (control->body->components.size() != 1) {
return nullptr;
}
Expand Down
Loading

0 comments on commit e14d108

Please sign in to comment.