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

Crude port of Tofino dynamic hash to C++ #5043

Merged
merged 1 commit into from
Dec 3, 2024
Merged

Conversation

asl
Copy link
Contributor

@asl asl commented Dec 2, 2024

No description provided.

@asl asl requested review from hanw, ChrisDodd and fruffy December 2, 2024 16:04
@asl
Copy link
Contributor Author

asl commented Dec 2, 2024

Spin-off from #5033

@asl asl added the tofino Topics related to the Tofino switch and back end. label Dec 2, 2024
Copy link
Collaborator

@fruffy fruffy left a comment

Choose a reason for hiding this comment

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

Thanks! We can also remove that unnecessary split into include/src since we do not do that in the rest of the code. I can also take care of that later.

@@ -970,7 +970,7 @@ target_include_directories(
# FIXME: This should be pulled with FetchContent.
PRIVATE "${BFN_P4C_SOURCE_DIR}/third_party/spdlog/include"
)
add_dependencies(tofinobackend ir-generated frontend genLogging bfn_p4runtime)
add_dependencies(tofinobackend frontend genLogging bfn_p4runtime)
Copy link
Collaborator

Choose a reason for hiding this comment

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

Assume this is remove because of the frontend dependency?

Copy link
Contributor Author

@asl asl Dec 2, 2024

Choose a reason for hiding this comment

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

Yes. Actually, the majority add_dependencies are not needed at all. This seems to be an old hack used to "close" some absence of proper dependencies. This needs to be eventually removed from everywhere and replaced with proper deps (e.g. target_link_libraries, etc.)

@asl asl added this pull request to the merge queue Dec 3, 2024
Merged via the queue into p4lang:main with commit ad9a2cc Dec 3, 2024
20 checks passed
@asl asl deleted the tofino-dyn-hash branch December 3, 2024 05:22
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
tofino Topics related to the Tofino switch and back end.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants