[P4Testgen] The actions' coverage works weirdly if action is called from multiple tables

[vlstill]

Currently the --track-coverage ACTIONS (introduced for #4305) considers actions cloned for different tables to be equal. While that does make some sense, I believe it would make more sense to cover all such clones independently. That way, it would correspond to the more natural "for each table, cover all actions it can invoke and also cover directly-invoked actions".

The tricky part is that p4testgen itself clones actions when executing tables, so we would need some way to distinguish actions cloned in midend when actions are localized to each table and actions cloned in testgen.

@fruffy, @jnfoster what do you think?

[fruffy]

I think we should stick with whatever definition of uniqueness source info gives us for now. Just to keep things simple. Based on a program coverage view this behavior technically correct since we have already invoked that action.

One thing we can do is to use ActionListElement instead. That element should be unique per table.

[vlstill]

I think we should stick with whatever definition of uniqueness source info gives us for now. Just to keep things simple. Based on a program coverage view this behavior technically correct since we have already invoked that action.

I'm not saying we need to change the way coverage tracking is implemented. That said, I think the to interpret actions' coverage as I suggested is the more natural way.

One thing we can do is to use ActionListElement instead. That element should be unique per table.

Probably not instead, as that would discard directly-invoked actions. But we could track it on top of the action itself (but still as part of --track-coverage ACTIONS). That would probably work fine and not require change in the way we keep track of the tracked nodes.

---

BTW. I've now realized this touches on something more complex still. For example, if there is a sub-control invoked from multiple controls (say in a multi-pipeline architecture, or from both ingress and egress in something like PSA), we would probably also track everything there together (i.e. it would be sufficient to just cover all the statements/actions in the first invocation). This is probably more niche then the case for actions, but we might want to consider what to do in this case too.

[fruffy]

BTW. I've now realized this touches on something more complex still. For example, if there is a sub-control invoked from multiple controls (say in a multi-pipeline architecture, or from both ingress and egress in something like PSA), we would probably also track everything there together (i.e. it would be sufficient to just cover all the statements/actions in the first invocation). This is probably more niche then the case for actions, but we might want to consider what to do in this case too.

Yes, our approach to this type of coverage has been "good enough" so far. There are a lot of subtleties that emerge with how the compiler performs node transformation, inlining etc. An other example: What does coverage mean for something like

bit<8> value
if (flag)
  value = 1;
else
  value = 2

which is transformed into bit<8> value = flag ? 1 : 2; or vice versa. Often source information is duplicated or simply lost.

I have been thinking of something like uniquely stacking source information to make sure we accurately keep track. Something like a stack trace. But so far there has not been any pressing need and this opens a deep rabbit hole.

[vlstill]

BTW. I've now realized this touches on something more complex still. For example, if there is a sub-control invoked from multiple controls (say in a multi-pipeline architecture, or from both ingress and egress in something like PSA), we would probably also track everything there together (i.e. it would be sufficient to just cover all the statements/actions in the first invocation). This is probably more niche then the case for actions, but we might want to consider what to do in this case too.

Yes, our approach to this type of coverage has been "good enough" so far. There are a lot of subtleties that emerge with how the compiler performs node transformation, inlining etc. An other example: What does coverage mean for something like

bit<8> value
if (flag)
  value = 1;
else
  value = 2

which is transformed into bit<8> value = flag ? 1 : 2; or vice versa. Often source information is duplicated or simply lost.

It would make more sense to me to consider IR::Mux and IR::IfStatement just as two different syntaxes for the same thing. So both of then should create a branch in the search space and both branches should count to the coverage. But I can see how that is not the case now, since the branches of if are statements, but branches of Mux are not.

I have been thinking of something like uniquely stacking source information to make sure we accurately keep track. Something like a stack trace. But so far there has not been any pressing need and this opens a deep rabbit hole.

That would make a lot of sense to me. It could be quite tricky to do as there can be:

• action used in different table;
• sub-control instantiated multiple times in a control;
• possibly multiple instances of the top-level control itself in the main package;
• and even worse, the inlining of actions and functions makes this even harder.

Therefore this would probably need a change in the SourceInfo itself and the inlining and actions splitting passes would need to be responsible for adding the context. I don't see a way of doing this in the testgen, as a lot of this information is already lost at that stage (mainly action/function/control/parser inlining mainly). That, I think, would solve all of the cases, except possibly for the multiple instances of top-level control but that is probably only relevant for architectures with multiple pipelines.