[WIP] Export tool starter, trial and error code

[Joejiong]

[https://github.com//issues/151]
I have another experimental code in progress not yet uploaded, and it uses another approach.

There are a couple of paths to eventually make BMV2 JSON.

This code only served as the first part of the two-step JSON paths.
p4hir-> p4hir-iso-json (debug+correctness+extensibililty)->bmv2-json (deal with the p4c-bmv2 json generator logic, with ID and others)

[fruffy]

Why is the mlir code for this file empty? Does it not compile? What is the expected json output for it? Maybe you should add it as a reference.

[Joejiong]

oh thx for the reminder, forgot to upload. Let me upload it

[Joejiong]

The problem I'm facing with this method is looped reference and typedefs, which can form a dead loop, for complex real p4hir code with includes v1 arch and core. I'm trying to build a set to store the KV mapping. However, there is another completely different methodology. For this skeleton, I'm experimenting with this starter code, building a set for typedefs, and traversing nodes could probably prevent the dead loop. But fundamentally, a simple static DAG cannot represent a loop in the current frontend. This will be left for future discussions.

[fruffy]

Try this code as example: https://github.com/p4lang/p4c/blob/main/backends/p4tools/modules/testgen/targets/bmv2/test/p4-programs/bmv2_extract_1.p4

I know this already compiles to mlir and it is a very simple example of a BMV2 program. Then compile this program and generate the json file using p4c-bm2-ss. You should be able to generate basic elements already.

I am not sure what looped references or typedefs are.

[fruffy]

How to produce text output from this particular MLIR dialect you can check https://github.com/p4lang/p4mlir-incubator/blob/fruffy/top4/lib/Utilities/mlir_to_p4.h#L20. It writes MLIR to P4 again. This code is WiP, by no means complete, and will change. But it is a start and shows that it can be done.

[Joejiong]

Don't worry about for loops for now, there is very little Bmv2 code written that way. Just throw an error that it is not implemented,

got you, thx

[qobilidop]

@Joejiong Could you also check in the expected BMv2 .json file? I still don't see it.

[Joejiong]

OK, let me include it

[Joejiong]

@Joejiong Could you also check in the expected BMv2 .json file? I still don't see it.

The output also has a p4i file, I probably only upload the .json

[qobilidop]

Yes, including .json file is enough.

[qobilidop]

As explained above, let's not invent this P4HIR-ISO json format here. Instead, try to emit the BMv2 JSON format directly.

I'd suggest that you only keep the emitModule function, and start with returning an empty or constant JSON value. Once you have that working (in the sense that the added test passes, see another comment), we can discuss the next steps.

[qobilidop]

Please add a test in the test/ directory that invokes the binary on this file as input.

[qobilidop]

@Joejiong Could you also check in the expected BMv2 .json file? I still don't see it.

[qobilidop]

This code only served as the first part of the two-step JSON paths.
p4hir-> p4hir-iso-json (debug+correctness+extensibililty)->bmv2-json (deal with the p4c-bmv2 json generator logic, with ID and others)

@Joejiong I think it makes sense to have multiple steps in converting from P4HIR to BMv2 JSON, but:

1. For this PR, we are not solving that problem. I'd keep the scope of this PR to be just setting up some starter code without assuming how to do the conversion.
2. I don't think it's necessary to introduce an intermediate textual format for solving the conversion problem. We are planning to create a BMv2 dialect for this instead.
3. Without introducing any intermediate steps, in the starter code, we are going to directly convert some easy parts of P4HIR to BMv2 JSON directly. To see which parts are easy, we need to see the expected BMv2 JSON file first. If there is no easy parts, outputing a skeleton BMv2 JSON file is also fine.

[Joejiong]

2. I don't think it's necessary to introduce an intermediate textual format for solving the conversion problem. We are planning to create a BMv2 dialect for this instead.

I also feel it is too difficult to transfer; there are so many corner cases, which bmv2 dialect would better capture.

Conversion p4hirdialect->bmv2dialect would be more accurate than my JSON converter. So much manual work.

[qobilidop]

Now that we have this expected BMv2 JSON file, we can discuss the next step. The exercise I'd like to see you implement in this PR is to generate the "header_types" section automatically.

The implementation shall be very straight-forward. You just need to traverse the input P4HIR program in some way and collect all header type related definitions. Make your best guesses at what is counted as header types. Reference the existing P4C BMv2 backend code for inspirations.

You don't have to generate code that look exactly the same (e.g. assigned IDs could be different). If they look roughly the same, that's good enough for this PR.

@Joejiong Any questions?

[Joejiong]

header_types is like a struct declaration？ Got to look at it later, EST time is late. Thx for the elaboration.

[qobilidop]

No problem. Take your time. https://github.com/p4lang/behavioral-model/blob/main/docs/JSON_format.md has some explanations.

[Joejiong]

I finally found the error, but it stopped at my newly added test. Why could I run it on my local machine?

/mlirTojson/p4mlir-incubator/third_party/llvm-project/build/bin/llvm-lit -v /mlirTojson/p4mlir-incubator/third_party/p4c/build/extensions/p4mlir/test/Export/P4HIR/bool_test.mlir
-- Testing: 1 tests, 1 workers --
PASS: P4MLIR :: Export/P4HIR/bool_test.mlir (1 of 1)

Testing Time: 0.14s

Total Discovered Tests: 1
  Passed: 1 (100.00%)
+ PASSED_TESTS=35
+ echo PASSED
PASSED

[qobilidop]

In general your local environment and CI environment are not exactly the same. So things could be different. I usually use Docker to create a reproducible local environment that's very close to the CI environment to rule out such possibilities.

Other than that, here are my suggestions on what you could try to debug:

1. Rebase to or merge from HEAD to rule out the possibility that something already fixed in HEAD has been causing your issue.
2. Replace the test file by an existing passing test.
   1. If this works in CI, it means your added test has some issue.
   2. If this also doesn't work in CI, then it must be something else.

[asl]

Other than that, here are my suggestions on what you could try to debug:

GC is likely culprit. It is not compatible with MLIR as the latter uses TLS and GC is not looking over TLS causing premature frees (and double frees). GC should be explicitly turned off if P4C libraries are linked in.

This is why it is extremely important to ensure everything is "pure" MLIR w/o P4C libraries.

[fruffy]

GC is likely culprit. It is not compatible with MLIR as the latter uses TLS and GC is not looking over TLS causing premature frees (and double frees). GC should be explicitly turned off if P4C libraries are linked in.

If you are referring to thread-local storage with TLS BDWGC should support that. Probably just an option to tweak.

[asl]

If you are referring to thread-local storage with TLS BDWGC should support that. Probably just an option to tweak.

No, it does not. See https://github.com/ivmai/bdwgc/blob/master/README.md:

WARNING: the collector does not guarantee to scan thread-local storage (e.g. of the kind accessed with pthread_getspecific). The collector does scan thread stacks, though, so generally the best solution is to ensure that any pointers stored in thread-local storage are also stored on the thread's stack for the duration of their lifetime. (This is arguably a longstanding bug, but it hasn't been fixed yet.)

[fruffy]

If you are referring to thread-local storage with TLS BDWGC should support that. Probably just an option to tweak.

No, it does not. See https://github.com/ivmai/bdwgc/blob/master/README.md:

WARNING: the collector does not guarantee to scan thread-local storage (e.g. of the kind accessed with pthread_getspecific). The collector does scan thread stacks, though, so generally the best solution is to ensure that any pointers stored in thread-local storage are also stored on the thread's stack for the duration of their lifetime. (This is arguably a longstanding bug, but it hasn't been fixed yet.)

It works in practice, but the guarantees aren't good, see https://github.com/ivmai/bdwgc/blob/ef01762da61fc803f430751506f6ee2176fe9221/docs/platforms/README.linux#L59.

[asl]

It works in practice, but the guarantees aren't good, see https://github.com/ivmai/bdwgc/blob/ef01762da61fc803f430751506f6ee2176fe9221/docs/platforms/README.linux#L59.

Well, I definitely saw crashes and double frees with MLIR with GC enabled :) So, I disabled it by purpose.

[fruffy]

Well, I definitely saw crashes and double frees with MLIR with GC enabled :) So, I disabled it by purpose.

I guess the question is, does GC_disable() work properly? Or did you have to disable GC entirely? The MLIR code doesn't need it anyway and we only really need GC for the front-end for large programs.

[asl]

I guess the question is, does GC_disable() work properly?

It stops all mark&sweep threads, so it means no automatic memory frees anymore. Manual allocation / deallocation work (albeit slow).

need GC for the front-end for large programs.

Sadly, "large" is just couple of thousands of LOCs, if entire frontend is run :)

[fruffy]

It stops all mark&sweep threads, so it means no automatic memory frees anymore. Manual allocation / deallocation work (albeit slow).

So, in the case of the translate utility, if we call GC_disable() after the front end passes, the only leakage we should have is the P4 Program plus whatever leaks from the call to toMLIR, right?

[asl]

So, in the case of the translate utility, if we call GC_disable() after the front end passes, the only leakage we should have is the P4 Program plus whatever leaks from the call to toMLIR, right?

plus whatever is in the static variables of the frontend, some some data structures (we still rely on type map, sadly in few places), plus whatever is not cleared after the frontend, yes.