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

Use abseil maps even more #4473

Merged
merged 11 commits into from
Mar 1, 2024
2 changes: 2 additions & 0 deletions BUILD.bazel
Original file line number Diff line number Diff line change
Expand Up @@ -239,6 +239,8 @@ cc_library(
"@com_github_p4lang_p4runtime//:p4runtime_cc_proto",
"@com_github_p4lang_p4runtime//:p4types_cc_proto",
"@com_google_absl//absl/container:flat_hash_map",
"@com_google_absl//absl/container:flat_hash_set",
"@com_google_absl//absl/container:inlined_vector",
"@com_google_protobuf//:protobuf",
],
)
Expand Down
9 changes: 6 additions & 3 deletions CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -203,15 +203,17 @@ endif()
# we require -pthread to make std::call_once work, even if we're not using threads...
set(THREADS_PREFER_PTHREAD_FLAG ON)
find_package(Threads REQUIRED)
set (P4C_LIB_DEPS "${P4C_LIB_DEPS};${CMAKE_THREAD_LIBS_INIT}")
list (APPEND P4C_LIB_DEPS ${CMAKE_THREAD_LIBS_INIT})
include_directories(SYSTEM ${Boost_INCLUDE_DIRS})
include_directories(SYSTEM ${PROTOBUF_INCLUDE_DIRS})
include_directories(SYSTEM ${LIBGC_INCLUDE_DIR})
set (HAVE_LIBBOOST_IOSTREAMS 1)
set (P4C_LIB_DEPS "${P4C_LIB_DEPS};${Boost_LIBRARIES}")
list (APPEND P4C_LIB_DEPS ${Boost_LIBRARIES})
if (ENABLE_GC)
set (P4C_LIB_DEPS "${P4C_LIB_DEPS};${LIBGC_LIBRARIES}")
list (APPEND P4C_LIB_DEPS ${LIBGC_LIBRARIES})
endif ()
set (P4C_ABSL_LIBRARIES absl::flat_hash_map absl::flat_hash_set)
list (APPEND P4C_LIB_DEPS ${P4C_ABSL_LIBRARIES})

# other required libraries
p4c_add_library (rt clock_gettime HAVE_CLOCK_GETTIME)
Expand Down Expand Up @@ -507,6 +509,7 @@ add_custom_target(genIR DEPENDS ${IR_GENERATED_SRCS})
set_source_files_properties(${IR_GENERATOR} PROPERTIES GENERATED TRUE)
add_library(ir-generated OBJECT ${IR_GENERATED_SRCS} ${EXTENSION_IR_SOURCES})
add_dependencies(ir-generated ir genIR)
target_link_libraries(ir-generated ${P4C_LIB_DEPS})

######################################## IR Generation End ########################################

Expand Down
2 changes: 1 addition & 1 deletion backends/bmv2/CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -83,7 +83,7 @@ set (BMV2_BACKEND_COMMON_HDRS
set (IR_DEF_FILES ${IR_DEF_FILES} ${CMAKE_CURRENT_SOURCE_DIR}/bmv2.def PARENT_SCOPE)

add_library(bmv2backend STATIC ${BMV2_BACKEND_COMMON_SRCS})
add_dependencies(bmv2backend ir-generated frontend)
target_link_libraries(bmv2backend ir-generated frontend)

add_executable(p4c-bm2-ss ${BMV2_SIMPLE_SWITCH_SRCS})
target_link_libraries (p4c-bm2-ss bmv2backend ${P4C_LIBRARIES} ${P4C_LIB_DEPS})
Expand Down
3 changes: 1 addition & 2 deletions backends/dpdk/CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -99,8 +99,7 @@ endforeach()
set(EXTENSION_IR_SOURCES ${EXTENSION_IR_SOURCES} ${QUAL_DPDK_IR_SRCS} PARENT_SCOPE)

add_executable(p4c-dpdk ${P4C_DPDK_SOURCES})
target_link_libraries (p4c-dpdk dpdk_runtime ${P4C_LIBRARIES} ${P4C_LIB_DEPS})
add_dependencies(p4c-dpdk dpdk_runtime frontend)
target_link_libraries (p4c-dpdk dpdk_runtime frontend ${P4C_LIBRARIES} ${P4C_LIB_DEPS})

install (TARGETS p4c-dpdk
RUNTIME DESTINATION ${P4C_RUNTIME_OUTPUT_DIRECTORY})
Expand Down
5 changes: 2 additions & 3 deletions backends/graphs/CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -30,11 +30,10 @@ set (GRAPHS_HDRS
)

add_library(p4cgraphs STATIC ${GRAPHS_SRCS} ${EXTENSION_P4_14_CONV_SOURCES})
add_dependencies(p4cgraphs ir-generated frontend)
target_link_libraries(p4cgraphs ir-generated frontend)

add_executable(p4c-graphs p4c-graphs.cpp)
target_link_libraries (p4c-graphs p4cgraphs ${P4C_LIBRARIES} ${P4C_LIB_DEPS})
add_dependencies(p4c-graphs ir-generated frontend)
target_link_libraries (p4c-graphs p4cgraphs ir-generated frontend ${P4C_LIBRARIES} ${P4C_LIB_DEPS})

install (TARGETS p4c-graphs
RUNTIME DESTINATION ${P4C_RUNTIME_OUTPUT_DIRECTORY})
Expand Down
5 changes: 1 addition & 4 deletions backends/p4tools/common/CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,7 @@ add_p4tools_library(p4tools-common ${P4C_TOOLS_COMMON_SOURCES})
target_link_libraries(
p4tools-common
PUBLIC ${P4TOOLS_Z3_LIB}
PRIVATE frontend
)

target_include_directories(
Expand All @@ -54,9 +55,5 @@ target_include_directories(
PUBLIC "${P4C_BINARY_DIR}"
)

add_dependencies(p4tools-common ir-generated frontend)

# Add control-plane-specific extensions.
add_subdirectory(${CMAKE_CURRENT_LIST_DIR}/control_plane)


4 changes: 3 additions & 1 deletion frontends/CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -259,4 +259,6 @@ if (FLEX_INCLUDE_DIRS)
endif ()
add_library (frontend STATIC ${FRONTEND_SOURCES})
add_dependencies (frontend frontend-parser-gen)
target_link_libraries (frontend frontend-parser-gen)
target_link_libraries (frontend frontend-parser-gen
absl::flat_hash_set
absl::flat_hash_map)
14 changes: 7 additions & 7 deletions frontends/common/resolveReferences/referenceMap.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -35,15 +35,15 @@ void ReferenceMap::clear() {
program = nullptr;
used.clear();
thisToDeclaration.clear();
for (auto &reserved : P4::reservedWords) usedNames.insert({reserved, 0});
for (auto &reserved : P4::reservedWords) usedNames.emplace(reserved, 0);
ProgramMap::clear();
}

void ReferenceMap::setDeclaration(const IR::Path *path, const IR::IDeclaration *decl) {
CHECK_NULL(path);
CHECK_NULL(decl);
LOG3("Resolved " << dbp(path) << " to " << dbp(decl));
auto previous = get(pathToDeclaration, path);
const auto *previous = get(pathToDeclaration, path);
if (previous != nullptr && previous != decl)
BUG("%1% already resolved to %2% instead of %3%", dbp(path), dbp(previous),
dbp(decl->getNode()));
Expand All @@ -56,15 +56,15 @@ void ReferenceMap::setDeclaration(const IR::This *pointer, const IR::IDeclaratio
CHECK_NULL(pointer);
CHECK_NULL(decl);
LOG3("Resolved " << dbp(pointer) << " to " << dbp(decl));
auto previous = get(thisToDeclaration, pointer);
const auto *previous = get(thisToDeclaration, pointer);
if (previous != nullptr && previous != decl)
BUG("%1% already resolved to %2% instead of %3%", dbp(pointer), dbp(previous), dbp(decl));
thisToDeclaration.emplace(pointer, decl);
}

const IR::IDeclaration *ReferenceMap::getDeclaration(const IR::This *pointer, bool notNull) const {
CHECK_NULL(pointer);
auto result = get(thisToDeclaration, pointer);
const auto *result = get(thisToDeclaration, pointer);

if (result)
LOG3("Looking up " << dbp(pointer) << " found " << dbp(result));
Expand All @@ -77,7 +77,7 @@ const IR::IDeclaration *ReferenceMap::getDeclaration(const IR::This *pointer, bo

const IR::IDeclaration *ReferenceMap::getDeclaration(const IR::Path *path, bool notNull) const {
CHECK_NULL(path);
auto result = get(pathToDeclaration, path);
const auto *result = get(pathToDeclaration, path);

if (result)
LOG3("Looking up " << dbp(path) << " found " << dbp(result));
Expand Down Expand Up @@ -108,7 +108,7 @@ cstring ReferenceMap::newName(cstring base) {

cstring name = base;
if (usedNames.count(name)) name = cstring::make_unique(usedNames, name, usedNames[base], '_');
usedNames.insert({name, 0});
usedNames.emplace(name, 0);
return name;
}

Expand All @@ -127,7 +127,7 @@ cstring MinimalNameGenerator::newName(cstring base) {

cstring name = base;
if (usedNames.count(name)) name = cstring::make_unique(usedNames, name, usedNames[base], '_');
usedNames.insert({name, 0});
usedNames.emplace(name, 0);
return name;
}

Expand Down
15 changes: 8 additions & 7 deletions frontends/common/resolveReferences/referenceMap.h
Original file line number Diff line number Diff line change
Expand Up @@ -17,11 +17,12 @@ limitations under the License.
#ifndef COMMON_RESOLVEREFERENCES_REFERENCEMAP_H_
#define COMMON_RESOLVEREFERENCES_REFERENCEMAP_H_

#include "absl/container/flat_hash_map.h"
#include "absl/container/flat_hash_set.h"
#include "frontends/common/programMap.h"
#include "ir/ir.h"
#include "ir/visitor.h"
#include "lib/cstring.h"
#include "lib/map.h"

namespace P4 {

Expand All @@ -34,7 +35,7 @@ class NameGenerator {
class MinimalNameGenerator : public NameGenerator, public Inspector {
/// All names used in the program. Key is a name, value represents how many times
/// this name was used as a base for newly generated unique names.
std::unordered_map<cstring, int> usedNames;
absl::flat_hash_map<cstring, int, Util::Hash> usedNames;
void postorder(const IR::Path *p) override { usedName(p->name.name); }
void postorder(const IR::Type_Declaration *t) override { usedName(t->name.name); }
void postorder(const IR::Declaration *d) override { usedName(d->name.name); }
Expand Down Expand Up @@ -67,17 +68,17 @@ class ReferenceMap final : public ProgramMap, public NameGenerator, public Decla
bool isv1;

/// Maps paths in the program to declarations.
ordered_map<const IR::Path *, const IR::IDeclaration *> pathToDeclaration;
absl::flat_hash_map<const IR::Path *, const IR::IDeclaration *, Util::Hash> pathToDeclaration;
vlstill marked this conversation as resolved.
Show resolved Hide resolved

/// Set containing all declarations in the program.
std::set<const IR::IDeclaration *> used;
absl::flat_hash_set<const IR::IDeclaration *, Util::Hash> used;

/// Map from `This` to declarations (an experimental feature).
std::map<const IR::This *, const IR::IDeclaration *> thisToDeclaration;
absl::flat_hash_map<const IR::This *, const IR::IDeclaration *, Util::Hash> thisToDeclaration;

/// All names used in the program. Key is a name, value represents how many times
/// this name was used as a base for newly generated unique names.
std::unordered_map<cstring, int> usedNames;
absl::flat_hash_map<cstring, int, Util::Hash> usedNames;

public:
ReferenceMap();
Expand Down Expand Up @@ -116,7 +117,7 @@ class ReferenceMap final : public ProgramMap, public NameGenerator, public Decla
bool isUsed(const IR::IDeclaration *decl) const { return used.count(decl) > 0; }

/// Indicate that @p name is used in the program.
void usedName(cstring name) { usedNames.insert({name, 0}); }
void usedName(cstring name) { usedNames.emplace(name, 0); }
};

} // namespace P4
Expand Down
10 changes: 6 additions & 4 deletions frontends/common/resolveReferences/resolveReferences.h
Original file line number Diff line number Diff line change
Expand Up @@ -17,9 +17,9 @@ limitations under the License.
#ifndef COMMON_RESOLVEREFERENCES_RESOLVEREFERENCES_H_
#define COMMON_RESOLVEREFERENCES_RESOLVEREFERENCES_H_

#include "absl/container/flat_hash_map.h"
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actually, can we make absl a bracketed system include to be consistent?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think someone asked somewhere exactly the opposite for fetched dependencies :)

Copy link
Collaborator

@fruffy fruffy Feb 29, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Really? I believe all our fetched dependencies (protobuf, gtest, inja, z3) are included using bracketed includes. Can you point me to the discussion?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Need to find. But I'm happy to change to angled brackets. This makes much more sense to me :)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ok, apparently abseil expects to use quoted includes. And bazel build even enforces this (the path is exported as -iquoted).

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ugh, seems to be an opinionated maintainer. This might cause issues with Werror and clang-tidy. Not a big deal but annoying.

We can just patch this, I can get this done in a separate PR.

#include "ir/ir.h"
#include "lib/cstring.h"
#include "lib/exceptions.h"
#include "lib/iterator_range.h"
#include "referenceMap.h"

Expand All @@ -41,10 +41,12 @@ class ResolutionContext : virtual public Visitor, public DeclarationLookup {
std::unordered_multimap<cstring, const IR::IDeclaration *> &memoizeDeclsByName(
const IR::INamespace *ns) const;

mutable std::unordered_map<const IR::INamespace *, std::vector<const IR::IDeclaration *>>
mutable absl::flat_hash_map<const IR::INamespace *, std::vector<const IR::IDeclaration *>,
Util::Hash>
namespaceDecls;
mutable std::unordered_map<const IR::INamespace *,
std::unordered_multimap<cstring, const IR::IDeclaration *>>
mutable absl::flat_hash_map<const IR::INamespace *,
std::unordered_multimap<cstring, const IR::IDeclaration *>,
Util::Hash>
namespaceDeclNames;

protected:
Expand Down
22 changes: 15 additions & 7 deletions frontends/p4/def_use.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -253,14 +253,22 @@ bool LocationSet::overlaps(const LocationSet *other) const {
return false;
}

void ProgramPoints::add(const ProgramPoints *from) {
points.insert(from->points.begin(), from->points.end());
}

const ProgramPoints *ProgramPoints::merge(const ProgramPoints *with) const {
auto result = new ProgramPoints(points);
for (auto p : with->points) result->points.emplace(p);
auto *result = new ProgramPoints(points);
result->points.insert(with->points.begin(), with->points.end());
return result;
}

ProgramPoint::ProgramPoint(const ProgramPoint &context, const IR::Node *node) {
for (auto e : context.stack) stack.push_back(e);
assign(context, node);
}

void ProgramPoint::assign(const ProgramPoint &context, const IR::Node *node) {
stack.assign(context.stack.begin(), context.stack.end());
stack.push_back(node);
}

Expand Down Expand Up @@ -325,10 +333,10 @@ void Definitions::removeLocation(const StorageLocation *location) {
}

const ProgramPoints *Definitions::getPoints(const LocationSet *locations) const {
const ProgramPoints *result = new ProgramPoints();
for (auto sl : *locations->canonicalize()) {
auto points = getPoints(sl->to<BaseLocation>());
result = result->merge(points);
ProgramPoints *result = new ProgramPoints();
for (const auto *sl : *locations->canonicalize()) {
const auto *points = getPoints(sl->to<BaseLocation>());
result->add(points);
}
return result;
}
Expand Down
36 changes: 24 additions & 12 deletions frontends/p4/def_use.h
Original file line number Diff line number Diff line change
Expand Up @@ -17,14 +17,15 @@ limitations under the License.
#ifndef FRONTENDS_P4_DEF_USE_H_
#define FRONTENDS_P4_DEF_USE_H_

#include <typeindex> // IWYU pragma: keep
#include <unordered_set>

#include "frontends/p4/typeChecking/typeChecker.h"
#include "absl/container/flat_hash_set.h"
#include "absl/container/inlined_vector.h"
#include "frontends/common/resolveReferences/referenceMap.h"
#include "ir/ir.h"
#include "lib/alloc_trace.h"
#include "lib/hash.h"
#include "lib/hvec_map.h"
#include "lib/ordered_set.h"
#include "typeMap.h"

namespace P4 {

Expand Down Expand Up @@ -129,7 +130,7 @@ class StructLocation : public WithFieldsLocation {
/// Interface for locations that support an index operation
class IndexedLocation : public StorageLocation {
protected:
std::vector<const StorageLocation *> elements;
absl::InlinedVector<const StorageLocation *, 8> elements;
vlstill marked this conversation as resolved.
Show resolved Hide resolved
friend class StorageFactory;

void createElement(unsigned index, StorageLocation *element) {
Expand All @@ -145,8 +146,8 @@ class IndexedLocation : public StorageLocation {
elements.resize(it->getSize());
}
void addElement(unsigned index, LocationSet *result) const;
std::vector<const StorageLocation *>::const_iterator begin() const { return elements.cbegin(); }
std::vector<const StorageLocation *>::const_iterator end() const { return elements.cend(); }
auto begin() const { return elements.cbegin(); }
auto end() const { return elements.cend(); }

DECLARE_TYPEINFO(IndexedLocation, StorageLocation);
};
Expand Down Expand Up @@ -278,14 +279,14 @@ class ProgramPoint : public IHasDbPrint {
/// the previous context. E.g., a stack [Function] is the context before
/// the function, while [Function, nullptr] is the context after the
/// function terminates.
std::vector<const IR::Node *> stack;
absl::InlinedVector<const IR::Node *, 8> stack; // Has inline space for 8 nodes
vlstill marked this conversation as resolved.
Show resolved Hide resolved

public:
ProgramPoint() = default;
ProgramPoint(const ProgramPoint &other) : stack(other.stack) {}
explicit ProgramPoint(const IR::Node *node) {
CHECK_NULL(node);
stack.push_back(node);
assign(node);
}
ProgramPoint(const ProgramPoint &context, const IR::Node *node);
/// A point logically before the function/control/action start.
Expand Down Expand Up @@ -313,10 +314,13 @@ class ProgramPoint : public IHasDbPrint {
out << "[[" << l << "]]";
}
}
void assign(const ProgramPoint &context, const IR::Node *node);
void assign(const IR::Node *node) { stack.assign({node}); }
void clear() { stack.clear(); }
const IR::Node *last() const { return stack.empty() ? nullptr : stack.back(); }
bool isBeforeStart() const { return stack.empty(); }
std::vector<const IR::Node *>::const_iterator begin() const { return stack.begin(); }
std::vector<const IR::Node *>::const_iterator end() const { return stack.end(); }
auto begin() const { return stack.begin(); }
auto end() const { return stack.end(); }
ProgramPoint &operator=(const ProgramPoint &) = default;
ProgramPoint &operator=(ProgramPoint &&) = default;
};
Expand All @@ -332,16 +336,24 @@ struct hash<P4::ProgramPoint> {
};
} // namespace std

namespace Util {
template <>
struct Hasher<P4::ProgramPoint> {
size_t operator()(const P4::ProgramPoint &p) const { return p.hash(); }
};
} // namespace Util

namespace P4 {
class ProgramPoints : public IHasDbPrint {
typedef std::unordered_set<ProgramPoint> Points;
typedef absl::flat_hash_set<ProgramPoint, Util::Hash> Points;
vlstill marked this conversation as resolved.
Show resolved Hide resolved
Points points;
explicit ProgramPoints(const Points &points) : points(points) {}

public:
ProgramPoints() = default;
explicit ProgramPoints(ProgramPoint point) { points.emplace(point); }
void add(ProgramPoint point) { points.emplace(point); }
void add(const ProgramPoints *from);
const ProgramPoints *merge(const ProgramPoints *with) const;
bool operator==(const ProgramPoints &other) const;
void dbprint(std::ostream &out) const override {
Expand Down
Loading
Loading