Add the Barefoot Tofino Assembler to the Tofino back end.

[fruffy]

The Tofino compiler is not really useful without the assembler. Hence we add it directly. Now the assembler is always active when we use bf-p4c.

The problem is that the assembler code is a little rough and does not pass many of the linting checks P4C has. This required a couple fixes and workarounds. I commented the remaining TODOs in the CMake files. Ideally, we gradually fix all the warnings that are thrown over time.

[jafingerhut]

@vlstill Feel free to give an actual detailed review (unlike this one, which is based on CI tests passing, the fact that these changes should affect ONLY the Tofino back end, and wanting to get the other parts of this working in the open-p4studio repo so we can leave it in a mostly-working state for others). However, as far as I can see right now, Fabian is the only one with the knowledge and interest to make these changes, and I'm the only other one interested in seeing that work move forward.

I have probably over-shared my qualifications on this approval :-)

[ChrisDodd]

Well, I guess as I originally wrote much of this code, I'm qualified to comment on it, if not review it.

The stuff in bf-asm/test is an old broken (non-functional) test suite that is based on running the even older python-based P4_14 compiler, and should probably just be deleted (nothing in it is used, and I don't think any of it is useful)

It looks like it does build fine (with just -DENABLE_TOFINO), but there aren't any tofino test cases? It seems like it should be possible to run some of the code in testdata through p4c --target tofino and then the assembler, but there's nothing set up to try to do that. When run manually, it looks ok, though. Currently ctest does run gtestasm, which runs some assembler internal unit tests, so there is that much.

It does have additional dependencies to build (I noticed at least pyyaml -- needed pip3 install pyyaml to get it to build), but the README.md is very out of date overall with respect to dependencies.

[fruffy]

The stuff in bf-asm/test is an old broken (non-functional) test suite that is based on running the even older python-based P4_14 compiler, and should probably just be deleted (nothing in it is used, and I don't think any of it is useful)

Okay, I will remove that code.

It looks like it does build fine (with just -DENABLE_TOFINO), but there aren't any tofino test cases? It seems like it should be possible to run some of the code in testdata through p4c --target tofino and then the assembler, but there's nothing set up to try to do that. When run manually, it looks ok, though. Currently ctest does run gtestasm, which runs some assembler internal unit tests, so there is that much.

Currently, compilation should be covered by the P4Testgen tests which runs the compiler with the following arguments: https://github.com/p4lang/p4c/blob/main/backends/p4tools/modules/testgen/targets/tofino/test/TestTemplate.cmake#L94

But it is not running packet-tests yet because the model isn't integrated.

One problem is the amount of warnings being thrown: https://productionresultssa18.blob.core.windows.net/actions-results/2f425077-cf0d-43ba-b2d1-299f3e6c0924/workflow-job-run-db3cdbd7-db2f-5c6d-ea50-8a2acf176300/logs/job/job-logs.txt?rsct=text%2Fplain&se=2025-02-18T13%3A36%3A26Z&sig=znJVJB1RLVnjLjLQb3KD732dHNwMwxMpYLNV7gyj%2FKg%3D&ske=2025-02-19T00%3A43%3A35Z&skoid=ca7593d4-ee42-46cd-af88-8b886a2f84eb&sks=b&skt=2025-02-18T12%3A43%3A35Z&sktid=398a6654-997b-47e9-b12b-9515b896b4de&skv=2025-01-05&sp=r&spr=https&sr=b&st=2025-02-18T13%3A26%3A21Z&sv=2025-01-05

Most of them seem related to particular macro usage in the tables file. The expansion causes additional ; being added.

[ChrisDodd]

One problem is the amount of warnings being thrown:

Most of the warning seem to come from base class virtual methods that have unused arguments -- these arguments are named for documentation/clarity (basically noting what the derived classes will do with them) even though they are not used in the base class method. Fixing the warnings by removing the arguments will make the code less self-documenting and harder to read/understand. I guess the arg names should be replaced by comments to both keep the info and silence the warnings.

[fruffy]

One problem is the amount of warnings being thrown:

Most of the warning seem to come from base class virtual methods that have unused arguments -- these arguments are named for documentation/clarity (basically noting what the derived classes will do with them) even though they are not used in the base class method. Fixing the warnings by removing the arguments will make the code less self-documenting and harder to read/understand. I guess the arg names should be replaced by comments to both keep the info and silence the warnings.

Slowly working my way through the warnings. There are a lot of warnings I can not fix because they are generated using chip.schema. Is there any way we can influence the generation of that code?

[ChrisDodd]

Slowly working my way through the warnings. There are a lot of warnings I can not fix because they are generated using chip.schema. Is there any way we can influence the generation of that code?

That code is all generated by the python code in bf-asm/walle/csr.py -- I'll try to take a look at it this weekend.

[fruffy]

That code is all generated by the python code in bf-asm/walle/csr.py -- I'll try to take a look at it this weekend.

Currently disable these warnings using -Wno-unused-parameter -Wno-unused-variable -Wno-type-limits -Wno-sign-compare. I think a lot of them are trivially fixable.