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

Fix for issue #638 #653

Merged
merged 12 commits into from
May 30, 2017
Merged

Fix for issue #638 #653

merged 12 commits into from
May 30, 2017

Conversation

mihaibudiu
Copy link
Contributor

By misusing git I lost this code, so I had to rewrite it.
This is a reimplementation of the fix which removes requirement for a default_action.
The compiler creates a default_action of NoAction is none is present; this makes all the dataflow analyses work correctly, since the dataflow analysis for a table is the join of the dataflow for all actions in the actions list. It also sets the stage for cross table code motion - one needs to have all possible actions in order to push code into all branches.

mbudiu-vmw and others added 7 commits May 23, 2017 13:18
- support for autogenerating methods in nested classes
- support for friend functions
- misc bug fixes
In the bmv2 JSON, a match-table can indicate that its default action is
constant (using 'action_const') and that the default action entry
(action + action parameter values) is constant (using
'action_entry_const'). `action_entry_const == true => action_const ==
true`. The bmv2 backend was setting 'action_entry_const' correctly, but
'action_const' was always set to false. Note that P4_16 does not make a
difference between the 2, the presence of `const` in the P4_16 source
implies that both attributes should be true.
* Support emits of structs; test parsing of varbit fields

* Fixes issue p4lang#447 and p4lang#630

* Updated reference outputs

* Additional tests and bug-fixes for varbits
Copy link
Contributor

@ChrisDodd ChrisDodd left a comment

Choose a reason for hiding this comment

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

This runs into trouble with P4_14 tables with no actions (not that such are meaningful, but they are apparently legal in P4_14). Such tables now get translated to P4_16 with an empty actions list (actions = {}) which causes a syntax error on p4test reparsing. Not sure what the best way of dealing with it is -- leaving out the actions altogether still gives an error (table X does not have an 'actions' property). Changing the grammar to allow an empty actionList fixes the problem but seems counter to the spec.

@mihaibudiu
Copy link
Contributor Author

The only logical solution seems to be to allow an empty actions list.
Is this the p4-14 spec?

@mihaibudiu
Copy link
Contributor Author

Let me then rework this PR by changing the grammar too.
We will need to amend the spec.

@mihaibudiu
Copy link
Contributor Author

Maybe I should not have merged... pulled some needless commits.
Anyway, this now allows empty actions lists in both P4-14 and P4-16, there are two test cases.

@ChrisDodd
Copy link
Contributor

Should be fine as long as you do a 'squash and merge' rather than 'rebase and merge' to merge the change -- that will get rid of the spurious merges.

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