Do not recompute write sets for equal child nodes belonging to a given `IR::Vector`
built on top of https://github.com/p4lang/p4c/pull/4797
Title and discussion in https://github.com/p4lang/p4c/pull/4797 are self-explanatory.
This still does not fully fix the problem described in https://github.com/p4lang/p4c/pull/4797 (but it is yet another incremental improvement). ComputeWriteSet's callingContext is not properly updated during the IR traversal, so ProgramPoints are not correct in many places. This can be fixed by either:
- Making sure that
ComputeWriteSet'scallingContextis properly updated andProgramPoints created correctly everywhere, or - Replace all
ProgramPoints withloc_t, so that updating the calling context is the responsibility of theVisitoroverhead, rather than ofComputeWriteSet.
Neither of these are trivial, but the latter is probably the most invasive option.
See https://github.com/p4lang/p4c/issues/4811
Not particularly fond of this API. Seems built just to hide this particular bug.
@mihaibudiu By API do you mean visit_unique_children()? I thought it might be useful for other passes that also do not want to traverse equal child nodes, but if you insist, I can make it local to ComputeWriteSet.
Also, this PR is not "hiding" any bug. It is one of several steps needed to fix the root problem that has been previously discussed.
This is @ChrisDodd's territory, so he should decide the fate of this API
(sorry I missed this PR when it first came around)
How do we get IR::Vectors with duplicate nodes in them? In all the cases where you're calling visit_unique_children it would seem that non-unique children are impossible (or at least should be a BUG if they ever occurred)?
For example, a BlockStatement like
a = a + 1;
a = a + 1;
that might be represented as the same Statement repeated twice, but the effect of each will be independent -- in particular, the use/defs of each will be different. This would imply that if this is a DAG with the same child appearing twice, we need to include the child_index in the loc_t to distinguish the two references.