loop fixes to get through midend/p4test

- allow ForIn to refer to decl in the enclosing scope - ensure valid P4 code output for loops in toP4 - fix testcase to no deadcode elim everything
p4lang · Mar 27, 2024 · 4c1b7c7 · 4c1b7c7
1 parent 21088d8
commit 4c1b7c7
Show file tree

Hide file tree

Showing 11 changed files with 121 additions and 14 deletions.
diff --git a/frontends/p4/resetHeaders.cpp b/frontends/p4/resetHeaders.cpp
@@ -53,9 +53,11 @@ void DoResetHeaders::generateResets(const TypeMap *typeMap, const IR::Type *type
 const IR::Node *DoResetHeaders::postorder(IR::Declaration_Variable *decl) {
     if (decl->initializer != nullptr) return decl;
     LOG3("DoResetHeaders context " << dbp(getContext()->node));
+    auto parent = getContext()->node;
+    // Don't reset index var in for..in statements.
+    if (parent->is<IR::ForInStatement>()) return decl;
     auto type = typeMap->getType(getOriginal(), true);
     auto path = new IR::PathExpression(decl->getName());
-    auto parent = getContext()->node;
     // For declarations in parsers and controls we have to insert the
     // reset in the start state or the body respectively.
     bool separate = parent->is<IR::P4Parser>() || parent->is<IR::P4Control>();

diff --git a/frontends/p4/toP4/toP4.cpp b/frontends/p4/toP4/toP4.cpp
@@ -1206,6 +1206,7 @@ bool ToP4::preorder(const IR::ForStatement *s) {
     builder.append("; ");
     first = true;
     for (auto *e : s->updates) {
+        if (e->is<IR::EmptyStatement>()) continue;
         if (!first) builder.append(", ");
         builder.supressStatementSemi();
         visit(e, "updates");
@@ -1230,8 +1231,12 @@ bool ToP4::preorder(const IR::ForStatement *s) {
 bool ToP4::preorder(const IR::ForInStatement *s) {
     dump(2);
     builder.append("for (");
-    builder.supressStatementSemi();
-    visit(s->decl, "decl");
+    if (s->decl) {
+        builder.supressStatementSemi();
+        visit(s->decl, "decl");
+    } else {
+        visit(s->ref, "ref");
+    }
     builder.append(" in ");
     visit(s->collection);
     builder.append(") ");

diff --git a/frontends/parsers/p4/p4parser.ypp b/frontends/parsers/p4/p4parser.ypp
@@ -1311,9 +1311,10 @@ forStatement
 	      forUpdateStatements ")"
       statement
     { $$ = new IR::ForStatement(@1+@8, *$3, $5, *$7, $9); }
-    | FOR "(" typeRef name IN forCollectionExpr ")"
-      statement
+    | FOR "(" typeRef name IN forCollectionExpr ")" statement
     { $$ = new IR::ForInStatement(@1+@6, new IR::Declaration_Variable(@3+@4, *$4, $3), $6, $8); }
+    | FOR "(" prefixedNonTypeName IN forCollectionExpr ")" statement
+    { $$ = new IR::ForInStatement(@1+@6, new IR::PathExpression(@3, $3), $5, $7); }
     ;
 
 forInitStatements

diff --git a/ir/dbprint-stmt.cpp b/ir/dbprint-stmt.cpp
@@ -99,11 +99,17 @@ void IR::ForStatement::dbprint(std::ostream &out) const {
         if (!first) out << ", ";
         out << sd;
         first = false; }
-    out << "{" << indent << Log::endl << body << " }" << unindent << setprec(prec);
+    out << ") {" << indent << Log::endl << body << " }" << unindent << setprec(prec);
 }
 
 void IR::ForInStatement::dbprint(std::ostream &out) const {
     int prec = getprec(out);
-    out << Prec_Low << "for (" << decl << " in " << collection << ") {" << indent
-        << Log::endl << body << " }" << unindent << setprec(prec);
+    out << Prec_Low << "for (";
+    if (decl) {
+        out << decl;
+    } else {
+        out << ref;
+    }
+    out << " in " << collection << ") {" << indent << Log::endl
+        << body << " }" << unindent << setprec(prec);
 }
diff --git a/ir/ir.def b/ir/ir.def
@@ -536,10 +536,13 @@ class ForInStatement : Statement, ISimpleNamespace {
     Expression                          collection;
     Statement                           body;
     ForInStatement { ref = new PathExpression(decl->name); }
+    ForInStatement(Util::SourceInfo si, PathExpression pe, Expression c, Statement b)
+        : Statement(si), decl(nullptr), ref(pe), collection(c), body(b) {}
     IDeclaration getDeclByName(cstring name) const override {
-        return decl->name == name ? decl : nullptr; }
+        return decl && decl->name == name ? decl : nullptr; }
     Util::Enumerator<IDeclaration>* getDeclarations() const override {
-        return new Util::SingleEnumerator<const IDeclaration *>(decl); }
+        if (decl) return new Util::SingleEnumerator<const IDeclaration *>(decl);
+        return new Util::EmptyEnumerator<const IDeclaration *>(); }
 }
 
 /////////////////////////////////////////////////////////////

diff --git a/testdata/p4_16_samples/forloop1.p4 b/testdata/p4_16_samples/forloop1.p4
@@ -1,3 +1,6 @@
+#include <core.p4>
+control generic<M>(inout M m);
+package top<M>(generic<M> c);
 
 header t1 {
     bit<32>     f1;
@@ -25,3 +28,5 @@ control c(inout headers_t hdrs) {
         }
     }
 }
+
+top(c()) main;
diff --git a/testdata/p4_16_samples_outputs/forloop1-first.p4 b/testdata/p4_16_samples_outputs/forloop1-first.p4
@@ -1,3 +1,7 @@
+#include <core.p4>
+
+control generic<M>(inout M m);
+package top<M>(generic<M> c);
 header t1 {
     bit<32> f1;
     bit<16> h1;
@@ -25,3 +29,4 @@ control c(inout headers_t hdrs) {
     }
 }
 
+top<headers_t>(c()) main;
diff --git a/testdata/p4_16_samples_outputs/forloop1-frontend.p4 b/testdata/p4_16_samples_outputs/forloop1-frontend.p4
@@ -1,3 +1,7 @@
+#include <core.p4>
+
+control generic<M>(inout M m);
+package top<M>(generic<M> c);
 header t1 {
     bit<32> f1;
     bit<16> h1;
@@ -14,3 +18,18 @@ struct headers_t {
     t2[8] stack;
 }
 
+control c(inout headers_t hdrs) {
+    @name("c.v") t2 v_0;
+    @name("c.v") bit<8> v_1;
+    @name("c.x") bit<16> x_0;
+    apply {
+        for (v_0 in hdrs.stack) {
+            hdrs.head.f1 = hdrs.head.f1 + v_0.x;
+        }
+        for (v_1 = 8w0, x_0 = 16w1; v_1 < hdrs.head.cnt; ) {
+            hdrs.head.h1 = hdrs.head.h1 + x_0;
+        }
+    }
+}
+
+top<headers_t>(c()) main;
diff --git a/testdata/p4_16_samples_outputs/forloop1-midend.p4 b/testdata/p4_16_samples_outputs/forloop1-midend.p4
@@ -0,0 +1,51 @@
+#include <core.p4>
+
+control generic<M>(inout M m);
+package top<M>(generic<M> c);
+header t1 {
+    bit<32> f1;
+    bit<16> h1;
+    bit<8>  b1;
+    bit<8>  cnt;
+}
+
+header t2 {
+    bit<32> x;
+}
+
+struct headers_t {
+    t1    head;
+    t2[8] stack;
+}
+
+control c(inout headers_t hdrs) {
+    @name("c.v") t2 v_0;
+    @hidden action forloop1l24() {
+        hdrs.head.f1 = hdrs.head.f1 + v_0.x;
+    }
+    @hidden action forloop1l27() {
+        hdrs.head.h1 = hdrs.head.h1 + 16w1;
+    }
+    @hidden table tbl_forloop1l24 {
+        actions = {
+            forloop1l24();
+        }
+        const default_action = forloop1l24();
+    }
+    @hidden table tbl_forloop1l27 {
+        actions = {
+            forloop1l27();
+        }
+        const default_action = forloop1l27();
+    }
+    apply {
+        for (v_0 in hdrs.stack) {
+            tbl_forloop1l24.apply();
+        }
+        for (; 8w0 < hdrs.head.cnt; ) {
+            tbl_forloop1l27.apply();
+        }
+    }
+}
+
+top<headers_t>(c()) main;
diff --git a/testdata/p4_16_samples_outputs/forloop1.p4 b/testdata/p4_16_samples_outputs/forloop1.p4
@@ -1,3 +1,7 @@
+#include <core.p4>
+
+control generic<M>(inout M m);
+package top<M>(generic<M> c);
 header t1 {
     bit<32> f1;
     bit<16> h1;
@@ -25,3 +29,4 @@ control c(inout headers_t hdrs) {
     }
 }
 
+top(c()) main;
diff --git a/testdata/p4_16_samples_outputs/forloop1.p4-stderr b/testdata/p4_16_samples_outputs/forloop1.p4-stderr
@@ -1,4 +1,9 @@
-forloop1.p4(18): [--Wwarn=unused] warning: Control c is not used; removing
-control c(inout headers_t hdrs) {
-        ^
-[--Wwarn=missing] warning: Program does not contain a `main' module
+forloop1.p4(23): [--Wwarn=uninitialized_use] warning: v may not be completely initialized
+        for (t2 v in hdrs.stack) {
+                ^
+forloop1.p4(24): [--Wwarn=invalid_header] warning: accessing a field of a potentially invalid header v
+            hdrs.head.f1 = hdrs.head.f1 + v.x;
+                                          ^
+forloop1.p4(24): [--Wwarn=uninitialized_use] warning: v.x may be uninitialized
+            hdrs.head.f1 = hdrs.head.f1 + v.x;
+                                          ^^^