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

Typedef annotations is not generated correctly #4914

Open
komaljai opened this issue Sep 16, 2024 · 5 comments
Open

Typedef annotations is not generated correctly #4914

komaljai opened this issue Sep 16, 2024 · 5 comments
Labels
bug This behavior is unintended and should be fixed. control-plane Topics related to the control-plane or P4Runtime. core Topics concerning the core segments of the compiler (frontend, midend, parser)

Comments

@komaljai
Copy link
Contributor

komaljai commented Sep 16, 2024

@tc_type("mac") typedef bit<48> macaddr_t;

header ethernet_t {
    macaddr_t dstAddr;
    macaddr_t srcAddr;
    bit<16> etherType;
}

This should translate to

header ethernet_t {
    @tc_type("mac") bit<48> dstAddr;
    @tc_type("mac") bit<48> srcAddr;
    bit<16> etherType;
}

Compiler should copy annotations for 'typedef' (which is not currently supported)

@komaljai komaljai added core Topics concerning the core segments of the compiler (frontend, midend, parser) bug This behavior is unintended and should be fixed. labels Sep 16, 2024
@fruffy
Copy link
Collaborator

fruffy commented Sep 19, 2024

This is probably the eliminate typedefs or eliminate new type pass. Both of them are simple and this can be fixed. Are you able to submit a PR for this?

@jafingerhut
Copy link
Contributor

Are you also expecting that such annotations on typedef declarations are copied over to all declarations that use that typedef name, e.g.

  • local variables declared within a control
  • local variables declared within a parser
  • parameters of parsers, controls, actions, functions

If you are only expecting them to copy over to fields in header type definitions, why not the others?

If you are expecting them to copy over to all places that a typedef name is used, are you expecting that if the compiler renames a local variable in a control in some pass, that these annotations are preserved?

I think there are also cases where a compiler pass can create new variables with types that are copied from existing variables. If so, would you expect that the annotations be copied from the existing variable to the new variable in such cases?

@komaljai
Copy link
Contributor Author

komaljai commented Sep 24, 2024

I believe the annotations should be copied everywhere wherever "typedef" is replaced, otherwise we are losing information.

@jafingerhut
Copy link
Contributor

Makes sense, and that does seem like a good goal to me.

I mainly wanted to warn you that besides the one pass that replaces typedef with the underlying types, there might be other compiler passes that then introduce new synthesized variables whose types are copies of those variables, and it might be important to ensure that such annotations are copied to those synthetic variables, too. Sorry, but I do not have a complete list of passes where such introduction of synthetic variables with a copy of the type of an existing variable is done.

@jafingerhut jafingerhut added the control-plane Topics related to the control-plane or P4Runtime. label Oct 27, 2024
@jafingerhut
Copy link
Contributor

@komaljai or anyone else looking at this issue: Is your goal to generate P4Info files where table key fields, action parameters, and/or other kinds of things represented in a P4Info file, have annotations on them that are "inherited" from annotations on the typedef declaration in the user's P4 source program?

If so, note that p4c's P4Info generation is after the last front-end pass, and before the first mid-end pass, and the pass that removes typedef from the IR is in the mid-end pass.

If you want annotations on typedef to affect the contents of the P4Info file, you must either:

(a) modify the P4Info generation code to look up the typedef type of key fields, action parameters, etc.
(b) add a new front-end pass that propagates the annotations from typedef to expressions with that type.

or perhaps both (a) and (b) might be required.

For (a), note that this issue and some of its code snippets and discussion in comments might be useful in finding a solution: #2310

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug This behavior is unintended and should be fixed. control-plane Topics related to the control-plane or P4Runtime. core Topics concerning the core segments of the compiler (frontend, midend, parser)
Projects
None yet
Development

No branches or pull requests

3 participants