p4c icon indicating copy to clipboard operation
p4c copied to clipboard

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

Open jafingerhut opened this issue 1 year ago • 3 comments

Now that PR https://github.com/p4lang/p4c/pull/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 #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

jafingerhut avatar Jan 29 '24 13:01 jafingerhut

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.

fruffy avatar Jan 29 '24 13:01 fruffy

@fruffy I have attempted to do so in this PR https://github.com/p4lang/p4c/pull/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?

jafingerhut avatar Jan 29 '24 13:01 jafingerhut

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 avatar Feb 07 '24 19:02 jafingerhut