Fix FlattenLogMsg not properly working on nested structures (#5036)

* Fix FlattenLogMsg not properly working on nested structure argument Signed-off-by: Jiri Havranek <[email protected]> * Update reference output of issue2201-bmv2.p4 test file Signed-off-by: Jiri Havranek <[email protected]> * Fix structs with error type not being dumped by ToP4 Signed-off-by: Jiri Havranek <[email protected]> * Fix clang format complaints Signed-off-by: Jiri Havranek <[email protected]> * Remove v1model include; add log_msg function Signed-off-by: Jiri Havranek <[email protected]> * Rewrite ToP4 error type fix Signed-off-by: Jiri Havranek <[email protected]> * Rewrite ToP4 error type fix again Signed-off-by: Jiri Havranek <[email protected]> --------- Signed-off-by: Jiri Havranek <[email protected]>
p4lang · Dec 6, 2024 · cd56ab6 · cd56ab6
1 parent d9af61c
commit cd56ab6
Show file tree

Hide file tree

Showing 12 changed files with 244 additions and 22 deletions.
diff --git a/backends/p4test/midend.cpp b/backends/p4test/midend.cpp
@@ -43,6 +43,7 @@ limitations under the License.
 #include "midend/expandLookahead.h"
 #include "midend/flattenHeaders.h"
 #include "midend/flattenInterfaceStructs.h"
+#include "midend/flattenLogMsg.h"
 #include "midend/flattenUnions.h"
 #include "midend/global_copyprop.h"
 #include "midend/hsIndexSimplify.h"
@@ -101,6 +102,7 @@ MidEnd::MidEnd(CompilerOptions &options, std::ostream *outStream) {
          new P4::SimplifyParsers(),
          new P4::StrengthReduction(&typeMap),
          new P4::EliminateTuples(&typeMap),
+         new P4::FlattenLogMsg(&typeMap),
          new P4::SimplifyComparisons(&typeMap),
          new P4::CopyStructures(&typeMap, false),
          new P4::NestedStructs(&typeMap),

diff --git a/frontends/p4/toP4/toP4.cpp b/frontends/p4/toP4/toP4.cpp
@@ -513,6 +513,7 @@ bool ToP4::process(const IR::Type_StructLike *t, const char *name) {
     for (auto f : t->fields) {
         Util::SourceCodeBuilder builder;
         ToP4 rec(builder, showIR);
+        rec.isDeclaration = false;
 
         f->type->apply(rec);
         cstring t = builder.toString();
@@ -651,23 +652,31 @@ bool ToP4::preorder(const IR::Declaration_Variable *v) {
 
 bool ToP4::preorder(const IR::Type_Error *d) {
     dump(1);
-    bool first = true;
-    for (auto a : *d->getDeclarations()) {
-        if (ifSystemFile(a->getNode()).has_value())
-            // only print if not from a system file
-            continue;
-        if (!first) {
-            builder.append(",\n");
-        } else {
-            builder.append("error ");
-            builder.blockStart();
+
+    const auto userErrors = d->getDeclarations()
+                                ->where([this](const IR::IDeclaration *e) {
+                                    // only print if not from a system file
+                                    return !ifSystemFile(e->getNode()).has_value();
+                                })
+                                ->toVector();
+    if (!isDeclaration || !userErrors.empty()) {
+        builder.append("error");
+
+        if (!isDeclaration) {
+            return false;
+        }
+
+        bool first = true;
+        builder.blockStart();
+        for (auto a : userErrors) {
+            if (!first) {
+                builder.append(",\n");
+            }
+            dump(1, a->getNode(), 1);
+            first = false;
+            builder.emitIndent();
+            builder.append(a->getName());
         }
-        dump(1, a->getNode(), 1);
-        first = false;
-        builder.emitIndent();
-        builder.append(a->getName());
-    }
-    if (!first) {
         builder.newline();
         builder.blockEnd(true);
     }

diff --git a/midend/flattenLogMsg.cpp b/midend/flattenLogMsg.cpp
@@ -99,6 +99,7 @@ TypeLogMsgParams FindTypesInLogMsgInvocationToReplace::unfoldStruct(const IR::Ex
             for (auto field : structType->fields) {
                 std::string nm = field->name.name + std::string(":");
                 auto *newMember = new IR::Member(field->type, expr, field->name);
+                typeMap->setType(newMember, field->type);
                 if (field->type->is<IR::Type_StructLike>()) {
                     auto curResult = unfoldStruct(newMember, strParam, field->name.name.c_str());
                     nm += curResult.second;

diff --git a/testdata/p4_16_samples/issue5035.p4 b/testdata/p4_16_samples/issue5035.p4
@@ -0,0 +1,48 @@
+/*
+Copyright 2024 Intel Corporation
+
+Licensed under the Apache License, Version 2.0 (the "License");
+you may not use this file except in compliance with the License.
+You may obtain a copy of the License at
+
+    http://www.apache.org/licenses/LICENSE-2.0
+
+Unless required by applicable law or agreed to in writing, software
+distributed under the License is distributed on an "AS IS" BASIS,
+WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+See the License for the specific language governing permissions and
+limitations under the License.
+*/
+
+extern void log_msg(string msg);
+extern void log_msg<T>(string msg, in T data);
+
+header h0_t {
+    bit<32> f0;
+    bit<32> f1;
+}
+
+struct struct_data_t {
+    bit<3> f0;
+    bit<5> f1;
+    bit<8> f2;
+}
+
+struct data_t {
+    h0_t hdr;
+    struct_data_t data;
+    bit<32> field;
+}
+
+parser SimpleParser(inout data_t d);
+package SimpleArch(SimpleParser p);
+
+parser ParserImpl(inout data_t d)
+{
+    state start {
+        log_msg("Flattened hierarchical data: {}", {d});
+        transition accept;
+    }
+}
+
+SimpleArch(ParserImpl()) main;
diff --git a/testdata/p4_16_samples_outputs/issue2201-bmv2-midend.p4 b/testdata/p4_16_samples_outputs/issue2201-bmv2-midend.p4
@@ -85,15 +85,43 @@ struct tuple_6 {
 }
 
 struct tuple_7 {
-    ethernet_t f0;
+    bit<48> f0;
+    bit<48> f1;
+    bit<16> f2;
 }
 
 struct tuple_8 {
-    ipv4_t f0;
+    bit<4>  f0;
+    bit<4>  f1;
+    bit<8>  f2;
+    bit<16> f3;
+    bit<16> f4;
+    bit<3>  f5;
+    bit<13> f6;
+    bit<8>  f7;
+    bit<8>  f8;
+    bit<16> f9;
+    bit<32> f10;
+    bit<32> f11;
 }
 
 struct tuple_9 {
-    standard_metadata_t f0;
+    bit<9>  f0;
+    bit<9>  f1;
+    bit<9>  f2;
+    bit<32> f3;
+    bit<32> f4;
+    bit<32> f5;
+    bit<19> f6;
+    bit<32> f7;
+    bit<19> f8;
+    bit<48> f9;
+    bit<48> f10;
+    bit<16> f11;
+    bit<16> f12;
+    bit<1>  f13;
+    error   f14;
+    bit<3>  f15;
 }
 
 struct tuple_10 {
@@ -137,10 +165,10 @@ control ingressImpl(inout headers_t hdr, inout metadata_t meta, inout standard_m
     @hidden action issue2201bmv2l164() {
         log_msg<tuple_6>("GREPME serenum1={}", (tuple_6){f0 = serenum1_0});
         log_msg<tuple_0>("GREPME hdr.ethernet.isValid()={}", (tuple_0){f0 = hdr.ethernet.isValid()});
-        log_msg<tuple_7>("GREPME hdr.ethernet={}", (tuple_7){f0 = hdr.ethernet});
+        log_msg<tuple_7>("GREPME hdr.ethernet=(dstAddr:{},srcAddr:{},etherType:{})", (tuple_7){f0 = hdr.ethernet.dstAddr,f1 = hdr.ethernet.srcAddr,f2 = hdr.ethernet.etherType});
         log_msg<tuple_0>("GREPME hdr.ipv4.isValid()={}", (tuple_0){f0 = hdr.ipv4.isValid()});
-        log_msg<tuple_8>("GREPME hdr.ipv4={}", (tuple_8){f0 = hdr.ipv4});
-        log_msg<tuple_9>("GREPME stdmeta={}", (tuple_9){f0 = stdmeta});
+        log_msg<tuple_8>("GREPME hdr.ipv4=(version:{},ihl:{},diffserv:{},totalLen:{},identification:{},flags:{},fragOffset:{},ttl:{},protocol:{},hdrChecksum:{},srcAddr:{},dstAddr:{})", (tuple_8){f0 = hdr.ipv4.version,f1 = hdr.ipv4.ihl,f2 = hdr.ipv4.diffserv,f3 = hdr.ipv4.totalLen,f4 = hdr.ipv4.identification,f5 = hdr.ipv4.flags,f6 = hdr.ipv4.fragOffset,f7 = hdr.ipv4.ttl,f8 = hdr.ipv4.protocol,f9 = hdr.ipv4.hdrChecksum,f10 = hdr.ipv4.srcAddr,f11 = hdr.ipv4.dstAddr});
+        log_msg<tuple_9>("GREPME stdmeta=(ingress_port:{},egress_spec:{},egress_port:{},instance_type:{},packet_length:{},enq_timestamp:{},enq_qdepth:{},deq_timedelta:{},deq_qdepth:{},ingress_global_timestamp:{},egress_global_timestamp:{},mcast_grp:{},egress_rid:{},checksum_error:{},parser_error:{},priority:{})", (tuple_9){f0 = stdmeta.ingress_port,f1 = stdmeta.egress_spec,f2 = stdmeta.egress_port,f3 = stdmeta.instance_type,f4 = stdmeta.packet_length,f5 = stdmeta.enq_timestamp,f6 = stdmeta.enq_qdepth,f7 = stdmeta.deq_timedelta,f8 = stdmeta.deq_qdepth,f9 = stdmeta.ingress_global_timestamp,f10 = stdmeta.egress_global_timestamp,f11 = stdmeta.mcast_grp,f12 = stdmeta.egress_rid,f13 = stdmeta.checksum_error,f14 = stdmeta.parser_error,f15 = stdmeta.priority});
         log_msg<tuple_10>("GREPME error.PacketTooShort={}", (tuple_10){f0 = error.PacketTooShort});
     }
     @hidden table tbl_issue2201bmv2l101 {

diff --git a/testdata/p4_16_samples_outputs/issue5035-first.p4 b/testdata/p4_16_samples_outputs/issue5035-first.p4
@@ -0,0 +1,29 @@
+extern void log_msg(string msg);
+extern void log_msg<T>(string msg, in T data);
+header h0_t {
+    bit<32> f0;
+    bit<32> f1;
+}
+
+struct struct_data_t {
+    bit<3> f0;
+    bit<5> f1;
+    bit<8> f2;
+}
+
+struct data_t {
+    h0_t          hdr;
+    struct_data_t data;
+    bit<32>       field;
+}
+
+parser SimpleParser(inout data_t d);
+package SimpleArch(SimpleParser p);
+parser ParserImpl(inout data_t d) {
+    state start {
+        log_msg<tuple<data_t>>("Flattened hierarchical data: {}", { d });
+        transition accept;
+    }
+}
+
+SimpleArch(ParserImpl()) main;
diff --git a/testdata/p4_16_samples_outputs/issue5035-frontend.p4 b/testdata/p4_16_samples_outputs/issue5035-frontend.p4
@@ -0,0 +1,29 @@
+extern void log_msg(string msg);
+extern void log_msg<T>(string msg, in T data);
+header h0_t {
+    bit<32> f0;
+    bit<32> f1;
+}
+
+struct struct_data_t {
+    bit<3> f0;
+    bit<5> f1;
+    bit<8> f2;
+}
+
+struct data_t {
+    h0_t          hdr;
+    struct_data_t data;
+    bit<32>       field;
+}
+
+parser SimpleParser(inout data_t d);
+package SimpleArch(SimpleParser p);
+parser ParserImpl(inout data_t d) {
+    state start {
+        log_msg<tuple<data_t>>("Flattened hierarchical data: {}", { d });
+        transition accept;
+    }
+}
+
+SimpleArch(ParserImpl()) main;
diff --git a/testdata/p4_16_samples_outputs/issue5035-midend.p4 b/testdata/p4_16_samples_outputs/issue5035-midend.p4
@@ -0,0 +1,38 @@
+extern void log_msg(string msg);
+extern void log_msg<T>(string msg, in T data);
+header h0_t {
+    bit<32> f0;
+    bit<32> f1;
+}
+
+struct struct_data_t {
+    bit<3> f0;
+    bit<5> f1;
+    bit<8> f2;
+}
+
+struct data_t {
+    h0_t          hdr;
+    struct_data_t data;
+    bit<32>       field;
+}
+
+parser SimpleParser(inout data_t d);
+package SimpleArch(SimpleParser p);
+struct tuple_0 {
+    bit<32> f0;
+    bit<32> f1;
+    bit<3>  f2;
+    bit<5>  f3;
+    bit<8>  f4;
+    bit<32> f5;
+}
+
+parser ParserImpl(inout data_t d) {
+    state start {
+        log_msg<tuple_0>("Flattened hierarchical data: (hdr:(f0:{},f1:{}),data:(f0:{},f1:{},f2:{}),field:{})", (tuple_0){f0 = d.hdr.f0,f1 = d.hdr.f1,f2 = d.data.f0,f3 = d.data.f1,f4 = d.data.f2,f5 = d.field});
+        transition accept;
+    }
+}
+
+SimpleArch(ParserImpl()) main;
diff --git a/testdata/p4_16_samples_outputs/issue5035.p4 b/testdata/p4_16_samples_outputs/issue5035.p4
@@ -0,0 +1,29 @@
+extern void log_msg(string msg);
+extern void log_msg<T>(string msg, in T data);
+header h0_t {
+    bit<32> f0;
+    bit<32> f1;
+}
+
+struct struct_data_t {
+    bit<3> f0;
+    bit<5> f1;
+    bit<8> f2;
+}
+
+struct data_t {
+    h0_t          hdr;
+    struct_data_t data;
+    bit<32>       field;
+}
+
+parser SimpleParser(inout data_t d);
+package SimpleArch(SimpleParser p);
+parser ParserImpl(inout data_t d) {
+    state start {
+        log_msg("Flattened hierarchical data: {}", { d });
+        transition accept;
+    }
+}
+
+SimpleArch(ParserImpl()) main;
diff --git a/testdata/p4_16_samples_outputs/issue5035.p4-stderr b/testdata/p4_16_samples_outputs/issue5035.p4-stderr
diff --git a/testdata/p4_16_samples_outputs/issue5035.p4.entries.txtpb b/testdata/p4_16_samples_outputs/issue5035.p4.entries.txtpb
@@ -0,0 +1,3 @@
+# proto-file: p4/v1/p4runtime.proto
+# proto-message: p4.v1.WriteRequest
+
diff --git a/testdata/p4_16_samples_outputs/issue5035.p4.p4info.txtpb b/testdata/p4_16_samples_outputs/issue5035.p4.p4info.txtpb
@@ -0,0 +1,6 @@
+# proto-file: p4/config/v1/p4info.proto
+# proto-message: p4.config.v1.P4Info
+
+pkg_info {
+  arch: "v1model"
+}
Original file line number	Diff line number	Diff line change
		@@ -0,0 +1,3 @@
		# proto-file: p4/v1/p4runtime.proto
		# proto-message: p4.v1.WriteRequest