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

Clean up the P4-14 dependent code. #4925

Merged
merged 1 commit into from
Sep 28, 2024
Merged

Clean up the P4-14 dependent code. #4925

merged 1 commit into from
Sep 28, 2024

Conversation

fruffy
Copy link
Collaborator

@fruffy fruffy commented Sep 27, 2024

First attempt to make P4-14 specific code optional in the compiler. This can make it a little easier to reason about IR changes and might reduce compile times, too.

The goal is to remove all "v1model"-specific code from the compiler core and move it into the appropriate folders. What makes this change a little tedious is that https://github.com/p4lang/p4c/blob/main/frontends/p4/fromv1.0/v1model.h is a bunch of constant strings that are referenced by the control plane and the toP4 pass.

In subsequent PRs, I will try to find a replacement for this file.

@fruffy fruffy added the core Topics concerning the core segments of the compiler (frontend, midend, parser) label Sep 27, 2024
@fruffy fruffy force-pushed the fruffy/p4_14_refactor branch 2 times, most recently from c027db9 to da3cef8 Compare September 27, 2024 01:01
@fruffy fruffy marked this pull request as ready for review September 27, 2024 12:23
@fruffy fruffy requested review from vlstill, grg and ChrisDodd and removed request for vlstill September 27, 2024 12:23
@@ -165,8 +165,10 @@ bool ToP4::preorder(const IR::P4Program *program) {
if (sourceFile.startsWith(p4includePath)) {
const char *p = sourceFile.c_str() + strlen(p4includePath);
if (*p == '/') p++;
// TODO: This is v1model-specific code. This should be not part of the core
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

With #4899 we can likely get rid of this code and move it into the bmv2 back end.

@grg
Copy link
Contributor

grg commented Sep 27, 2024

We also have a P4-16 implementation of the v1model (p4include/v1model.p4). I'm not sure how many of the declarations you've moved are used by v1model.p4, but is the expectation that these won't compile if the P4-14 code is excluded?

@fruffy
Copy link
Collaborator Author

fruffy commented Sep 27, 2024

We also have a P4-16 implementation of the v1model (p4include/v1model.p4). I'm not sure how many of the declarations you've moved are used by v1model.p4, but is the expectation that these won't compile if the P4-14 code is excluded?

The code doesn't seem to be used much in the P4-16 parts except for the constants in v1model.h but I will know whether better there are IR references once I actually disable it. If that is the case, these IR nodes should be part of the bmv2 def files. Otherwise, the changes here are fairly safe.

But I do not know how Tofino depends on the code here.

@fruffy fruffy added this pull request to the merge queue Sep 27, 2024
Merged via the queue into main with commit abf696e Sep 28, 2024
19 checks passed
@fruffy fruffy deleted the fruffy/p4_14_refactor branch September 28, 2024 00:04
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
core Topics concerning the core segments of the compiler (frontend, midend, parser)
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants