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

Move the assembler to P4C. #70

Open
wants to merge 3 commits into
base: main
Choose a base branch
from
Open

Move the assembler to P4C. #70

wants to merge 3 commits into from

Conversation

fruffy
Copy link
Contributor

@fruffy fruffy commented Feb 7, 2025

Fixes #54.

Signed-off-by: fruffy <[email protected]>
@fruffy fruffy force-pushed the fruffy/assembler branch from 277456d to f9a1a39 Compare March 17, 2025 20:04
@fruffy fruffy marked this pull request as ready for review March 18, 2025 16:32
@jafingerhut
Copy link
Contributor

If I do a diff on these directories in the latest versions of p4c and open-p4studio repos (before this PR's changes), I see this:

$ diff -cr open-p4studio/pkgsrc/p4-compilers/bf-asm p4c/backends/tofino/bf-asm | grep '^Only in '
Only in open-p4studio/pkgsrc/p4-compilers/bf-asm/cmake: Abseil.cmake
Only in p4c/backends/tofino/bf-asm/cmake: config.h.cmake
Only in open-p4studio/pkgsrc/p4-compilers/bf-asm/cmake: config.h.in
Only in open-p4studio/pkgsrc/p4-compilers/bf-asm: CPPLINT.cfg
Only in open-p4studio/pkgsrc/p4-compilers/bf-asm: test

is it intentional not to copy over the pkgsrc/p4-compilers/bf-asm/test directory, and all of the many tests it contains? I understand they are still in the git commit history here, but would it perhaps be better if they were also added to the p4c repo, for possible future use?

@fruffy
Copy link
Contributor Author

fruffy commented Mar 18, 2025

is it intentional not to copy over the pkgsrc/p4-compilers/bf-asm/test directory, and all of the many tests it contains?

Yes, the context is this comment: p4lang/p4c#5121 (comment)

@jafingerhut
Copy link
Contributor

is it intentional not to copy over the pkgsrc/p4-compilers/bf-asm/test directory, and all of the many tests it contains?

Yes, the context is this comment: p4lang/p4c#5121 (comment)

Makes sense. I did see that earlier, but thank for the reminder.

Copy link
Contributor

@jafingerhut jafingerhut left a comment

Choose a reason for hiding this comment

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

LGTM

@fruffy
Copy link
Contributor Author

fruffy commented Mar 18, 2025

Does it build on your local setup? I remember there were some problems in #65.

@jafingerhut
Copy link
Contributor

I am away from home and my x86_64 systems until evening of Mar 19, and don't have remote login set up for them. I can try it then.

@jafingerhut
Copy link
Contributor

I created a freshly installed Ubuntu 22.04 desktop Linux VM for x86_64, and ran the commands listed in the file cmds.txt that is part of the attached zip file, but it gave errors during building the code during the p4studio profile appply ... command. The zip file contains the full output of that command, and the contents of the log file created in the logs directory. The cmds.txt file shows exactly which commit SHAs of the open-p4studio and p4c repo were in use.

logs.zip

@fruffy
Copy link
Contributor Author

fruffy commented Mar 20, 2025

Could you try the new refpoint? Nevermind, there are quite a few more issues...back to the drawing board.

@fruffy fruffy force-pushed the fruffy/assembler branch 2 times, most recently from 521e8fd to 83df6fd Compare March 21, 2025 21:56
Signed-off-by: fruffy <[email protected]>
@fruffy fruffy force-pushed the fruffy/assembler branch from 83df6fd to fd370c2 Compare March 24, 2025 05:57
@jafingerhut
Copy link
Contributor

@fruffy Ready for another local test run on this?

@fruffy
Copy link
Contributor Author

fruffy commented Mar 24, 2025 via email

@jafingerhut
Copy link
Contributor

Attached are build logs with errors from my attempt to build this PR's version of open-p4studio on 2024-Mar-24 on a freshly installed Ubuntu 22.04 x86_64 VM running in VirtualBox. I have not attempted to analyze the errors myself.
logs.zip

@jafingerhut
Copy link
Contributor

There is no need to explain if it is complex, but are there any briefly-described known reasons why CI on Ubuntu 22.04 gives clean build and test, but on a freshly installed Ubuntu 22.04 VM I get build errors? I am guessing there must be something different about the software installed on my VM vs. the CI version, but don't have good guesses what those are.

@fruffy
Copy link
Contributor Author

fruffy commented Mar 24, 2025

The error you are encountering is likely related to the preprocessor (and GNU extension support?). I am not sure what could be different. If I disable this line: https://github.com/p4lang/p4c/blob/main/backends/tofino/bf-asm/CMakeLists.txt#L249

I can reproduce it.. but not sure it doesn't work for you.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

P4 Compiler must invoke the assembler by default
2 participants