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

Better support for header stacks in expr for bmv2 #338

Merged
merged 2 commits into from
Mar 1, 2017

Conversation

antoninbas
Copy link
Member

bmv2 now supports 'stack_field' type in expressions (not just for parser
transition keys). Until now the p4c bmv2 backend was assuming that this
support existed, which resulted in an incorrect JSON being generated in
many cases. See issue #321.

bmv2 now supports 'stack_field' type in expressions (not just for parser
transition keys). Until now the p4c bmv2 backend was assuming that this
support existed, which resulted in an incorrect JSON being generated in
many cases. See issue #321.
Copy link
Contributor

@mihaibudiu mihaibudiu left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There isn't much to comment on, this change seems very simple. I assume that the BMv2 side changed more significantly to support this feature.

auto version = mkArrayField(meta, "version");
version->append(version_major);
version->append(version_minor);

meta->emplace("compiler", "https://github.com/p4lang/p4c");
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There are multiple back-ends with this url, maybe this should be p4c-bm2-ss instead.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

my reasoning is that there is only one p4c backend for bmv2; anyway this has no real use right now except providing the ability to quickly see which compiler produced the json (old one or new one)

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

Successfully merging this pull request may close these issues.

3 participants