int parameters must be directionless

[mihaibudiu]

Fixes #2354

[mihaibudiu]

Fixes #2357

[jafingerhut]

Is it true that all values of type int in a P4_16 program must be compile-time known values?

Is a directionless parameter of an action a compile-time known value?

If so, I do not understand how it can be, since the control plane software could install table entries using such an action with values for such parameters that are not known at compile time.

[mihaibudiu]

I guess this is necessary, but not sufficient. Actions cannot have any int parameters at all. I should add a check for that.

[jafingerhut]

Not only actions, but any run-time parameters in general, whether they be for actions, functions (extern or written-in-P4 kinds), methods, controls, or parsers.

Constructor parameters for extern objects, parsers, and controls seem reasonable to have type int, but I cannot think of any other kinds of parameters where type int makes sense to me.

[mihaibudiu]

No, functions can have int parameters. For example, the compiler typechecks the declaration of pop_front as if it is an extern method of a stack that has the signature pop_front(int index);. This requires the argument to be a constant.

[jafingerhut]

I can see that it makes sense for a control, parser, or function to have a run-time parameter that is directionless or in with type int, as long as all calls in the program source code have compile-time known integer values given in the calls, for all such parameters.

Can actions have directionless parameters with type int? I do not see how that would make sense if the control plane API ever was required to provide such a parameter value, since that would not be a compile-time known value.

[jnfoster]

I think it could be legal if the action were used as a default_action? action a(int x) { ...}

table t = { ... default_action = a(42); }

Unclear if this is useful of course :-)

…

-N

On Wed, May 6, 2020 at 3:44 AM Andy Fingerhut ***@***.***> wrote: I can see that it makes sense for a control, parser, or function to have a run-time parameter that is directionless or in with type int, as long as all calls in the program source code have compile-time known integer values given in the calls, for all such parameters. Can actions have directionless parameters with type int? I do not see how that would make sense. — You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub <#2360 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AAOCFURWUN3B2BPQUWZK7MTRQEIMFANCNFSM4MY56E5A> .

[mihaibudiu]

Actually in #2360 I do reject all int parameters for actions.
So that should probably make it to the spec too.

[mihaibudiu]

Are these changes in line with the spec?
If so, perhaps someone can approve this PR?

[jafingerhut]

@mbudiu-vmw I am currently unclear on what the spec actually says should be allowed, versus not allowed, for values with type int. I mean, I can look at some example programs and see 'yeah, that is clearly a run-time value with type int, so not allowed', and I can look at another program like the issue2354.p4 added as part of this P4, excerpted below:

control ingress(inout Headers h, inout Meta m, inout standard_metadata_t sm) {
    action do_action1(in int val) {
        bool test = 8w1 == val;
    }
    action do_action2(in int val) {
        h.eth_hdr.eth_type = h.eth_hdr.eth_type + val;
    }
    apply {
        do_action1(1);
        do_action2(1);
    }
}

and it seems to me that it is open to interpretation of the English text in the current spec whether it allows that, or not.

[mihaibudiu]

this would be illegal using the latest PRs against the spec.
any int parameter must be directionless.

[jafingerhut]

Perhaps if there was a brief description of these p4c changes, along with proposed changes to the language spec that we think are consistent with those, then I could try comparing those two things to each other, and the p4c changes to the code changes (or at least the example programs?)

[jafingerhut]

@mbudiu-vmw "this would be illegal using the latest PRs against the spec." Can you provide a link to the PR you mean?

[mihaibudiu]

I actually thought I had filed a PR against the spec, but I don't see one.
This PR against the compiler requires int arguments to be directionless and forbids them altogether for actions.

[jafingerhut]

Mihai, if you want me to take a shot at writing a spec PR for the changes below, let me know. If you already have one in the works, I can wait and review it.

Directionless parameters are used for two completely different purposes in P4_16 (and maybe others, but these are the two I could find in a quick scan for the term 'direction' in the spec):

• for everything other than an action (e.g. parser, control, function, ...), it represents the restriction that the argument value supplied in all calls must be a compile-time known value.
• for an action, it represents the fact that the parameter value is (at least usually) supplied by the control plane software when adding entries to a table that use that action. I say "at least usually" because a P4_16 program can also supply an expression as the argument for such directionless action parameters in explicit calls to the action, or in the table property actions (at least I think the last part might be true).

Given those two very different meanings for directionless parameters, it does seem reasonable to me to restrict type int parameters to be directionless for everything except actions, and to completely disallow type int parameters for actions.

If we wanted to be slightly more permissive, I could imagine allowing type int parameters for action parameters that were directionless, but only if all calls to the action supplied a compile-time known value for those parameters. However, the benefit of allowing such calls seems to me to be lower than the complexity of specifying that corner case, and implementing it in the compiler.

Aside: It does strike me as a bit odd that directionless parameters are used for these two completely different purposes, but changing that seems impossible without breaking backwards compatibility and confusing everyone familiar with the current language.

[jnfoster]

I agree. Unfortunately we didn't fix this issue (better syntax for control-plane-supplied action data) before P4_16 got finalized.

…

On Fri, May 15, 2020 at 2:19 PM Andy Fingerhut ***@***.***> wrote: Mihai, if you want me to take a shot at writing a spec PR for the changes below, let me know. If you already have one in the works, I can wait and review it. Directionless parameters are used for two completely different purposes in P4_16 (and maybe others, but these are the two I could find in a quick scan for the term 'direction' in the spec): - for everything other than an action (e.g. parser, control, function, ...), it represents the restriction that the argument value supplied in all calls must be a compile-time known value. - for an action, it represents the fact that the parameter value is (at least usually) supplied by the control plane software when adding entries to a table that use that action. I say "at least usually" because a P4_16 program can also supply an expression as the argument for such directionless action parameters in explicit calls to the action, or in the table property actions (at least I think the last part might be true). Given those two very different meanings for directionless parameters, it does seem reasonable to me to restrict type int parameters to be directionless for everything except actions, and to completely disallow type int parameters for actions. If we wanted to be slightly more permissive, I could imagine allowing type int parameters for action parameters that were directionless, but only if all calls to the action supplied a compile-time known value for those parameters. However, the benefit of allowing such calls seems to me to be lower than the complexity of specifying that corner case, and implementing it in the compiler. Aside: It does strike me as a bit odd that directionless parameters are used for these two completely different purposes, but changing that seems impossible without breaking backwards compatibility and confusing everyone familiar with the current language. — You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub <#2360 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AAOCFUW2F4DWC7BWU727YP3RRWBSPANCNFSM4MY56E5A> .

[mihaibudiu]

I wasn't planning to write anything, it is rather implicit in the spec. But we can always be more explicit.

[jafingerhut]

I created this proposed PR: p4lang/p4-spec#861

[ChrisDodd]

Technically this PR looks fine, only questions are about what the spec actually says and whether this is consistent.

[mihaibudiu]

I believe that whatever we do in the spec will not invalidate these assumptions, so I will merge this.
If this turns out to be false we'll file another issue.