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

Bug in p4c Predication pass with some kinds of if statements inside of action bodies #4370

Open
jafingerhut opened this issue Jan 29, 2024 · 3 comments
Labels
bug This behavior is unintended and should be fixed. predication-pass Issues related to p4c's Predication pass

Comments

@jafingerhut
Copy link
Contributor

jafingerhut commented Jan 29, 2024

Now that PR #4999 has been merged in, this bug will no longer be reproducible by compiling the given test program for the BMv2 back end. Possible ways to reproduce the issue now:

  • Make a local change in your copy of the p4c code that adds the Predication pass back in to the BMv2 midend passes, by undoing the parts of that PR that remove the Predication pass.
  • Revert to older version of p4c before Enable handling of arbitrary if statements within actions for BMv2 back end #4999 was merged in.
  • (untried, and seems less likely to work) Modify the test program source code to compile for a P4 architecture and back end that still uses the Predication pass, e.g. the Tofino back end, might also help in reproducing it.

I believe most occurrences of if statements inside action bodies are handled correctly by p4c, but the example P4 program nested_ifs_in_action-bmv2.p4 in the attached zip file appears to trigger a bug in p4c related to the if statements in its one action. From looking at the output of every compiler pass, the first time a bug is introduced appears to be in the Predication mid-end pass.

The attached program nested_ifs_in_control-bmv2.p4 is functionally the same as nested_ifs_in_action-bmv2.p4, except that all of the if statements are in the ingress control, instead of in an action. Otherwise, their behavior in processing packets should be the same.

I have tested this with the following version of p4c source code, but I would guess this bug exists in many other versions of the source code, as I do not think the Predication implementation code changes very often:

commit cbbe594d55e9c09a09d2ad61187b954aa84a0b92 (HEAD -> main, origin/main, origin/HEAD)
Author: jkhsjdhjs <[email protected]>
Date:   Sat Jan 13 11:31:55 2024 +0100

p4c-issue-4370.zip

@fruffy
Copy link
Collaborator

fruffy commented Jan 29, 2024

Could you add a PR with this example program and predication enabled in P4Test? I believe the pass is not used in many back ends because of how bug prone it is. There at least 7-8 incorrect transformation bugs in it.

It might be difficult to fix.

@jafingerhut
Copy link
Contributor Author

@fruffy I have attempted to do so in this PR #4371. Let me know if you wanted a different form for such a PR.

Understood if this might be difficult to fix -- I was not expecting anyone to tackle it soon. I mainly wanted to be sure the test case was preserved for testing a possible future fix.

Is there already a label to mark p4c issues that are believed to be related to bugs in the predication pass? Do you think it would be useful to create one?

@fruffy fruffy added the bug This behavior is unintended and should be fixed. label Jan 29, 2024
@jafingerhut jafingerhut added the predication-pass Issues related to p4c's Predication pass label Feb 7, 2024
@jafingerhut
Copy link
Contributor Author

I created a new label "Predication pass" and added that label to this issue. Feel free to use it for any other issues that might be bugs in that pass.

@jafingerhut jafingerhut changed the title Bug in p4c with some kinds of if statements inside of action bodies Bug in p4c Predication pass with some kinds of if statements inside of action bodies Nov 6, 2024
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. predication-pass Issues related to p4c's Predication pass
Projects
None yet
Development

No branches or pull requests

2 participants