Split the start state more conservatively when a parser contains decl initializers

frontends/p4/moveDeclarations.cpp

@@ -97,7 +97,30 @@ const IR::Node *MoveDeclarations::postorder(IR::Declaration_Constant *decl) {
 
 ////////////////////////////////////////////////////////////////////
 
+// Returns true iff the given state is _directly_ reachable from any of the given
+// set of states.
+bool isReachable(cstring stateName, const IR::IndexedVector<IR::ParserState> &states) {
+    for (const auto *state : states) {
+        const auto *selectExpr = state->selectExpression;
+        if (selectExpr == nullptr) continue;  // implicit reject transition
+
+        if (const auto *sel = selectExpr->to<IR::SelectExpression>()) {
+            // select expression
+            for (const auto *selectCase : sel->selectCases)
+                if (selectCase->state->path->name == stateName) return true;
+        } else if (const auto *pathExpr = selectExpr->to<IR::PathExpression>()) {
+            // direct transition
+            if (pathExpr->path->name == stateName) return true;
+        }
+    }
+
+    return false;
+}
+
 const IR::Node *MoveInitializers::preorder(IR::P4Parser *parser) {
+    oldStart = nullptr;
+    loopsBackToStart = false;
+
     // Determine if we have anything to do in this parser.
     // Check if we have top-level declarations with intializers.
     auto someInitializers = false;
@@ -109,24 +132,22 @@ const IR::Node *MoveInitializers::preorder(IR::P4Parser *parser) {
             }
         }
     }
+
     if (someInitializers) {
+        // Check whether any of the parser's states loop back to the start state.
+        // If this is the case, then the parser's decl initializers cannot be moved
+        // to the start state. A new start state that is never looped back to must
+        // be inserted at the head of the graph.
+        loopsBackToStart = isReachable(IR::ParserState::start, parser->states);
+
         newStartName = nameGen.newName(IR::ParserState::start.string_view());
         oldStart = parser->states.getDeclaration(IR::ParserState::start)->to<IR::ParserState>();
         CHECK_NULL(oldStart);
     }
     return parser;
 }
 
-const IR::Node *MoveInitializers::postorder(IR::P4Parser *parser) {
-    if (oldStart == nullptr) return parser;
-    auto newStart = new IR::ParserState(IR::ID(IR::ParserState::start), *toMove,
-                                        new IR::PathExpression(newStartName));
-    toMove = new IR::IndexedVector<IR::StatOrDecl>();
-    parser->states.insert(parser->states.begin(), newStart);
-    return parser;
-}
-
-const IR::Node *MoveInitializers::postorder(IR::Declaration_Variable *decl) {
+const IR::Node *MoveInitializers::preorder(IR::Declaration_Variable *decl) {
     if (getContext() == nullptr) return decl;
     auto parent = getContext()->node;
     if (!parent->is<IR::P4Control>() && !parent->is<IR::P4Parser>())
@@ -136,34 +157,61 @@ const IR::Node *MoveInitializers::postorder(IR::Declaration_Variable *decl) {
 
     auto varRef = new IR::PathExpression(decl->name);
     auto assign = new IR::AssignmentStatement(decl->srcInfo, varRef, decl->initializer);
-    toMove->push_back(assign);
+    toMove.push_back(assign);
     decl->initializer = nullptr;
     return decl;
 }
 
 const IR::Node *MoveInitializers::postorder(IR::ParserState *state) {
-    if (oldStart == nullptr) return state;
-    if (state->name == IR::ParserState::start) state->name = newStartName;
-    return state;
-}
+    if (state->name == IR::ParserState::start && !toMove.empty()) {
+        if (!loopsBackToStart) {
+            // Just prepend all initializing assignments to the original start
+            // state if there are no loops back to the start state.
+            state->components.insert(state->components.begin(), toMove.begin(), toMove.end());
+            toMove.clear();
+        } else {
+            // Otherwise, we need to insert all such assignments into a new start state
+            // that can only run once, so that the assignments are only executed one time.
+            state->name = newStartName;
+        }
+    }
 
-const IR::Node *MoveInitializers::postorder(IR::P4Control *control) {
-    if (toMove->empty()) return control;
-    toMove->append(control->body->components);
-    auto newBody =
-        new IR::BlockStatement(control->body->srcInfo, control->body->annotations, *toMove);
-    control->body = newBody;
-    toMove = new IR::IndexedVector<IR::StatOrDecl>();
-    return control;
+    return state;
 }
 
 const IR::Node *MoveInitializers::postorder(IR::Path *path) {
-    if (oldStart == nullptr || path->name != IR::ParserState::start) return path;
+    if (!oldStart || !loopsBackToStart || path->name != IR::ParserState::start) return path;
+
+    // Only rename start state references if the parser contains initializing assignments
+    // and loops back to the start state.
     auto decl = getDeclaration(getOriginal()->to<IR::Path>());
     if (!decl->is<IR::ParserState>()) return path;
     return new IR::Path(newStartName);
 }
 
+const IR::Node *MoveInitializers::postorder(IR::P4Parser *parser) {
+    if (oldStart && loopsBackToStart) {
+        // Only add a new state if the parser has initializers and contains one
+        // or more states that loop back to the start state.
+        auto newStart = new IR::ParserState(IR::ID(IR::ParserState::start), toMove,
+                                            new IR::PathExpression(newStartName));
+        toMove.clear();
+        parser->states.insert(parser->states.begin(), newStart);
+    }
+
+    return parser;
+}
+
+const IR::Node *MoveInitializers::postorder(IR::P4Control *control) {
+    if (toMove.empty()) return control;
+    toMove.append(control->body->components);
+    auto newBody =
+        new IR::BlockStatement(control->body->srcInfo, control->body->annotations, toMove);
+    control->body = newBody;
+    toMove.clear();
+    return control;
+}
+
 Visitor::profile_t MoveInitializers::init_apply(const IR::Node *node) {
     auto rv = Transform::init_apply(node);
     node->apply(nameGen);

frontends/p4/moveDeclarations.h

@@ -101,22 +101,20 @@ class MoveDeclarations : public Transform {
  */
 class MoveInitializers : public Transform, public ResolutionContext {
     MinimalNameGenerator nameGen;
-    IR::IndexedVector<IR::StatOrDecl> *toMove;  // This contains just IR::AssignmentStatement
+    IR::IndexedVector<IR::StatOrDecl> toMove;  // This contains just IR::AssignmentStatement
     const IR::ParserState *oldStart;  // nullptr if we do not want to rename the start state
-    cstring newStartName;             // name allocated to the old start state
+    cstring newStartName;             // name allocated to the new start state
+    bool loopsBackToStart;            // true if start is reachable from any other state
 
  public:
-    MoveInitializers()
-        : toMove(new IR::IndexedVector<IR::StatOrDecl>()), oldStart(nullptr), newStartName("") {
-        setName("MoveInitializers");
-    }
+    MoveInitializers() : newStartName("") { setName("MoveInitializers"); }
     profile_t init_apply(const IR::Node *node) override;
     const IR::Node *preorder(IR::P4Parser *parser) override;
-    const IR::Node *postorder(IR::P4Parser *parser) override;
-    const IR::Node *postorder(IR::Declaration_Variable *decl) override;
+    const IR::Node *preorder(IR::Declaration_Variable *decl) override;
     const IR::Node *postorder(IR::ParserState *state) override;
-    const IR::Node *postorder(IR::P4Control *control) override;
     const IR::Node *postorder(IR::Path *path) override;
+    const IR::Node *postorder(IR::P4Parser *parser) override;
+    const IR::Node *postorder(IR::P4Control *control) override;
 };
 
 }  // namespace P4

testdata/p4_16_samples/issue4901_new_start_with_no_start_loopback.p4

@@ -0,0 +1,22 @@
+parser Parser1() {
+  bit<8> l = 0;
+  state start {
+    transition next;
+  }
+  state next {
+    transition select(l) {
+        default: accept;
+    }
+  }
+}
+
+parser Parser2() {
+  state start {
+    transition  accept;
+  }
+}
+
+parser proto();
+package top(proto p1, proto p2);
+top(Parser1(), Parser2()) main;
+

testdata/p4_16_samples/issue4901_new_start_with_start_state_loopback.p4

@@ -0,0 +1,23 @@
+parser Parser1() {
+  bit<8> l = 0;
+  state start {
+    transition next;
+  }
+  state next {
+    transition select(l) {
+        2 : start;
+        default: accept;
+    }
+  }
+}
+
+parser Parser2() {
+  state start {
+    transition  accept;
+  }
+}
+
+parser proto();
+package top(proto p1, proto p2);
+top(Parser1(), Parser2()) main;
+

testdata/p4_16_samples/issue4901_new_start_with_start_state_loopback2.p4

@@ -0,0 +1,30 @@
+parser Parser1() {
+  bit<8> l = 0;
+  state start {
+    transition next;
+  }
+  state next {
+    transition select(l) {
+        2 : start;
+        default: accept;
+    }
+  }
+}
+
+parser Parser2() {
+  bit<7> l = 1;
+  state start {
+    transition select (l) {
+        0 : s0;
+        default : accept;
+    }
+  }
+  state s0 {
+    transition accept;
+  }
+}
+
+parser proto();
+package top(proto p1, proto p2);
+top(Parser1(), Parser2()) main;
+

testdata/p4_16_samples_outputs/default-frontend.p4

@@ -8,9 +8,6 @@ parser p0(packet_in p, out Header h) {
     @name("p0.b") bool b_0;
     state start {
         b_0 = true;
-        transition start_0;
-    }
-    state start_0 {
         p.extract<Header>(h);
         transition select(h.data, b_0) {
             (default, true): next;

testdata/p4_16_samples_outputs/fabric_20190420/fabric-frontend.p4

@@ -214,9 +214,6 @@ parser FabricParser(packet_in packet, out parsed_headers_t hdr, inout fabric_met
     @name("FabricParser.tmp") bit<4> tmp;
     @name("FabricParser.tmp_0") bit<4> tmp_0;
     state start {
-        transition start_0;
-    }
-    state start_0 {
         transition select(standard_metadata.ingress_port) {
             9w255: parse_packet_out;
             default: parse_ethernet;

testdata/p4_16_samples_outputs/initializers-frontend.p4

@@ -10,9 +10,6 @@ parser P() {
     @name("P.fake") Fake() fake_0;
     state start {
         x_0 = 32w0;
-        transition start_0;
-    }
-    state start_0 {
         fake_0.call(x_0);
         transition accept;
     }

testdata/p4_16_samples_outputs/issue1538-frontend.p4

@@ -26,9 +26,6 @@ parser ParserImpl(packet_in packet, out headers hdr, inout metadata meta, inout
         x = (bit<16>)standard_metadata.ingress_port;
         retval = x + 16w1;
         tmp_port_0 = retval;
-        transition start_0;
-    }
-    state start_0 {
         packet.extract<ethernet_t>(hdr.ethernet);
         x_6 = hdr.ethernet.etherType;
         retval_0 = x_6 + 16w1;

testdata/p4_16_samples_outputs/issue2314-frontend.p4

@@ -28,9 +28,6 @@ struct m {
 parser MyParser(packet_in b, out h hdr, inout m meta, inout standard_metadata_t std) {
     @name("MyParser.l3.etherType") bit<16> l3_etherType;
     state start {
-        transition start_0;
-    }
-    state start_0 {
         b.extract<ethernet_t>(hdr.ether);
         transition L3_start;
     }

testdata/p4_16_samples_outputs/issue281-frontend.p4

@@ -42,9 +42,6 @@ struct m {
 parser MyParser(packet_in b, out h hdr, inout m meta, inout standard_metadata_t std) {
     @name("MyParser.l3.etherType") bit<16> l3_etherType;
     state start {
-        transition start_0;
-    }
-    state start_0 {
         hdr.ether.setInvalid();
         hdr.vlan.setInvalid();
         hdr.ipv4.setInvalid();

testdata/p4_16_samples_outputs/issue2957-frontend.p4

@@ -39,9 +39,6 @@ parser p(packet_in pkt, out Headers hdr) {
         tmp_3 = hdr.h[tmp_2];
         tmp_4 = extern_call(tmp_3);
         hdr.h[tmp_2] = tmp_3;
-        transition start_0;
-    }
-    state start_0 {
         pkt.extract<ethernet_t>(hdr.eth_hdr);
         pkt.extract<H>(hdr.h.next);
         transition accept;

testdata/p4_16_samples_outputs/issue361-bmv2-frontend.p4

@@ -17,9 +17,6 @@ parser MyParser(packet_in b, out my_packet p, inout my_metadata m, inout standar
     @name("MyParser.bv") bool bv_0;
     state start {
         bv_0 = true;
-        transition start_0;
-    }
-    state start_0 {
         transition select(bv_0) {
             false: next;
             true: accept;

testdata/p4_16_samples_outputs/issue4901_new_start_with_no_start_loopback-first.p4

@@ -0,0 +1,21 @@
+parser Parser1() {
+    bit<8> l = 8w0;
+    state start {
+        transition next;
+    }
+    state next {
+        transition select(l) {
+            default: accept;
+        }
+    }
+}
+
+parser Parser2() {
+    state start {
+        transition accept;
+    }
+}
+
+parser proto();
+package top(proto p1, proto p2);
+top(Parser1(), Parser2()) main;

testdata/p4_16_samples_outputs/issue4901_new_start_with_no_start_loopback-frontend.p4

@@ -0,0 +1,19 @@
+parser Parser1() {
+    @name("Parser1.l") bit<8> l_0;
+    state start {
+        l_0 = 8w0;
+        transition select(l_0) {
+            default: accept;
+        }
+    }
+}
+
+parser Parser2() {
+    state start {
+        transition accept;
+    }
+}
+
+parser proto();
+package top(proto p1, proto p2);
+top(Parser1(), Parser2()) main;

testdata/p4_16_samples_outputs/issue4901_new_start_with_no_start_loopback-midend.p4

@@ -0,0 +1,15 @@
+parser Parser1() {
+    state start {
+        transition accept;
+    }
+}
+
+parser Parser2() {
+    state start {
+        transition accept;
+    }
+}
+
+parser proto();
+package top(proto p1, proto p2);
+top(Parser1(), Parser2()) main;