p4lang / p4c

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

Why is the local copy propagation pass not transforming my program? #1841

Open antoninbas opened 5 years ago

antoninbas commented 5 years ago

This is a follow-up question on the LDWG discussion we had on April 1st.

I have the following program:

#include <v1model.p4>

header Hdr {
    bit<32> fA;
    bit<16> fB;
    bit<16> fC;
    bit<16> fD;
    bit<16> fE;
    bit<32> fF;
    bit<8>  fG;
}
struct H {
    Hdr h;
};
struct M { }

struct FieldList1_t {
  bit<8> a;
  bit<16> b;
}

struct FieldList2_t {
  bit<16> a;
  bit<16> b;
  bit<32> c;
}

struct FieldLists_t {
  FieldList1_t fl1;
  FieldList2_t fl2;
}

extern Hash<T> {
  Hash();
  T get<I>(in I input);
}

parser ParserI(packet_in pk, out H hdr, inout M meta, inout standard_metadata_t smeta) {
    state start { pk.extract(hdr.h); transition accept; }
}

action empty() { }

control IngressI(inout H hdr, inout M meta, inout standard_metadata_t smeta) {
    Hash<bit<16> /* output */>() hash1;
    apply {
        FieldList1_t fl1 = {hdr.h.fA[31:24], hdr.h.fB};
        FieldList2_t fl2 = {hdr.h.fB, hdr.h.fC, hdr.h.fA};
        bit<16> output = hash1.get<FieldLists_t>({fl1, fl2});

        smeta.egress_spec = (bit<9>) output;
    }
};

control EgressI(inout H hdr, inout M meta, inout standard_metadata_t smeta) {
    apply { }
};

control DeparserI(packet_out pk, in H hdr) {
    apply { pk.emit(hdr); }
}

control VerifyChecksumI(inout H hdr, inout M meta) {
    apply { }
}

control ComputeChecksumI(inout H hdr, inout M meta) {
    apply { }
}

V1Switch(ParserI(), VerifyChecksumI(), IngressI(), EgressI(),
         ComputeChecksumI(), DeparserI()) main;

What is preventing the local copy propagation pass from replacing this code:

FieldList1_t fl1 = {hdr.h.fA[31:24], hdr.h.fB};
FieldList2_t fl2 = {hdr.h.fB, hdr.h.fC, hdr.h.fA};
bit<16> output = hash1.get<FieldLists_t>({fl1, fl2});

with:

bit<16> output = hash1.get<FieldLists_t>(
    { {hdr.h.fA[31:24], hdr.h.fB}, {hdr.h.fB, hdr.h.fC, hdr.h.fA} });
ChrisDodd commented 5 years ago

Localcopyprop currently does not copyprop list expressions (deliberately). The code from localcopyprop.cpp:323

        if (as->right->is<IR::ListExpression>()) {
            /* FIXME -- List Expressions need to be turned into constructor calls before
             * we can copyprop them */
            return as; }

That's because list expressions are only valid in some contexts, so we can't generally copyprop them.

The code could be rearranged to allow list expression copyprops and only copyprop list expressions into contexts where they are supported. So remove these 4 lines and instead have copyprop_name (the method that figures out if there's a value to copyprop to replace a use of a name) check the context when trying to copyprop a list expression.