compiler transformations don't preserve names

[smolkaj]

(For future reference @jnfoster @cc10512 @sethfowler @cole-barefoot)
Certain compiler transformations do not preserve names. As a result, the auto-generated APIs (pd/thrift/...) for a p4 program may refer to names that appear nowhere in the p4 source code.

I'm attaching a minimal example for future reference. Here is the core of the issue:

control nested(inout H hdr, inout M meta) {

    table t {
        key = { hdr.ethernet.isValid() : exact; }
        actions = { NoAction; }
        default_action = NoAction;
    }

    apply { 
        t.apply();
    }
}

control IngressI(inout H hdr, inout M meta, inout std_meta_t std_meta) {
    nested() nested_instance;
    apply {
        nested_instance.apply(hdr, meta);
    }
}

The ingress control block invokes a second control block nested that reads from the ethernet header. The compiler implements this by creating a temporary header tmp_0_ethernet, copying ethernet to tmp_0_ethernet, invoking nested with tmp_0_ethernet, and finally copying tmp_0_ethernet back to ethernet (copy-in/copy-out). The relevant bits from the generated .json file are these:

"headers" : [
    {
      "name" : "ethernet",
      "id" : 0,
      "header_type" : "ethernet_t",
      "metadata" : false
    },
    {
      "name" : "tmp_0_ethernet",
      "id" : 1,
      "header_type" : "ethernet_t",
      "metadata" : true,
      "pi_omit" : true
    },
    ...
]
...
"tables" : [
{
          "name" : "nested_instance.t",
          "id" : 1,
          "key" : [
            {
              "match_type" : "valid",
              "target" : "tmp_0_ethernet",
              "mask" : null
            }
          ],
          ....

The important thing to notice is that the table t matches on the header tmp_0_ethernet, instead of ethernet as specified the P4 source code. As a result, the API generated for table t use the name tmp_0_ethernet. The relevant bit from pd/thrift/p4_pd_rpc.thrift is this:

struct dc_nested_instance_t_match_spec_t {
  1: required byte tmp_0_ethernet_valid;
}

[mihaibudiu]

This is a very good point. We can fix this issue by reducing it to #219, by generating @name annotations.
However, this shows that we actually need some support from BMv2, because in P4-16 you can have multiple variables with the same name, and when we generate BMv2 code we have to give them different names.

So we have to work with @antoninbas to add support to the BMv2 JSON to give us the ability to use different names for table key fields than the program expression. Names must be unique for each table, they do not have to be globally unique.

@sethfowler: this is also related to the PI generation.

[sethfowler]

Yes, the same problem exists in PI.

[mihaibudiu]

I wonder whether this issue is solved.

[sethfowler]

It's not clear. At a minimum, I think we need to write some tests for this.

[mihaibudiu]

I will also mark this as fixed.

[mihaibudiu]

Since no one complained and we have more tests for p4runtime I assume this can be closed.