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 #249 #250

Closed
wants to merge 5 commits into from
Closed

Fix for issue #249 #250

wants to merge 5 commits into from

Conversation

mihaibudiu
Copy link
Contributor

I didn't mean to add the previous commit with the exit tables, but I guess I didn't create the branch at the correct time.
The fix has two halves:

  • SynthesizeActions now has a more precise policy function, which is used to skip the verify, update and deparser blocks in the bmv2 back-end. (It is also used in the p4test back-end when compiling for the v1model).
  • Undo the effects of the SideEffectOrdering pass only for the update checksum control. This is a hack, but it is no worse than the current code generation for checksums; the proper solution will require the checksum block to be implemented using an extern in BMv2.
    @anirudhSK: this is (hopefully) a fix for the issues with the checksum you have reported on the BMv2 repo

@ChrisDodd
Copy link
Contributor

I believe I've merged this as 59150a7

@ChrisDodd ChrisDodd closed this Jan 23, 2017
mihaibudiu pushed a commit to mihaibudiu/p4c-clone that referenced this pull request Feb 1, 2017
ChrisDodd pushed a commit that referenced this pull request Feb 2, 2017
* Use cstring() instead of the empty cstring

* Two more improvements

* Fix for issue #270: improve on code submitted with PR #250 with a more principled approach

* Fixed some incorrect comments

* Bugfix following review comments

* Improvement suggested by review
@mihaibudiu mihaibudiu deleted the mbudiu branch February 3, 2017 02:22
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.

2 participants