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

Types supported by emit #161

Closed
mihaibudiu opened this issue Apr 14, 2017 · 6 comments
Closed

Types supported by emit #161

mihaibudiu opened this issue Apr 14, 2017 · 6 comments
Milestone

Comments

@mihaibudiu
Copy link
Contributor

The spec mentions that emit can handle headers, arrays or structs, but does not say what happens if a bit field is passed as argument.

@jafingerhut
Copy link
Collaborator

And if emit supports structs, perhaps it should also say only those structs that contain types allowed by emit, which should probably be only bit<W> int<W> and varbit<W> ?

@mihaibudiu
Copy link
Contributor Author

This is a reasonable choice.
I have just implemented a check in the compiler which is more strict: it requires a struct that recursively contains just headers inside (or stacks).
We can definitely support all legal header fields too - and claim they are always valid.
If we decide to accept scalars it will require some work in the BMv2 back-end - in that case we should file a bug with the compiler.

@jafingerhut
Copy link
Collaborator

If by scalars you include enums and errors, I believe the current spec explicitly says that their representations can be target-dependent and do not support casts between them and other types. If that stays, then it is not possible to support emit on those types (or at least not in a way portable across targets).

@mihaibudiu
Copy link
Contributor Author

mihaibudiu commented Apr 14, 2017

I was too lazy to say bit<> etc.

@jafingerhut
Copy link
Collaborator

From memory on what was discussed on this topic at the 2017-Apr-25: emit handling headers, arrays (i.e. header stacks), and structs is sufficient to do real work.

Being able to emit a 'bitty type' (i.e. bit<W> int<W> or varbit<W>) is over and above what the spec requires today, over and above what the open source tools support today, and raises questions like what should happen if one emits a field whose length is not a multiple of 8 bits long on a target that only supports headers that are multiples of 8 bits. It seems safe to postpone a decision on such an enhancement until after the first P4_16 release.

Another comment: If someone does want to emit a bit vector allowed by the target, they can certainly do so by putting it into a new header type, making an instance of that type valid, and emitting that (or a struct). So the lack of ability to emit a bit vector directly is not a big restriction.

@mihaibudiu
Copy link
Contributor Author

We have decided not to change the spec, so I am closing this issue.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants