Add check of uniqueness of control plane names

[dmatousek]

Control plane names are not checked for uniqueness. It may lead to various issues in subsequent passes when uniqueness is assumed.

[jafingerhut]

By any chance, is this intended to address an issue like the one described in this issue? https://github.com/p4lang/p4c/issues/2755

One of the example programs on that issue has identical @name annotations on two different actions, which seems to me like a bug to accept, but note that internal passes in open source p4c create this situation by design right now (which I personally also think is a mistake, and would be better to avoid, but it has been like this for years).

[mihaibudiu]

See also #3037, which didn't work.

[jafingerhut]

I tried building p4c with the proposed changes on this PR after 3 commits, and ran it locally on my system.

It fails for a very simple P4 program with 2 tables, both of which have NoAction in their actions list. You should be able to reproduce this with the following commands:

$ git clone https://github.com/jafingerhut/p4-guide
$ cd p4-guide/name-annotations/v1model
$ p4c --target bmv2 --arch v1model --p4runtime-files actions-5-no-annot.p4info.txt actions-5-no-annot.p4 
/usr/local/share/p4c/p4include/core.p4(70): [--Werror=duplicate] error: .NoAction: conflicting control plane name: .NoAction
action NoAction() {}
       ^^^^^^^^
/usr/local/share/p4c/p4include/core.p4(70)
action NoAction() {}
       ^^^^^^^^

As far as the original source code given to the compiler, there are no name conflicts at all.

The error messages occur, I believe, because of the way that the p4c LocalizeAllActions frontend pass duplicates actions in the IR when the same action is an action of multiple P4 tables, and automatically creates identical @name annotations on these duplicated actions.

See #2755 for additional notes and details. I would love to have a meeting of the minds of P4 Runtime API folks, compiler folks, etc. to see what can or should be done about this. I have ideas described on that issue, but not sure what the best way to go here is.

[dmatousek]

Thanks for your feedback and referencing related issues. I agree that there is no clear fix and it has to be further discussed.

[jafingerhut]

@dmatousek I have scheduled a meeting with a few Intel folks knowledgeable about the P4 compiler to discuss a possible way forward on issue #2755. I will update that issue when/if there is any news to share.

[jfingerh]

@dmatousek I spoke with some folks at Intel including @hanw about the underlying issue here, and he had a suggestion for an experiment to try: put the @name conflict checking somewhere in the front end before the LocalizeAllActions pass, which is one that we know intentionally creates copies of actions with identical @name annotations. If there is a @name conflict before that pass, it should be one that the P4 developer themselves wrote.

The only disadvantage I can think of to such an approach is that now P4_16 source code produced as the output of passes after LocalizeAllActions could cause errors when used as input to p4c. I do not know how serious that is, but perhaps having a command line option that skips the new @name conflict check would help with that.