Fix p4lang#4969 by modifying pass LocalizeActions

so that it prepends a "." to @name annotations of global actions, if they do not have one already. Signed-off-by: Andy Fingerhut <[email protected]>
jafingerhut · Oct 21, 2024 · 682564b · 682564b
1 parent da04995
commit 682564b
Show file tree

Hide file tree

Showing 9 changed files with 463 additions and 3 deletions.
diff --git a/frontends/p4/localizeActions.cpp b/frontends/p4/localizeActions.cpp
@@ -43,9 +43,25 @@ const IR::Node *TagGlobalActions::preorder(IR::P4Action *action) {
     if (findContext<IR::P4Control>() == nullptr) {
         auto annos = action->annotations;
         if (annos == nullptr) annos = IR::Annotations::empty;
-        cstring name = "."_cs + action->name;
-        annos = annos->addAnnotationIfNew(IR::Annotation::nameAnnotation,
-                                          new IR::StringLiteral(name), false);
+        auto nameAnno = annos->getSingle(IR::Annotation::nameAnnotation);
+        if (nameAnno) {
+            // If the value of the existing name annotation does not
+            // begin with ".", prepend "." so that the name remains
+            // global if control plane APIs are generated later.
+            const auto *e0 = nameAnno->expr.at(0);
+            auto nameString = e0->to<IR::StringLiteral>()->value;
+            if (!nameString.startsWith(".")) {
+                nameString = "."_cs + nameString;
+                auto newLit = new IR::StringLiteral(e0->srcInfo, nameString);
+                annos = annos->addOrReplace(IR::Annotation::nameAnnotation,
+                                            newLit);
+            }
+        } else {
+            // Add new name annotation beginning with "."
+            cstring name = "."_cs + action->name;
+            annos = annos->addAnnotationIfNew(IR::Annotation::nameAnnotation,
+                                              new IR::StringLiteral(name), false);
+        }
         action->annotations = annos;
     }
     prune();

diff --git a/testdata/p4_16_samples/issue-4969-bmv2.p4 b/testdata/p4_16_samples/issue-4969-bmv2.p4
@@ -0,0 +1,106 @@
+/*
+Copyright 2022 Cisco Systems, Inc.
+
+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.
+*/
+
+#include <core.p4>
+#include <v1model.p4>
+
+typedef bit<48>  EthernetAddress;
+
+header ethernet_t {
+    EthernetAddress dstAddr;
+    EthernetAddress srcAddr;
+    bit<16>         etherType;
+}
+
+struct headers_t {
+    ethernet_t    ethernet;
+}
+
+struct metadata_t {
+}
+
+parser parserImpl(
+    packet_in pkt,
+    out headers_t hdr,
+    inout metadata_t meta,
+    inout standard_metadata_t stdmeta)
+{
+    state start {
+        pkt.extract(hdr.ethernet);
+        transition accept;
+    }
+}
+
+control verifyChecksum(
+    inout headers_t hdr,
+    inout metadata_t meta)
+{
+    apply { }
+}
+
+action foo1() {
+}
+
+@name("bar") action foo2() {
+}
+
+control ingressImpl(
+    inout headers_t hdr,
+    inout metadata_t meta,
+    inout standard_metadata_t stdmeta)
+
+{
+    table t1 {
+        actions = { NoAction; foo1; foo2; }
+        key = { hdr.ethernet.etherType: exact; }
+        default_action = NoAction();
+        size = 512;
+    }
+    apply {
+        t1.apply();
+    }
+}
+
+control egressImpl(
+    inout headers_t hdr,
+    inout metadata_t meta,
+    inout standard_metadata_t stdmeta)
+{
+    apply { }
+}
+
+control updateChecksum(
+    inout headers_t hdr,
+    inout metadata_t meta)
+{
+    apply { }
+}
+
+control deparserImpl(
+    packet_out pkt,
+    in headers_t hdr)
+{
+    apply {
+        pkt.emit(hdr.ethernet);
+    }
+}
+
+V1Switch(parserImpl(),
+         verifyChecksum(),
+         ingressImpl(),
+         egressImpl(),
+         updateChecksum(),
+         deparserImpl()) main;
diff --git a/testdata/p4_16_samples_outputs/issue-4969-bmv2-first.p4 b/testdata/p4_16_samples_outputs/issue-4969-bmv2-first.p4
@@ -0,0 +1,69 @@
+#include <core.p4>
+#define V1MODEL_VERSION 20180101
+#include <v1model.p4>
+
+typedef bit<48> EthernetAddress;
+header ethernet_t {
+    EthernetAddress dstAddr;
+    EthernetAddress srcAddr;
+    bit<16>         etherType;
+}
+
+struct headers_t {
+    ethernet_t ethernet;
+}
+
+struct metadata_t {
+}
+
+parser parserImpl(packet_in pkt, out headers_t hdr, inout metadata_t meta, inout standard_metadata_t stdmeta) {
+    state start {
+        pkt.extract<ethernet_t>(hdr.ethernet);
+        transition accept;
+    }
+}
+
+control verifyChecksum(inout headers_t hdr, inout metadata_t meta) {
+    apply {
+    }
+}
+
+action foo1() {
+}
+@name("bar") action foo2() {
+}
+control ingressImpl(inout headers_t hdr, inout metadata_t meta, inout standard_metadata_t stdmeta) {
+    table t1 {
+        actions = {
+            NoAction();
+            foo1();
+            foo2();
+        }
+        key = {
+            hdr.ethernet.etherType: exact @name("hdr.ethernet.etherType");
+        }
+        default_action = NoAction();
+        size = 512;
+    }
+    apply {
+        t1.apply();
+    }
+}
+
+control egressImpl(inout headers_t hdr, inout metadata_t meta, inout standard_metadata_t stdmeta) {
+    apply {
+    }
+}
+
+control updateChecksum(inout headers_t hdr, inout metadata_t meta) {
+    apply {
+    }
+}
+
+control deparserImpl(packet_out pkt, in headers_t hdr) {
+    apply {
+        pkt.emit<ethernet_t>(hdr.ethernet);
+    }
+}
+
+V1Switch<headers_t, metadata_t>(parserImpl(), verifyChecksum(), ingressImpl(), egressImpl(), updateChecksum(), deparserImpl()) main;
diff --git a/testdata/p4_16_samples_outputs/issue-4969-bmv2-frontend.p4 b/testdata/p4_16_samples_outputs/issue-4969-bmv2-frontend.p4
@@ -0,0 +1,71 @@
+#include <core.p4>
+#define V1MODEL_VERSION 20180101
+#include <v1model.p4>
+
+typedef bit<48> EthernetAddress;
+header ethernet_t {
+    EthernetAddress dstAddr;
+    EthernetAddress srcAddr;
+    bit<16>         etherType;
+}
+
+struct headers_t {
+    ethernet_t ethernet;
+}
+
+struct metadata_t {
+}
+
+parser parserImpl(packet_in pkt, out headers_t hdr, inout metadata_t meta, inout standard_metadata_t stdmeta) {
+    state start {
+        pkt.extract<ethernet_t>(hdr.ethernet);
+        transition accept;
+    }
+}
+
+control verifyChecksum(inout headers_t hdr, inout metadata_t meta) {
+    apply {
+    }
+}
+
+control ingressImpl(inout headers_t hdr, inout metadata_t meta, inout standard_metadata_t stdmeta) {
+    @noWarn("unused") @name(".NoAction") action NoAction_1() {
+    }
+    @name(".foo1") action foo1_0() {
+    }
+    @name(".bar") action foo2_0() {
+    }
+    @name("ingressImpl.t1") table t1_0 {
+        actions = {
+            NoAction_1();
+            foo1_0();
+            foo2_0();
+        }
+        key = {
+            hdr.ethernet.etherType: exact @name("hdr.ethernet.etherType");
+        }
+        default_action = NoAction_1();
+        size = 512;
+    }
+    apply {
+        t1_0.apply();
+    }
+}
+
+control egressImpl(inout headers_t hdr, inout metadata_t meta, inout standard_metadata_t stdmeta) {
+    apply {
+    }
+}
+
+control updateChecksum(inout headers_t hdr, inout metadata_t meta) {
+    apply {
+    }
+}
+
+control deparserImpl(packet_out pkt, in headers_t hdr) {
+    apply {
+        pkt.emit<ethernet_t>(hdr.ethernet);
+    }
+}
+
+V1Switch<headers_t, metadata_t>(parserImpl(), verifyChecksum(), ingressImpl(), egressImpl(), updateChecksum(), deparserImpl()) main;
diff --git a/testdata/p4_16_samples_outputs/issue-4969-bmv2-midend.p4 b/testdata/p4_16_samples_outputs/issue-4969-bmv2-midend.p4
@@ -0,0 +1,70 @@
+#include <core.p4>
+#define V1MODEL_VERSION 20180101
+#include <v1model.p4>
+
+header ethernet_t {
+    bit<48> dstAddr;
+    bit<48> srcAddr;
+    bit<16> etherType;
+}
+
+struct headers_t {
+    ethernet_t ethernet;
+}
+
+struct metadata_t {
+}
+
+parser parserImpl(packet_in pkt, out headers_t hdr, inout metadata_t meta, inout standard_metadata_t stdmeta) {
+    state start {
+        pkt.extract<ethernet_t>(hdr.ethernet);
+        transition accept;
+    }
+}
+
+control verifyChecksum(inout headers_t hdr, inout metadata_t meta) {
+    apply {
+    }
+}
+
+control ingressImpl(inout headers_t hdr, inout metadata_t meta, inout standard_metadata_t stdmeta) {
+    @noWarn("unused") @name(".NoAction") action NoAction_1() {
+    }
+    @name(".foo1") action foo1_0() {
+    }
+    @name(".bar") action foo2_0() {
+    }
+    @name("ingressImpl.t1") table t1_0 {
+        actions = {
+            NoAction_1();
+            foo1_0();
+            foo2_0();
+        }
+        key = {
+            hdr.ethernet.etherType: exact @name("hdr.ethernet.etherType");
+        }
+        default_action = NoAction_1();
+        size = 512;
+    }
+    apply {
+        t1_0.apply();
+    }
+}
+
+control egressImpl(inout headers_t hdr, inout metadata_t meta, inout standard_metadata_t stdmeta) {
+    apply {
+    }
+}
+
+control updateChecksum(inout headers_t hdr, inout metadata_t meta) {
+    apply {
+    }
+}
+
+control deparserImpl(packet_out pkt, in headers_t hdr) {
+    apply {
+        pkt.emit<ethernet_t>(hdr.ethernet);
+    }
+}
+
+V1Switch<headers_t, metadata_t>(parserImpl(), verifyChecksum(), ingressImpl(), egressImpl(), updateChecksum(), deparserImpl()) main;