Clarify whether P4-16 allows `switch` statements in actions

[kfcripps]

Current behavior of the p4c compiler is:

1. Report the following error message for backends that do run the EliminateSwitch midend pass:

error: SwitchStatement: switch statements not supported in actions on this target

2. Supports switch statements in actions for backends that do not run the EliminateSwitch midend pass.
   • Note that this is not tested in the p4c repository because the p4test backend does run the EliminateSwitch midend pass.

However, the P4-16 spec seems to indicate that switch statements are not supported in actions at all.

For example, returns are not supported in parsers, and switch statements are only supported in control blocks.

The switch statement can only be used within control blocks.

Should the P4-16 spec be modified to allow switch statements in actions? Or should the above error message be moved to a frontend pass that is by default run for all backends?

Below is an example program, which backends that do not run EliminateSwitch do accept, and backends that do run EliminateSwitch do not accept.

extern void __e(in bit<28> arg);

struct meta_t {
    bit<8> f1;
    bit<8> f2;
}

action foo(inout meta_t meta) {
    switch (meta.f1 + meta.f2) {
        0 :       { __e(0); }
        2 :       { }
        3 :
        5 :       { __e(5); }
        7 :
        default : { __e(99); }
    }
}

control C(inout meta_t meta) {
    table t {
        actions = { foo(meta); }
        default_action = foo(meta);
    }

    apply {
        t.apply();
    }
}

control proto(inout meta_t meta);
package top(proto p);

top(C()) main;

[kfcripps]

@jafingerhut @vlstill @vgurevich

[jafingerhut]

I have no objections language-wise to allowing switch statements in action bodies.

Historically, my initial proposed implementation of switch (expr) ... was to desugar it with a switch (newtable.apply().action_run) ... statement, which is not supported in an action, so that implementation would need to change within action bodies to support switch (expr) ... in action bodies.

[kfcripps]

@jafingerhut I suppose that a backend that cannot support switch statement in actions has two options:

1. EliminateSwitch (or a different midend pass) can transform the switch statements into if/else statements or
2. EliminateSwitch can continue to emit the same error message that it currently does

[vgurevich]

@kfcripps -- having two distinct midend passes definitely makes sense. It might even make sense to have an additional annotation to guide the compiler, e.g. in case the desugaring happens inside a control, where both approaches should be possible and the compiler might have difficulty choosing one method over another.

The implementation that uses action_run clearly cannot be used today inside an action.

[jafingerhut]

Another option would be:

3. The switch statement IR node is left in the action body until it reaches the back end, and back ends must be updated in order to support switch statements in action bodies.

Is #3 possibly the best choice for back ends that can optimize switch statements inside actions, in the implementation on their target?

If we really want P4 programs that have switch statements inside of actions to work on BMv2 (without updating BMv2), then yes, at least an option to replace such switch statements with if/else would be needed.

[kfcripps]

Ah, there were already a couple of PRs merged several years ago that intended to allow switch statements in actions in both the P4-16 spec (see #945) as well as p4c compiler (by simply removing an error message that used to be in the ValidateParsedProgram frontend pass: p4lang/p4c#2820 (comment)).

I think there are just a couple of places in the spec (mentioned in the issue description) that were missed by #945 and need to be cleaned up. If nobody has any objection to this, I can open a PR to do that.

[kfcripps]

#1363

[vlstill]

Another option would be:

3. The switch statement IR node is left in the action body until it reaches the back end, and back ends must be updated in order to support switch statements in action bodies.

Is #3 possibly the best choice for back ends that can optimize switch statements inside actions, in the implementation on their target?

I would say it is. Luckily we can always just not use the desugaring passes for them :-).

Side note: this is another case that highlights that we should have some more flexibility than p4test currently has for testing P4C. We should definitely be able to test it, but that is a compiler issue, not spec one.

[jafingerhut]

IIRC there are already command line options for p4c for completely skipping arbitrary passes. I don't think p4c CI ever uses that today, but such tests could be developed.