Skip to content

Commit

Permalink
Enhancement: Issue p4lang#2396 duplicated setValid() call removal
Browse files Browse the repository at this point in the history 
Frontend pass setHeaders shifted after InlineFunctions pass, so that return values of functions could be processed as right operands of assignment statements, where left operand is header type or a header field. The idea was to remove the redundant setValid() calls in some outputs. The solution was tested on code given in issue 2383, and tests that had a duplicated setValid() call.

With this change, there is no need for checking for header types when handling structures or lists in copyStructures pass in midend, because setValid() call is handled in frontend. There are no changes at the end of midend in test p4lang#2383 after implementing these changes, and tests with redundant setValid() calls have different outputs. For an example, in test issue2261.p4, there is no redundant setValid() call after midend anymore:

    @hidden action issue2261l22() {
        tmp_struct_0_eth_hdr.setValid();
        tmp_struct_0_eth_hdr.dst_addr = 48w0;
        tmp_struct_0_eth_hdr.src_addr = 48w0;
        tmp_struct_0_eth_hdr.eth_type = 16w0;
        h.eth_hdr.eth_type = 16w0;
    }
  • Loading branch information
@anasyrmia
anasyrmia committed Aug 18, 2020
1 parent f8d2bc1 commit 2adb674
Show file tree
Hide file tree
Showing 12 changed files with 2 additions and 76 deletions.
2 changes: 1 addition & 1 deletion frontends/p4/frontend.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -173,7 +173,6 @@ const IR::P4Program *FrontEnd::run(const CompilerOptions &options, const IR::P4P
new MoveDeclarations(), // Move all local declarations to the beginning
new MoveInitializers(&refMap),
new SideEffectOrdering(&refMap, &typeMap, skipSideEffectOrdering),
new SetHeaders(&refMap, &typeMap),
new SimplifyControlFlow(&refMap, &typeMap),
new MoveDeclarations(), // Move all local declarations to the beginning
new SimplifyDefUse(&refMap, &typeMap),
Expand All @@ -190,6 +189,7 @@ const IR::P4Program *FrontEnd::run(const CompilerOptions &options, const IR::P4P
new Inline(&refMap, &typeMap, evaluator),
new InlineActions(&refMap, &typeMap),
new InlineFunctions(&refMap, &typeMap),
new SetHeaders(&refMap, &typeMap),
// Check for constants only after inlining
new CheckConstants(&refMap, &typeMap),
new SimplifyControlFlow(&refMap, &typeMap),
Expand Down
14 changes: 0 additions & 14 deletions midend/copyStructures.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -71,13 +71,6 @@ const IR::Node* DoCopyStructures::postorder(IR::AssignmentStatement* statement)
auto retval = new IR::IndexedVector<IR::StatOrDecl>();
auto strct = ltype->to<IR::Type_StructLike>();
if (auto list = statement->right->to<IR::ListExpression>()) {
if (ltype->is<IR::Type_Header>()) {
auto setValid = new IR::Member(
statement->srcInfo, statement->left, IR::Type_Header::setValid);
auto mc = new IR::MethodCallStatement(
new IR::MethodCallExpression(statement->srcInfo, setValid));
retval->push_back(mc);
}
unsigned index = 0;
for (auto f : strct->fields) {
auto right = list->components.at(index);
Expand All @@ -86,13 +79,6 @@ const IR::Node* DoCopyStructures::postorder(IR::AssignmentStatement* statement)
index++;
}
} else if (auto si = statement->right->to<IR::StructExpression>()) {
if (ltype->is<IR::Type_Header>()) {
auto setValid = new IR::Member(
statement->srcInfo, statement->left, IR::Type_Header::setValid);
auto mc = new IR::MethodCallStatement(
new IR::MethodCallExpression(statement->srcInfo, setValid));
retval->push_back(mc);
}
for (auto f : strct->fields) {
auto right = si->components.getDeclaration<IR::NamedExpression>(f->name);
auto left = new IR::Member(statement->left, f->name);
Expand Down
1 change: 0 additions & 1 deletion testdata/p4_16_samples_outputs/issue2261-midend.p4
View file
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,6 @@ parser p(packet_in pkt, out Headers hdr, inout Meta m, inout standard_metadata_t
control ingress(inout Headers h, inout Meta m, inout standard_metadata_t sm) {
ethernet_t tmp_struct_0_eth_hdr;
@hidden action issue2261l22() {
tmp_struct_0_eth_hdr.setValid();
tmp_struct_0_eth_hdr.setValid();
tmp_struct_0_eth_hdr.dst_addr = 48w0;
tmp_struct_0_eth_hdr.src_addr = 48w0;
Expand Down
6 changes: 0 additions & 6 deletions testdata/p4_16_samples_outputs/issue2289-frontend.p4
View file
Original file line number Diff line number Diff line change
Expand Up @@ -37,8 +37,6 @@ control ingress(inout Headers h, inout Meta m, inout standard_metadata_t sm) {
{
bool hasReturned = false;
bit<16> retval;
nested_struct tmp_struct_0;
tmp_struct_0.s.setValid();
hasReturned = true;
retval = 16w1;
tmp = retval;
Expand All @@ -51,8 +49,6 @@ control ingress(inout Headers h, inout Meta m, inout standard_metadata_t sm) {
{
bool hasReturned_3 = false;
bit<16> retval_3;
nested_struct tmp_struct_1;
tmp_struct_1.s.setValid();
hasReturned_3 = true;
retval_3 = 16w1;
tmp_1 = retval_3;
Expand All @@ -65,8 +61,6 @@ control ingress(inout Headers h, inout Meta m, inout standard_metadata_t sm) {
{
bool hasReturned_4 = false;
bit<16> retval_4;
nested_struct tmp_struct_2;
tmp_struct_2.s.setValid();
hasReturned_4 = true;
retval_4 = 16w1;
}
Expand Down
15 changes: 0 additions & 15 deletions testdata/p4_16_samples_outputs/issue2289-midend.p4
View file
Original file line number Diff line number Diff line change
Expand Up @@ -31,32 +31,17 @@ parser p(packet_in pkt, out Headers hdr, inout Meta m, inout standard_metadata_t
}

control ingress(inout Headers h, inout Meta m, inout standard_metadata_t sm) {
simple_struct tmp_struct_0_s;
simple_struct tmp_struct_1_s;
simple_struct tmp_struct_2_s;
bit<16> byaA;
@name("ingress.simple_action") action simple_action() {
tmp_struct_0_s.setValid();
tmp_struct_1_s.setValid();
h.eth_hdr.eth_type = byaA;
}
@hidden action issue2289l26() {
tmp_struct_2_s.setValid();
}
@hidden table tbl_issue2289l26 {
actions = {
issue2289l26();
}
const default_action = issue2289l26();
}
@hidden table tbl_simple_action {
actions = {
simple_action();
}
const default_action = simple_action();
}
apply {
tbl_issue2289l26.apply();
tbl_simple_action.apply();
}
}
Expand Down
1 change: 1 addition & 0 deletions testdata/p4_16_samples_outputs/issue2383-bmv2-frontend.p4
View file
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@ control ingress(inout Headers h, inout Meta m, inout standard_metadata_t sm) {
bool hasReturned = false;
ethernet_t retval;
hasReturned = true;
retval.setValid();
retval = { 48w1, 48w1, 16w1 };
h.eth_hdr = retval;
}
Expand Down
3 changes: 0 additions & 3 deletions testdata/p4_16_samples_outputs/issue396-frontend.p4
View file
Original file line number Diff line number Diff line change
Expand Up @@ -11,17 +11,14 @@ package top(c _c);
control d(out bool b) {
H h_0;
H[2] h3_0;
S s_0;
S s1_0;
bool eout_0;
H tmp;
apply {
h_0.setValid();
h_0 = (H){x = 32w0};
s_0.h.setValid();
s1_0.h.setValid();
s1_0.h = (H){x = 32w0};
h3_0[0].setValid();
h3_0[1].setValid();
h3_0[1] = (H){x = 32w1};
tmp.setValid();
Expand Down
7 changes: 0 additions & 7 deletions testdata/p4_16_samples_outputs/issue396-midend.p4
View file
Original file line number Diff line number Diff line change
Expand Up @@ -11,24 +11,17 @@ package top(c _c);
control d(out bool b) {
H h_0;
H[2] h3_0;
H s_0_h;
H s1_0_h;
bool eout_0;
H tmp;
@hidden action issue396l28() {
h_0.setValid();
h_0.setValid();
h_0.x = 32w0;
s_0_h.setValid();
s1_0_h.setValid();
s1_0_h.setValid();
s1_0_h.x = 32w0;
h3_0[0].setValid();
h3_0[1].setValid();
h3_0[1].setValid();
h3_0[1].x = 32w1;
tmp.setValid();
tmp.setValid();
tmp.x = 32w0;
eout_0 = tmp.isValid();
b = h_0.isValid() && eout_0 && h3_0[1].isValid() && s1_0_h.isValid();
Expand Down
1 change: 0 additions & 1 deletion testdata/p4_16_samples_outputs/issue841-midend.p4
View file
Original file line number Diff line number Diff line change
Expand Up @@ -46,7 +46,6 @@ control MyComputeChecksum(inout headers hdr, inout metadata meta) {
h_t h_0;
@name("MyComputeChecksum.checksum") Checksum16() checksum_0;
apply {
h_0.setValid();
h_0.setValid();
h_0.src = hdr.h.src;
h_0.dst = hdr.h.dst;
Expand Down
1 change: 0 additions & 1 deletion testdata/p4_16_samples_outputs/issue982-midend.p4
View file
Original file line number Diff line number Diff line change
Expand Up @@ -386,7 +386,6 @@ control IngressDeparserImpl(packet_out packet, inout headers hdr, in metadata me
ostd.clone_metadata.data.h1 = clone_md_0_data.h1;
}
@hidden action issue982l417() {
clone_md_0_data.h1.setValid();
clone_md_0_data.h1.setValid();
clone_md_0_data.h1.data = 32w0;
}
Expand Down
4 changes: 0 additions & 4 deletions testdata/p4_16_samples_outputs/struct_init-1-frontend.p4
View file
Original file line number Diff line number Diff line change
Expand Up @@ -13,14 +13,10 @@ struct metadata_t {
control I(inout metadata_t meta) {
H h_0;
apply {
h_0.setValid();
if (meta.foo == (PortId_t){_v = 9w192}) {
meta.foo._v = meta.foo._v + 9w1;
h_0.setValid();
h_0 = (H){b = 32w2};
if (h_0 == (H){b = 32w1}) {
h_0.setValid();
}
}
}
}
Expand Down
23 changes: 0 additions & 23 deletions testdata/p4_16_samples_outputs/struct_init-1-midend.p4
View file
Original file line number Diff line number Diff line change
Expand Up @@ -12,43 +12,20 @@ struct metadata_t {

control I(inout metadata_t meta) {
H h_0;
@hidden action struct_init1l18() {
h_0.setValid();
}
@hidden action struct_init1l15() {
meta._foo__v0 = meta._foo__v0 + 9w1;
h_0.setValid();
h_0.setValid();
h_0.b = 32w2;
}
@hidden action struct_init1l13() {
h_0.setValid();
}
@hidden table tbl_struct_init1l13 {
actions = {
struct_init1l13();
}
const default_action = struct_init1l13();
}
@hidden table tbl_struct_init1l15 {
actions = {
struct_init1l15();
}
const default_action = struct_init1l15();
}
@hidden table tbl_struct_init1l18 {
actions = {
struct_init1l18();
}
const default_action = struct_init1l18();
}
apply {
tbl_struct_init1l13.apply();
if (meta._foo__v0 == 9w192) {
tbl_struct_init1l15.apply();
if (!h_0.isValid() && false || h_0.isValid() && true && false) {
tbl_struct_init1l18.apply();
}
}
}
}
Expand Down

0 comments on commit 2adb674

Please sign in to comment.