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

Strange build dependencies from Tofino backend #5013

Open
asl opened this issue Nov 12, 2024 · 6 comments
Open

Strange build dependencies from Tofino backend #5013

asl opened this issue Nov 12, 2024 · 6 comments
Assignees
Labels
bug This behavior is unintended and should be fixed. tofino Topics related to the Tofino switch and back end.

Comments

@asl
Copy link
Contributor

asl commented Nov 12, 2024

Building e.g. a frontend target actually builds Tofino backend, which does not look correct to me.

@asl asl added bug This behavior is unintended and should be fixed. tofino Topics related to the Tofino switch and back end. labels Nov 12, 2024
@asl asl assigned hanw Nov 12, 2024
@fruffy
Copy link
Collaborator

fruffy commented Nov 12, 2024

If I'd wager a guess it might be because of the Tofino IR sources: https://github.com/p4lang/p4c/blob/main/backends/tofino/CMakeLists.txt#L219 which need to be compile with the ir-generated blob.

@asl
Copy link
Contributor Author

asl commented Nov 12, 2024

https://github.com/p4lang/p4c/blob/main/backends/tofino/CMakeLists.txt#L219 which need to be compile with the ir-generated blob.

But looks like these are not entirely frontend...

@fruffy
Copy link
Collaborator

fruffy commented Nov 12, 2024

But looks like these are not entirely frontend...

Yeah I also think they could be cleaned up. There includes are quite messy and there are some circular dependencies.

@ChrisDodd
Copy link
Contributor

ChrisDodd commented Nov 12, 2024

The way the code is set up, it builds IR Visitor infrastructure as part of the frontend, supporting all IR types that are declared in any backend that is enabled in the build as part of the frontend. That ends up pulling a few backend files (those related to the IR types) into the frontend, and is required due C++ ODR requirements (otherwise you would get undefined behavior by linking code that doesn't agree on the vtable layout for classes like IR::Node).

To not include this backend code, you need to use -DENABLE_TOFINO=OFF to cmake and rebuild everything.

@asl
Copy link
Contributor Author

asl commented Nov 12, 2024

The way the code is set up, it builds IR Visitor infrastructure as part of the frontend, supporting all IR types that are declared in any backend that is enabled in the build as part of the frontend. That ends up pulling a few backend files (those related to the IR types) into the frontend, and is required due C++ ODR requirements (otherwise you would get undefined behavior by linking code that doesn't agree on the vtable layout for classes like IR::Node).

To not include this backend code, you need to use -DENABLE_TOFINO=OFF to cmake and rebuild everything.

But are files like bf-p4c/midend/path_linearizer.cpp or bf-utils/src/dynamic_hash/dynamic_hash.c bf-utils/src/dynamic_hash/bfn_hash_algorithm.c constitute IR Visitor infrastructure?

@ChrisDodd
Copy link
Contributor

But are files like bf-p4c/midend/path_linearizer.cpp or bf-utils/src/dynamic_hash/dynamic_hash.c bf-utils/src/dynamic_hash/bfn_hash_algorithm.c constitute IR Visitor infrastructure?

No, but disentangling them is likely to require splitting some source files to only include the minimum to get things to build while not including other things that pull in more dependencies.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug This behavior is unintended and should be fixed. tofino Topics related to the Tofino switch and back end.
Projects
None yet
Development

No branches or pull requests

4 participants