p4lang / p4c

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

I have some questions about using the "inout" parameter in the "action parameter" #4579

Open liao123123123 opened 7 months ago

liao123123123 commented 7 months ago

The command "p4test test.p4 -v" is used to compile the code. When I added the "inout" parameter to the action parameter and Changed parameters (act,act(h.mpls[0].ttl),act(h.mpls[add(0,0)].ttl)) in "act", there was some strange feedback

image The code is here: source_code.p4.txt test_1.p4.txt

test_2.p4.txt test_3.p4.txt

test_4.p4.txt test_5.p4.txt

my p4c version:p4c 1.2.4.9

Although it is strange to use "inout" here, I would like to know if there are any issues with such results,Thank you!

fruffy commented 7 months ago

Could you inline the outputs you have gotten from each program using the Github code formatting? It will make it easier to read the issue.

The error reports are working as intended. You are trying to set a runtime input to a parameter that is set by the control plane. That is not legal behavior according to the specification. https://p4.org/p4-spec/docs/P4-16-working-spec.html#sec-actions

A compiler bug is never okay and should be fixed.

liao123123123 commented 7 months ago

Could you inline the outputs you have gotten from each program using the Github code formatting? It will make it easier to read the issue.

Of course.I'm sorry for not expressing the problem clearly.

When i compile the source_code.p4:

source_code.p4(38): [--Wwarn=unused] warning: 'arg' is unused
    action act(bit<8> arg) {
                      ^^^

When i compile the test_1.p4 :

FrontEnd_11_(anonymous namespace)::SetStrictStruct
test_1.p4(45): [--Werror=type-error] error: act(): Parameter arg must be bound
        act;
        ^^^
test_1.p4(38)
    action act(inout bit<8> arg) {
                            ^^^

When i compile the test_2.p4:

test_2.p4(38): [--Wwarn=unused] warning: 'arg' is unused
    action act(inout bit<8> arg) {
                            ^^^

When i compile the test_3.p4:

test_3.p4(45): [--Werror=type-error] error: h.mpls[0].ttl: parameter arg cannot be bound: it is set by the control plane
        act(h.mpls[0].ttl);
            ^^^^^^^^^^^^^
test_3.p4(38)
    action act(bit<8> arg) {
                      ^^^

When i compile the test_4.p4:

PassRepeated_1_DiscoverFunctionsInlining
In file: /home/vagrant/new_p4c/p4c/frontends/p4/functionsInlining.cpp:109
Compiler Bug: test_4.p4(45): h.mpls[add(0, 0)].ttl;: expected a method call
        act(h.mpls[add(0,0)].ttl);
            ^^^^^^^^^^^^^^^^^^^^

When i compile the test_5.p4:

FrontEnd_11_(anonymous namespace)::SetStrictStruct
test_5.p4(45): [--Werror=type-error] error: h.mpls[add((bit<3>)0, (bit<3>)0)].ttl: parameter arg cannot be bound: it is set by the control plane
        act(h.mpls[add(0,0)].ttl);
            ^^^^^^^^^^^^^^^^^^^^
test_5.p4(38)
    action act( bit<8> arg) {
                       ^^^

and i read the p4 specification : hs[index]: produces a reference to the header at the specified position within the stack; if hs is an l-value, the result is also an l-value. The header may be invalid. Some implementations may impose the constraint that the index expression evaluates to a compile-time known value.

I would like to inquire if there are any issues with my understanding,When I use add [0,0] as the index.

fruffy commented 7 months ago

What would you like to achieve? Except test 4 all of these errors are correct. When you define an action list you can only list the actions but can not define input arguments. That is only possible for the default action or action entries.

liao123123123 commented 7 months ago

Thank you very much for your patient answer. This is my misunderstanding of my code, which I have understood now.

fruffy commented 7 months ago

For example, you should be able to add something like:

table tb {
    key = {
    }
    actions = {
        act;
    }
    default_action = act(h.mpls[add(0,0)].ttl);
}

and the compiler should throw no error. But at that point you might as well call the action directly.

vgurevich commented 7 months ago

@fruffy -- I am not sure this will work. Please, see #2501

fruffy commented 7 months ago

@fruffy -- I am not sure this will work. Please, see #2501

Right... this specific example requires a compile-time-known value.

action act(inout bit<8> arg) {
}

table tb {
    key = {
    }
    actions = {
        act(h.mpls[0].ttl);
    }
    default_action = act(h.mpls[0].ttl);
}

does compile at least. There seems to be some things broken here.

liao123123123 commented 7 months ago

I tried according to what you said.This seems to only bind the data parameters of act to h. mpls [0]. ttl (known at runtime)by defining an action list

    actions = {
        act(h.mpls[0].ttl);
    }

instead of default_action=act (h. mpls [0]. ttl);

fruffy commented 7 months ago

Yes, your code will always turn into h.mpls[0].ttl = arg; at the end.

liao123123123 commented 7 months ago

Thank you so much for the detailed and helpful explanation.