p4lang / p4c

P4_16 reference compiler
https://p4.org/
Apache License 2.0
683 stars 446 forks source link

`ComputeWriteSet` does not properly maintain the calling context #4811

Open kfcripps opened 4 months ago

kfcripps commented 4 months ago

See https://github.com/p4lang/p4c/pull/4810 and related discussions for context.

Consider an IR::SwitchStatement that has two unique IR::SwitchCases (let's call them "case 1 and 2"), that each point to the same IR::BlockStatement*.

When traversing from preorder(const IR::SwitchStatement *statement) down to preorder(const IR::BlockStatement *statement), the callingContext is not updated, so when we are setting the IR::BlockStatement*'s definitions, the ProgramPoint for the IR::BlockStatement* pointed by both case 1 and case 2 will be equivalent.

This is just one example of the problem, but there are also at least several other (and possibly many other) places in ComputeWriteSet where this is a problem.

Solving this problem should allow us to disable the Cloner and RemoveHidden passes, as suggested by @mihaibudiu in #4797.

https://github.com/p4lang/p4c/pull/4810#issue-2409982045 mentions two possible solutions for the problem.

To better understand the problem:

  1. Checkout changes from https://github.com/p4lang/p4c/pull/4810#issue-2409982045
  2. Disable Cloner and RemoveHidden passes, run unit tests, and analyze the failures
kfcripps commented 4 months ago

We should also consider reverting #4727 after this has been fully fixed.