p4lang · jafingerhut · Oct 7, 2024 · Oct 7, 2024 · Oct 8, 2024 · Oct 8, 2024
diff --git a/control-plane/p4RuntimeSerializer.cpp b/control-plane/p4RuntimeSerializer.cpp
@@ -1584,7 +1584,7 @@ void P4RuntimeSerializer::serializeP4RuntimeIfRequired(const IR::P4Program *prog
     std::vector<cstring> files;
     std::vector<P4::P4RuntimeFormat> formats;
 
-    // only generate P4Info is required by use-provided options
+    // only generate P4Info if required by user-provided options
     if (options.p4RuntimeFile.isNullOrEmpty() && options.p4RuntimeFiles.isNullOrEmpty() &&
         options.p4RuntimeEntriesFile.isNullOrEmpty() &&
         options.p4RuntimeEntriesFiles.isNullOrEmpty()) {

diff --git a/frontends/CMakeLists.txt b/frontends/CMakeLists.txt
@@ -33,6 +33,7 @@ set (P4_FRONTEND_SRCS
   p4/frontend.cpp
   p4/functionsInlining.cpp
   p4/hierarchicalNames.cpp
+  p4/duplicateActionControlPlaneNameCheck.cpp
   p4/inlining.cpp
   p4/localizeActions.cpp
   p4/methodInstance.cpp
@@ -110,6 +111,7 @@ set (P4_FRONTEND_HDRS
   p4/frontend.h
   p4/functionsInlining.h
   p4/hierarchicalNames.h
+  p4/duplicateActionControlPlaneNameCheck.h
   p4/inlining.h
   p4/localizeActions.h
   p4/methodInstance.h

diff --git a/frontends/common/options.h b/frontends/common/options.h
@@ -87,6 +87,17 @@ class CompilerOptions : public ParserOptions {
     bool optimizeSize = false;   // optimize favoring size
 
     virtual bool enable_intrinsic_metadata_fix();
+
+    /// Indicates whether control plane API generation is enabled.
+    /// @returns default to false unless a command line option was
+    /// given explicitly enabling control plane API generation.
+    virtual bool controlPlaneAPIGenEnabled() const {
+        if (p4RuntimeFile.isNullOrEmpty() && p4RuntimeFiles.isNullOrEmpty() &&
+            p4RuntimeEntriesFile.isNullOrEmpty() && p4RuntimeEntriesFiles.isNullOrEmpty()) {
+            return false;
+        }
+        return true;
+    }
 };
 
 }  // namespace P4

diff --git a/frontends/p4/duplicateActionControlPlaneNameCheck.cpp b/frontends/p4/duplicateActionControlPlaneNameCheck.cpp
@@ -0,0 +1,99 @@
+/*
+Copyright 2024 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 "duplicateActionControlPlaneNameCheck.h"
+
+#include "lib/log.h"
+
+namespace P4 {
+
+cstring DuplicateActionControlPlaneNameCheck::getName(const IR::IDeclaration *decl) {
+    return decl->getName();
+}
+
+void DuplicateActionControlPlaneNameCheck::checkForDuplicateName(cstring name,
+                                                                 const IR::Node *node) {
+    bool foundDuplicate = false;
+    auto *otherNode = node;
+    auto [it, inserted] = actions.insert(std::pair(name, node));
-    auto [it, inserted] = actions.insert(std::pair(name, node));
+    auto [it, inserted] = actions.emplace(name, node);
-    auto [it, inserted] = actions.insert(std::pair(name, node));
+    auto [it, inserted] = actions.emplace(name, node);
+    if (!inserted) {
+        foundDuplicate = true;
+        otherNode = (*it).second;
-        otherNode = (*it).second;
+        otherNode = it->second;
-        otherNode = (*it).second;
+        otherNode = it->second;
+    }
+    if (foundDuplicate) {
+        error(ErrorType::ERR_DUPLICATE, "%1%: conflicting control plane name: %2%", node,
+              otherNode);
+    }
+}
+
+const IR::Node *DuplicateActionControlPlaneNameCheck::postorder(IR::P4Action *action) {
+    // Actions declared at the top level that the developer did not
+    // write a @name annotation for it, should be included in the
+    // duplicate name check.  They do not yet have a @name annotation
+    // by the time this pass executes, so this method will handle
+    // such actions.
+    if (findContext<IR::P4Control>() == nullptr) {
+        auto annos = action->annotations;
+        bool hasNameAnnotation = false;
+        if (annos != nullptr) {
+            auto nameAnno = annos->getSingle(IR::Annotation::nameAnnotation);
+            if (nameAnno) {
+                hasNameAnnotation = true;
+            }
+        }
+        if (!hasNameAnnotation) {
+            // name is what this top-level action's @name annotation
+            // will be, after LocalizeAllActions pass adds one.
+            cstring name = "." + action->name;
+            checkForDuplicateName(name, action);
+        }
+    }
+    return action;
+}
+
+const IR::Node *DuplicateActionControlPlaneNameCheck::postorder(IR::Annotation *annotation) {
+    if (annotation->name != IR::Annotation::nameAnnotation) return annotation;
+    // The node the annotation belongs to
+    CHECK_NULL(getContext()->parent);
+    auto *annotatedNode = getContext()->parent->node;
+    CHECK_NULL(annotatedNode);
+    // We are only interested in name annotations on actions here, as
+    // the P4Runtime API control plane generation code already checks
+    // for duplicate control plane names for other kinds of objects.
+    if (!annotatedNode->is<IR::P4Action>()) {
+        return annotation;
+    }
+    cstring name = annotation->getName();
+    if (!name.startsWith(".")) {
+        // Create a fully hierarchical name beginning with ".", so we
+        // can compare it against any other @name annotation value
+        // that begins with "." and is equal.
+        if (stack.empty()) {
+            name = absl::StrCat(".", name.string_view());
+        } else {
+            name = absl::StrCat(".",
+                                absl::StrJoin(stack, ".",
+                                              [](std::string *out, cstring s) {
+                                                  absl::StrAppend(out, s.string_view());
+                                              }),
+                                ".", name.string_view());
+        }
+    }
+    checkForDuplicateName(name, annotatedNode);
+    return annotation;
+}
+
+}  // namespace P4
diff --git a/frontends/p4/duplicateActionControlPlaneNameCheck.h b/frontends/p4/duplicateActionControlPlaneNameCheck.h
@@ -0,0 +1,91 @@
+/*
+Copyright 2024 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.
+*/
+
+#ifndef FRONTENDS_P4_DUPLICATEACTIONCONTROLPLANENAMECHECK_H_
+#define FRONTENDS_P4_DUPLICATEACTIONCONTROLPLANENAMECHECK_H_
+
+#include "ir/ir.h"
+#include "ir/visitor.h"
+
+namespace P4 {
+
+/**
+ * This pass does not change anything in the IR.  It only gives an
+ * error if it finds two objects with the same hierarchical name.  I
+ * am not certain, but it might be that at this point in the front
+ * end, the only way such a duplicate can happen is due to @name
+ * annotations.  @name annotations are definitely taken into account
+ * in this duplicate check.
+ *
+ * See also the pass HierarchicalNames, on which the implementation of
+ * this pass was based.
+ *
+ * This pass should be run before pass LocalizeAllActions, because
+ * LocalizeAllActions can create actions with duplicate names, by
+ * design, that were not created by the P4 developer.  We should not
+ * issue an error if that pass creates duplicate hierarchical names.
+ */
+class DuplicateActionControlPlaneNameCheck : public Transform {
+    std::vector<cstring> stack;
+    /// Used for detection of conflicting control plane names among actions.
+    string_map<const IR::Node *> actions;
+
+ public:
+    cstring getName(const IR::IDeclaration *decl);
+
+    DuplicateActionControlPlaneNameCheck() {
+        setName("DuplicateActionControlPlaneNameCheck");
+        visitDagOnce = false;
+    }
+    const IR::Node *preorder(IR::P4Parser *parser) override {
+        stack.push_back(getName(parser));
+        return parser;
+    }
+    const IR::Node *postorder(IR::P4Parser *parser) override {
+        stack.pop_back();
+        return parser;
+    }
+
+    const IR::Node *preorder(IR::P4Control *control) override {
+        stack.push_back(getName(control));
+        return control;
+    }
+    const IR::Node *postorder(IR::P4Control *control) override {
+        stack.pop_back();
+        return control;
+    }
+
+    const IR::Node *preorder(IR::P4Table *table) override {
+        visit(table->annotations);
+        prune();
+        return table;
+    }
+
+    void checkForDuplicateName(cstring name, const IR::Node *node);
+
+    const IR::Node *postorder(IR::P4Action *action) override;
+
+    const IR::Node *postorder(IR::Annotation *annotation) override;
+    // Do not change name annotations on parameters
+    const IR::Node *preorder(IR::Parameter *parameter) override {
+        prune();
+        return parameter;
+    }
+};
+
+}  // namespace P4
+
+#endif /* FRONTENDS_P4_DUPLICATEACTIONCONTROLPLANENAMECHECK_H_ */
diff --git a/frontends/p4/frontend.cpp b/frontends/p4/frontend.cpp
@@ -35,6 +35,7 @@ limitations under the License.
 #include "deprecated.h"
 #include "directCalls.h"
 #include "dontcareArgs.h"
+#include "duplicateActionControlPlaneNameCheck.h"
 #include "entryPriorities.h"
 #include "evaluator/evaluator.h"
 #include "frontends/common/constantFolding.h"
@@ -234,6 +235,15 @@ const IR::P4Program *FrontEnd::run(const CompilerOptions &options, const IR::P4P
     if (policy->optimize(options)) {
         passes.addPasses({
             new Inline(&typeMap, *policy, options.optimizeParserInlining),
+        });
+    }
+    if (options.controlPlaneAPIGenEnabled()) {
+        passes.addPasses({
+            new DuplicateActionControlPlaneNameCheck(),
+        });
+    }
+    if (policy->optimize(options)) {
+        passes.addPasses({
             new InlineActions(&typeMap, *policy),
             new LocalizeAllActions(*policy),
             new UniqueNames(),

diff --git a/frontends/p4/localizeActions.cpp b/frontends/p4/localizeActions.cpp
@@ -43,9 +43,24 @@ 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);
-            const auto *e0 = nameAnno->expr.at(0);
+            const auto *e0 = nameAnno->getExpr(0);
-            const auto *e0 = nameAnno->expr.at(0);
+            const auto *e0 = nameAnno->getExpr(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();