p4lang / p4c

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

Compiler Bug: multiple table applications in one expression #4596

Open liao123123123 opened 6 months ago

liao123123123 commented 6 months ago

The command "p4test test.p4 -v" is used to compile the following code:

header ethernet_t {
    bit<48> dstAddr;
    bit<48> srcAddr;
}

bit<32> fun(in ethernet_t var){
    return 0;
}

control ingress(inout Headers h, inout Meta m, inout standard_metadata_t sm) {
    table tb1 {
        key = {
        }
        actions = {
        }
    }

    table tb2 {
        key = {
        }
        actions = {
        }
    }
    table tb {
        key = {
            fun(
            {dstAddr = (tb1.apply().hit ? 0 : 48w1),
            srcAddr = ( tb2.apply().hit  ? 0: 48w1)}
            ): exact @name("abc");
        }
        actions = {
        }
    }
    apply {
        tb.apply();
    }
}

result in :

In file: /home/vagrant/new_p4c/p4c/frontends/p4/sideEffects.h:49
Compiler Bug: bug_3.p4(36): table tb1_0/tb1 @name("tb1") {
  actions = { @defaultonly NoAction(); }
  default_action = NoAction(); } and table tb2_0/tb2 @name("tb2") {
  actions = { @defaultonly NoAction(); }
  default_action = NoAction(); }: multiple table applications in one expression
    table tb1 {
          ^^^
bug_3.p4(43)
    table tb2 {
          ^^^

However, in my example, such a key expression seems meaningless.

jafingerhut commented 6 months ago

Agreed that there should be no CompilerBug appearing, ideally.

Personally, I would not mind if open source p4c simply rejected such keys as unsupported, i.e. do not support key expressions that cause other tables to be invoked.

I also would not mind if this was supported, but if so, it would probably most reasonably be done as other "complex" key expressions are supported, i.e. by calculating the expression before the table is applied, and storing it in a temporary variable.

liao123123123 commented 6 months ago

Thank you for your answer. Yes, we can avoid it by using a temporary variable in most cases:

    bit<32> temp=fun(
    {dstAddr = (tb1.apply().hit ? 0 : 48w1),
    srcAddr = ( tb2.apply().hit  ? 0: 48w1)}
    );

    table tb {
        key = {
            temp: exact @name("abc");
        }
        actions = {
        }
    }