Move the assembler to P4C.

[fruffy]

Fixes #54.

[jafingerhut]

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]

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]

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.

[jafingerhut]

LGTM

[fruffy]

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

[jafingerhut]

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]

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]

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

[jafingerhut]

@fruffy Ready for another local test run on this?

[fruffy]

Yes, let's give it a try.

…

On March 24, 2025 1:48:38 PM GMT+01:00, Andy Fingerhut ***@***.***> wrote: jafingerhut left a comment (p4lang/open-p4studio#70) @fruffy Ready for another local test run on this? -- Reply to this email directly or view it on GitHub: #70 (comment) You are receiving this because you were mentioned. Message ID: ***@***.***>

[jafingerhut]

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]

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]

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.

[fruffy]

@jafingerhut Can you try this version? It disables the gnu extensions.

[jafingerhut]

@jafingerhut Can you try this version? It disables the gnu extensions.

Running now, and I will post results when I have them.

As a partial answer to things that differ in Ubuntu 22.04 Github Actions CI system, vs. my personal Ubuntu 22.04 VM, I added some commands to CI build that show all packages installed on the system just before the build starts, as shown in this test PR: #93

I do not know if this is the root cause of the difference, but I did notice that on Github Actions CI system, all of these Ubuntu 22.04 packages were installed, before the p4studio command began:

g++ 4:11.2.0-1ubuntu1
g++-10 10.5.0-1ubuntu1~22.04
g++-11 11.4.0-1ubuntu1~22.04
g++-12 12.3.0-1ubuntu1~22.04
g++-9 9.5.0-1ubuntu1~22.04
gcc 4:11.2.0-1ubuntu1
gcc-10 10.5.0-1ubuntu1~22.04
gcc-10-base 10.5.0-1ubuntu1~22.04
gcc-11 11.4.0-1ubuntu1~22.04
gcc-11-base 11.4.0-1ubuntu1~22.04
gcc-12 12.3.0-1ubuntu1~22.04
gcc-12-base 12.3.0-1ubuntu1~22.04
gcc-9 9.5.0-1ubuntu1~22.04
gcc-9-base 9.5.0-1ubuntu1~22.04

I do not know which of those versions that cmake chooses to use.

On my personal Ubuntu 22.04 x86_64 VM, which is a minimal install from the Ubuntu 22.04 ISO for x86_64, plus the Gnome Desktop, I see these packages:

gcc-11-base 11.4.0-1ubuntu1~22.04
gcc-12-base 12.3.0-1ubuntu1~22.04

After p4studio installs packages on my personal VM, I see these packages:

g++ 4:11.2.0-1ubuntu1
g++-11 11.4.0-1ubuntu1~22.04
gcc 4:11.2.0-1ubuntu1
gcc-11 11.4.0-1ubuntu1~22.04
gcc-11-base 11.4.0-1ubuntu1~22.04
gcc-12-base 12.3.0-1ubuntu1~22.04

Again, I do not know which version cmake chooses to use here, but obviously it has far fewer versions to choose from than it does on the Github Actions CI system.

[jafingerhut]

The build succeeded on my personal Ubuntu 22.04 VM, and tna_counter and several other test cases have passed. I am currently running a longer list of tests just to see the results, but I think it is fine to merge without waiting on that, given that the first 10 or so have all passed fine.

[jafingerhut]

105 out of 109 test cases that I ran passed. I do not have records of how many of them pass without the changes in this PR -- it could very well be that the 4 that failed started failing many changes ago, or were sometimes-pass sometimes-fail when I first saw them pass a month or so ago.

Regardless of the answer, it seems worth merging these changes.

[fruffy]

105 out of 109 test cases that I ran passed. I do not have records of how many of them pass without the changes in this PR -- it could very well be that the 4 that failed started failing many changes ago, or were sometimes-pass sometimes-fail when I first saw them pass a month or so ago.

Can we add these tests to CI? Would give us a bit more confidence in making changes...

[jafingerhut]

105 out of 109 test cases that I ran passed. I do not have records of how many of them pass without the changes in this PR -- it could very well be that the 4 that failed started failing many changes ago, or were sometimes-pass sometimes-fail when I first saw them pass a month or so ago.

Can we add these tests to CI? Would give us a bit more confidence in making changes...

It may need updates, but that is what this PR is intended to do: #39

[fruffy]

Finally merging this one..

[jafingerhut]

I am not sure, but I believe the reason that the --chip-type values allowed by the tofino-model program using the current code in this PR are 1 and 4 are because in this function: https://github.com/p4lang/bf-model/blob/main/runner/main.cpp#L292-L307

the preprocessor symbols MODEL_TOFINO (chip-type 1) and MODEL_JBAY (chip-type 4) have been defined during compilation.

The run_tofino_model.sh script is expecting the tofino-model process to have been compiled with the preprocessor symbols MODEL_TOFINOB0 (chip-type 2) and MODEL_JBAYB0 (chip-type 5) defined.

[fruffy]

I am not sure, but I believe the reason that the --chip-type values allowed by the tofino-model program using the current code in this PR are 1 and 4 are because in this function: https://github.com/p4lang/bf-model/blob/main/runner/main.cpp#L292-L307

the preprocessor symbols MODEL_TOFINO (chip-type 1) and MODEL_JBAY (chip-type 4) have been defined during compilation.

The run_tofino_model.sh script is expecting the tofino-model process to have been compiled with the preprocessor symbols MODEL_TOFINOB0 (chip-type 2) and MODEL_JBAYB0 (chip-type 5) defined.

@ChrisDodd Do you know the exact difference between these models? Are these actual chip variants or just slight configuration differences?

[ChrisDodd]

The difference between the A0 and B0 chips are a few bugs -- the config regs are all the same. The tofino/jbay compiler produces code for the B0 version by default, and I think most of the A0 workarounds were removed (very few A0 chips were ever shipped to customers), but it looks like there's an option to enable one of the jbayA0 workarounds still in the compiler (--tof2lab44). Search for the string A0 in the compiler source code will show a few remnants.

However, I would expect that anyone wanting to run on real hardware would have a B0 chip, andanyone wanting to test with the model would want it configured to use the B0 versions, not the A0.

[fruffy]

The difference between the A0 and B0 chips are a few bugs -- the config regs are all the same. The tofino/jbay compiler produces code for the B0 version by default, and I think most of the A0 workarounds were removed (very few A0 chips were ever shipped to customers), but it looks like there's an option to enable one of the jbayA0 workarounds still in the compiler (--tof2lab44). Search for the string A0 in the compiler source code will show a few remnants.

However, I would expect that anyone wanting to run on real hardware would have a B0 chip, andanyone wanting to test with the model would want it configured to use the B0 versions, not the A0.

Makes sense, let's go with B0 as the default then.