From e429c8a0cbc32aac315588e3acaa6ed841d29297 Mon Sep 17 00:00:00 2001 From: Andy Fingerhut Date: Mon, 7 Oct 2024 09:39:57 -0700 Subject: [PATCH 01/48] Add pass to give error if duplicate hierarchical names are present Signed-off-by: Andy Fingerhut --- .../p4/duplicateHierarchicalNameCheck.cpp | 48 ++++++++++ frontends/p4/duplicateHierarchicalNameCheck.h | 90 +++++++++++++++++++ frontends/p4/frontend.cpp | 2 + 3 files changed, 140 insertions(+) create mode 100644 frontends/p4/duplicateHierarchicalNameCheck.cpp create mode 100644 frontends/p4/duplicateHierarchicalNameCheck.h diff --git a/frontends/p4/duplicateHierarchicalNameCheck.cpp b/frontends/p4/duplicateHierarchicalNameCheck.cpp new file mode 100644 index 00000000000..fed631b1e44 --- /dev/null +++ b/frontends/p4/duplicateHierarchicalNameCheck.cpp @@ -0,0 +1,48 @@ +/* +Copyright 2017 VMware, 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 "duplicateHierarchicalNameCheck.h" + +#include "lib/log.h" + +namespace P4 { + +cstring DuplicateHierarchicalNameCheck::getName(const IR::IDeclaration *decl) { return decl->getName(); } + +const IR::Node *DuplicateHierarchicalNameCheck::postorder(IR::Annotation *annotation) { + if (annotation->name != IR::Annotation::nameAnnotation) return annotation; + + cstring name = annotation->getName(); + if (!name.startsWith(".")) { + std::string newName = ""; + for (cstring s : stack) newName += s + "."; + newName += name; + name = newName; + } + // The node the annotation belongs to + CHECK_NULL(getContext()->parent); + auto *annotatedNode = getContext()->parent->node; + CHECK_NULL(annotatedNode); + if (annotatedNodes.count(name)) { + error(ErrorType::ERR_DUPLICATE, "%1%: " ERR_STR_DUPLICATED_NAME ": %2%", + annotatedNode, annotatedNodes[name]); + } else { + annotatedNodes[name] = annotatedNode; + } + return annotation; +} + +} // namespace P4 diff --git a/frontends/p4/duplicateHierarchicalNameCheck.h b/frontends/p4/duplicateHierarchicalNameCheck.h new file mode 100644 index 00000000000..ad74fcda9cc --- /dev/null +++ b/frontends/p4/duplicateHierarchicalNameCheck.h @@ -0,0 +1,90 @@ +/* +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_DUPLICATEHIERARCHICALNAMECHECK_H_ +#define FRONTENDS_P4_DUPLICATEHIERARCHICALNAMECHECK_H_ + +#include "ir/ir.h" +#include "ir/visitor.h" + +namespace P4 { + +#define ERR_STR_DUPLICATED_NAME "conflicting control plane name" + +/** + * 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 is based. + * + * This pass should be run before LocalizeAllActions, because that + * pass can create actions with duplicate names, by design, that were + * not created by the P4 developer, but generated internally by the + * compiler. We should not issue an error if that pass creates + * duplicate hierarchical names. + */ +class DuplicateHierarchicalNameCheck : public Transform { + std::vector stack; + /// Used for detection of conflicting control plane names + std::map annotatedNodes; + + public: + cstring getName(const IR::IDeclaration *decl); + + DuplicateHierarchicalNameCheck() { + setName("DuplicateHierarchicalNameCheck"); + 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; + } + + 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_DUPLICATEHIERARCHICALNAMECHECK_H_ */ diff --git a/frontends/p4/frontend.cpp b/frontends/p4/frontend.cpp index f85c2163eda..2ed4814c71c 100644 --- a/frontends/p4/frontend.cpp +++ b/frontends/p4/frontend.cpp @@ -40,6 +40,7 @@ limitations under the License. #include "frontends/common/constantFolding.h" #include "functionsInlining.h" #include "getV1ModelVersion.h" +#include "duplicateHierarchicalNameCheck.h" #include "hierarchicalNames.h" #include "inlining.h" #include "localizeActions.h" @@ -238,6 +239,7 @@ const IR::P4Program *FrontEnd::run(const CompilerOptions &options, const IR::P4P if (policy->optimize(options)) { passes.addPasses({ new Inline(&refMap, &typeMap, evaluator, *policy, options.optimizeParserInlining), + new DuplicateHierarchicalNameCheck(), new InlineActions(&refMap, &typeMap, *policy), new LocalizeAllActions(&refMap, *policy), new UniqueNames(), From 39538bf98689a795f891993c35e4954015728a6f Mon Sep 17 00:00:00 2001 From: Andy Fingerhut Date: Mon, 7 Oct 2024 12:20:26 -0700 Subject: [PATCH 02/48] Add new source files to CMakeLists.txt Signed-off-by: Andy Fingerhut --- frontends/CMakeLists.txt | 2 ++ 1 file changed, 2 insertions(+) diff --git a/frontends/CMakeLists.txt b/frontends/CMakeLists.txt index 9a2acc3eeda..db6b590d3d1 100644 --- 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/duplicateHierarchicalNameCheck.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/duplicateHierarchicalNameCheck.h p4/inlining.h p4/localizeActions.h p4/methodInstance.h From 98940fb72c77896ebbbf4395b42cdb4b399d61c0 Mon Sep 17 00:00:00 2001 From: Andy Fingerhut Date: Tue, 8 Oct 2024 08:29:17 -0700 Subject: [PATCH 03/48] Skip new checks when compiling certain source files including: + Source files that are the output of the midend pass of the compiler, because they can include intentional name duplication created in the LocalizeActions pass. + P4-16 source files created by translating a P4-14 source file to P4-16. Signed-off-by: Andy Fingerhut --- backends/p4test/run-p4-sample.py | 15 +++++++++++---- 1 file changed, 11 insertions(+), 4 deletions(-) diff --git a/backends/p4test/run-p4-sample.py b/backends/p4test/run-p4-sample.py index ae0a89e2d73..5ab0fa373c6 100755 --- a/backends/p4test/run-p4-sample.py +++ b/backends/p4test/run-p4-sample.py @@ -164,7 +164,7 @@ def compare_files(options, produced, expected, ignore_case): return FAILURE -def recompile_file(options, produced, mustBeIdentical): +def recompile_file(options, produced, mustBeIdentical, origFileP414=False): # Compile the generated file a second time secondFile = produced + "-x" args = [ @@ -174,8 +174,10 @@ def recompile_file(options, produced, mustBeIdentical): secondFile, "--std", "p4-16", - produced, - ] + options.compilerOptions + ] + if origFileP414 or ('-midend' in produced): + args += ["--excludeFrontendPasses", "DuplicateHierarchicalNameCheck"] + args += [produced] + options.compilerOptions if options.runDebugger: if options.runDebugger_skip > 0: options.runDebugger_skip = options.runDebugger_skip - 1 @@ -314,7 +316,10 @@ def getArch(path): args.extend(["--p4runtime-files", p4runtimeFile]) args.extend(["--p4runtime-entries-files", p4runtimeEntriesFile]) + origFileP414 = False if "p4_14" in options.p4filename or "v1_samples" in options.p4filename: + origFileP414 = True + args.extend(["--excludeFrontendPasses", "DuplicateHierarchicalNameCheck"]) args.extend(["--std", "p4-14"]) args.extend(argv) if options.runDebugger: @@ -357,7 +362,9 @@ def getArch(path): if result == SUCCESS: result = check_generated_files(options, tmpdir, expected_dirname) if (result == SUCCESS) and (not expected_error): - result = recompile_file(options, ppfile, False) + print("jaf dbg just before recompile_file") + result = recompile_file(options, ppfile, False, origFileP414) + print("jaf dbg just after recompile_file") if ( (result == SUCCESS) and (not expected_error) From a2792ce380c41cbf50205b97302001ea1388a835 Mon Sep 17 00:00:00 2001 From: Andy Fingerhut Date: Tue, 8 Oct 2024 08:36:23 -0700 Subject: [PATCH 04/48] Skip new checks for P4-14 source files in run-bmv2-test.py Signed-off-by: Andy Fingerhut --- backends/bmv2/run-bmv2-test.py | 1 + 1 file changed, 1 insertion(+) diff --git a/backends/bmv2/run-bmv2-test.py b/backends/bmv2/run-bmv2-test.py index 94ec54d5dae..ac6fc2169a5 100755 --- a/backends/bmv2/run-bmv2-test.py +++ b/backends/bmv2/run-bmv2-test.py @@ -292,6 +292,7 @@ def process_file(options, argv): args = [binary, "-o", jsonfile] + options.compilerOptions if "p4_14" in options.p4filename or "v1_samples" in options.p4filename: + args.extend(["--excludeFrontendPasses", "DuplicateHierarchicalNameCheck"]) args.extend(["--std", "p4-14"]) args.append(options.p4filename) args.extend(argv) From a553fe17d0b549abda6b8cf0b5f841f0e71c31d2 Mon Sep 17 00:00:00 2001 From: Andy Fingerhut Date: Tue, 8 Oct 2024 09:47:58 -0700 Subject: [PATCH 05/48] Fix lint error Signed-off-by: Andy Fingerhut --- frontends/p4/duplicateHierarchicalNameCheck.cpp | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/frontends/p4/duplicateHierarchicalNameCheck.cpp b/frontends/p4/duplicateHierarchicalNameCheck.cpp index fed631b1e44..57fb1b7b5e5 100644 --- a/frontends/p4/duplicateHierarchicalNameCheck.cpp +++ b/frontends/p4/duplicateHierarchicalNameCheck.cpp @@ -1,5 +1,5 @@ /* -Copyright 2017 VMware, Inc. +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. @@ -20,7 +20,9 @@ limitations under the License. namespace P4 { -cstring DuplicateHierarchicalNameCheck::getName(const IR::IDeclaration *decl) { return decl->getName(); } +cstring DuplicateHierarchicalNameCheck::getName(const IR::IDeclaration *decl) { + return decl->getName(); +} const IR::Node *DuplicateHierarchicalNameCheck::postorder(IR::Annotation *annotation) { if (annotation->name != IR::Annotation::nameAnnotation) return annotation; From 26f05017458c4e7a6ab0e7210e6f90fd229b2a44 Mon Sep 17 00:00:00 2001 From: Andy Fingerhut Date: Tue, 8 Oct 2024 16:58:27 +0000 Subject: [PATCH 06/48] Fix lint problems. Signed-off-by: Andy Fingerhut Signed-off-by: Andy Fingerhut --- frontends/p4/duplicateHierarchicalNameCheck.cpp | 4 ++-- frontends/p4/frontend.cpp | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/frontends/p4/duplicateHierarchicalNameCheck.cpp b/frontends/p4/duplicateHierarchicalNameCheck.cpp index 57fb1b7b5e5..6b2ca0bb9fa 100644 --- a/frontends/p4/duplicateHierarchicalNameCheck.cpp +++ b/frontends/p4/duplicateHierarchicalNameCheck.cpp @@ -39,8 +39,8 @@ const IR::Node *DuplicateHierarchicalNameCheck::postorder(IR::Annotation *annota auto *annotatedNode = getContext()->parent->node; CHECK_NULL(annotatedNode); if (annotatedNodes.count(name)) { - error(ErrorType::ERR_DUPLICATE, "%1%: " ERR_STR_DUPLICATED_NAME ": %2%", - annotatedNode, annotatedNodes[name]); + error(ErrorType::ERR_DUPLICATE, "%1%: " ERR_STR_DUPLICATED_NAME ": %2%", annotatedNode, + annotatedNodes[name]); } else { annotatedNodes[name] = annotatedNode; } diff --git a/frontends/p4/frontend.cpp b/frontends/p4/frontend.cpp index 2ed4814c71c..7fb2e02e277 100644 --- a/frontends/p4/frontend.cpp +++ b/frontends/p4/frontend.cpp @@ -35,12 +35,12 @@ limitations under the License. #include "deprecated.h" #include "directCalls.h" #include "dontcareArgs.h" +#include "duplicateHierarchicalNameCheck.h" #include "entryPriorities.h" #include "evaluator/evaluator.h" #include "frontends/common/constantFolding.h" #include "functionsInlining.h" #include "getV1ModelVersion.h" -#include "duplicateHierarchicalNameCheck.h" #include "hierarchicalNames.h" #include "inlining.h" #include "localizeActions.h" From da456fe7a4388ee3f401c6a089a9fd037c6419ee Mon Sep 17 00:00:00 2001 From: Andy Fingerhut Date: Tue, 8 Oct 2024 15:37:04 -0700 Subject: [PATCH 07/48] Do not give errors for variable declarations with identical names Signed-off-by: Andy Fingerhut --- frontends/p4/duplicateHierarchicalNameCheck.cpp | 3 +++ 1 file changed, 3 insertions(+) diff --git a/frontends/p4/duplicateHierarchicalNameCheck.cpp b/frontends/p4/duplicateHierarchicalNameCheck.cpp index 6b2ca0bb9fa..31997edef2b 100644 --- a/frontends/p4/duplicateHierarchicalNameCheck.cpp +++ b/frontends/p4/duplicateHierarchicalNameCheck.cpp @@ -38,6 +38,9 @@ const IR::Node *DuplicateHierarchicalNameCheck::postorder(IR::Annotation *annota CHECK_NULL(getContext()->parent); auto *annotatedNode = getContext()->parent->node; CHECK_NULL(annotatedNode); + if (annotatedNode->is()) { + return annotation; + } if (annotatedNodes.count(name)) { error(ErrorType::ERR_DUPLICATE, "%1%: " ERR_STR_DUPLICATED_NAME ": %2%", annotatedNode, annotatedNodes[name]); From b683571353feccf765e06662d33ebead8723f397 Mon Sep 17 00:00:00 2001 From: Andy Fingerhut Date: Tue, 8 Oct 2024 16:20:08 -0700 Subject: [PATCH 08/48] Limit search for duplicate names to actions, tables, or "other objects" Explicitly allow a table and action to have the same name, since there is a test case that was written explicitly to test that this is permitted. Signed-off-by: Andy Fingerhut --- .../p4/duplicateHierarchicalNameCheck.cpp | 30 ++++++++++++++++--- frontends/p4/duplicateHierarchicalNameCheck.h | 9 ++++-- 2 files changed, 33 insertions(+), 6 deletions(-) diff --git a/frontends/p4/duplicateHierarchicalNameCheck.cpp b/frontends/p4/duplicateHierarchicalNameCheck.cpp index 31997edef2b..8008c68e699 100644 --- a/frontends/p4/duplicateHierarchicalNameCheck.cpp +++ b/frontends/p4/duplicateHierarchicalNameCheck.cpp @@ -41,11 +41,33 @@ const IR::Node *DuplicateHierarchicalNameCheck::postorder(IR::Annotation *annota if (annotatedNode->is()) { return annotation; } - if (annotatedNodes.count(name)) { - error(ErrorType::ERR_DUPLICATE, "%1%: " ERR_STR_DUPLICATED_NAME ": %2%", annotatedNode, - annotatedNodes[name]); + bool foundDuplicate = false; + auto *otherNode = annotatedNode; + if (annotatedNode->is()) { + if (annotatedTables.count(name)) { + foundDuplicate = true; + otherNode = annotatedTables[name]; + } else { + annotatedTables[name] = annotatedNode; + } + } else if (annotatedNode->is()) { + if (annotatedActions.count(name)) { + foundDuplicate = true; + otherNode = annotatedActions[name]; + } else { + annotatedActions[name] = annotatedNode; + } } else { - annotatedNodes[name] = annotatedNode; + if (annotatedOthers.count(name)) { + foundDuplicate = true; + otherNode = annotatedOthers[name]; + } else { + annotatedOthers[name] = annotatedNode; + } + } + if (foundDuplicate) { + error(ErrorType::ERR_DUPLICATE, "%1%: " ERR_STR_DUPLICATED_NAME ": %2%", annotatedNode, + otherNode); } return annotation; } diff --git a/frontends/p4/duplicateHierarchicalNameCheck.h b/frontends/p4/duplicateHierarchicalNameCheck.h index ad74fcda9cc..0b849e201b7 100644 --- a/frontends/p4/duplicateHierarchicalNameCheck.h +++ b/frontends/p4/duplicateHierarchicalNameCheck.h @@ -43,8 +43,13 @@ namespace P4 { */ class DuplicateHierarchicalNameCheck : public Transform { std::vector stack; - /// Used for detection of conflicting control plane names - std::map annotatedNodes; + /// Used for detection of conflicting control plane names among actions. + std::map annotatedActions; + /// Used for detection of conflicting control plane names among tables. + std::map annotatedTables; + /// Used for detection of conflicting control plane names among + /// objects other than actions and tables. + std::map annotatedOthers; public: cstring getName(const IR::IDeclaration *decl); From ad40fe9a3386948f9a4848a62e01936410531c81 Mon Sep 17 00:00:00 2001 From: Andy Fingerhut Date: Tue, 8 Oct 2024 17:18:25 -0700 Subject: [PATCH 09/48] Update test programs that had errors in their name annotations and for some others, update the error messages we expect to see. Signed-off-by: Andy Fingerhut --- backends/p4test/run-p4-sample.py | 2 - testdata/p4_16_errors/multicast-bmv2.p4 | 152 ++++++++++++++++++ .../issue1803_same_table_name.p4-stderr | 8 +- .../multicast-bmv2.p4-stderr | 6 + testdata/p4_16_samples/multicast-bmv2.p4 | 4 +- testdata/p4_16_samples/named_meter_1-bmv2.p4 | 4 +- testdata/p4_16_samples/named_meter_bmv2.p4 | 4 +- 7 files changed, 170 insertions(+), 10 deletions(-) create mode 100644 testdata/p4_16_errors/multicast-bmv2.p4 create mode 100644 testdata/p4_16_errors_outputs/multicast-bmv2.p4-stderr diff --git a/backends/p4test/run-p4-sample.py b/backends/p4test/run-p4-sample.py index 5ab0fa373c6..554583cca90 100755 --- a/backends/p4test/run-p4-sample.py +++ b/backends/p4test/run-p4-sample.py @@ -362,9 +362,7 @@ def getArch(path): if result == SUCCESS: result = check_generated_files(options, tmpdir, expected_dirname) if (result == SUCCESS) and (not expected_error): - print("jaf dbg just before recompile_file") result = recompile_file(options, ppfile, False, origFileP414) - print("jaf dbg just after recompile_file") if ( (result == SUCCESS) and (not expected_error) diff --git a/testdata/p4_16_errors/multicast-bmv2.p4 b/testdata/p4_16_errors/multicast-bmv2.p4 new file mode 100644 index 00000000000..808fefc0c3d --- /dev/null +++ b/testdata/p4_16_errors/multicast-bmv2.p4 @@ -0,0 +1,152 @@ +#include +#include + +struct routing_metadata_t { + bit<32> nhop_ipv4; +} + +header ethernet_t { + bit<48> dstAddr; + bit<48> srcAddr; + bit<16> etherType; +} + +header ipv4_t { + bit<4> version; + bit<4> ihl; + bit<8> diffserv; + bit<16> totalLen; + bit<16> identification; + bit<3> flags; + bit<13> fragOffset; + bit<8> ttl; + bit<8> protocol; + bit<16> hdrChecksum; + bit<32> srcAddr; + bit<32> dstAddr; +} + +struct metadata { + @name("routing_metadata") + routing_metadata_t routing_metadata; +} + +struct headers { + @name("ethernet") + ethernet_t ethernet; + @name("ipv4") + ipv4_t ipv4; +} + +parser ParserImpl(packet_in packet, out headers hdr, inout metadata meta, inout standard_metadata_t standard_metadata) { + @name("parse_ethernet") state parse_ethernet { + packet.extract(hdr.ethernet); + transition select(hdr.ethernet.etherType) { + 16w0x800: parse_ipv4; + default: accept; + } + } + @name("parse_ipv4") state parse_ipv4 { + packet.extract(hdr.ipv4); + transition accept; + } + @name("start") state start { + transition parse_ethernet; + } +} + +control egress(inout headers hdr, inout metadata meta, inout standard_metadata_t standard_metadata) { + @name(".rewrite_mac") action rewrite_mac(bit<48> smac) { + hdr.ethernet.srcAddr = smac; + } + @name("._drop") action _drop() { + mark_to_drop(standard_metadata); + } + @name("send_frame") table send_frame { + actions = { + rewrite_mac; + _drop; + } + key = { + standard_metadata.egress_port: exact; + } + size = 256; + } + apply { + send_frame.apply(); + } +} + +control ingress(inout headers hdr, inout metadata meta, inout standard_metadata_t standard_metadata) { + @name(".bcast") action bcast() { + standard_metadata.mcast_grp = 1; + } + @name(".set_dmac") action set_dmac(bit<48> dmac) { + hdr.ethernet.dstAddr = dmac; + } + @name("._drop") action _drop() { + mark_to_drop(standard_metadata); + } + @name(".set_nhop") action set_nhop(bit<32> nhop_ipv4, bit<9> port) { + meta.routing_metadata.nhop_ipv4 = nhop_ipv4; + standard_metadata.egress_spec = port; + hdr.ipv4.ttl = hdr.ipv4.ttl + 8w255; + } + @name(".set_nhop_redirect") action set_nhop_redirect(bit<32> nhop_ipv4, bit<9> port) { + meta.routing_metadata.nhop_ipv4 = nhop_ipv4; + standard_metadata.egress_spec = port; + hdr.ipv4.ttl = hdr.ipv4.ttl + 8w255; + } + @name("broadcast") table broadcast { + actions = { + bcast; + } + size = 1; + } + @name("forward") table forward { + actions = { + set_dmac; + _drop; + } + key = { + meta.routing_metadata.nhop_ipv4: exact; + } + size = 512; + } + @name("ipv4_lpm") table ipv4_lpm { + actions = { + set_nhop; + _drop; + } + key = { + hdr.ipv4.dstAddr: lpm; + } + size = 1024; + } + apply { + if (hdr.ipv4.isValid()) { + ipv4_lpm.apply(); + forward.apply(); + } + else { + broadcast.apply(); + } + } +} + +control DeparserImpl(packet_out packet, in headers hdr) { + apply { + packet.emit(hdr.ethernet); + packet.emit(hdr.ipv4); + } +} + +control verifyChecksum(inout headers hdr, inout metadata meta) { + apply { } +} + +control computeChecksum(inout headers hdr, inout metadata meta) { + apply { } +} + +V1Switch(ParserImpl(), verifyChecksum(), ingress(), egress(), computeChecksum(), DeparserImpl()) main; diff --git a/testdata/p4_16_errors_outputs/issue1803_same_table_name.p4-stderr b/testdata/p4_16_errors_outputs/issue1803_same_table_name.p4-stderr index d6929e2b197..1128bce3b15 100644 --- a/testdata/p4_16_errors_outputs/issue1803_same_table_name.p4-stderr +++ b/testdata/p4_16_errors_outputs/issue1803_same_table_name.p4-stderr @@ -1,2 +1,6 @@ -[--Werror=duplicate] error: Name 't0' is used for multiple table objects in the P4Info message -[--Werror=duplicate] error: Found 1 duplicate name(s) in the P4Info +issue1803_same_table_name.p4(19): [--Werror=duplicate] error: .t0: conflicting control plane name: .t0 + table t0 { + ^^ +issue1803_same_table_name.p4(19) + table t0 { + ^^ diff --git a/testdata/p4_16_errors_outputs/multicast-bmv2.p4-stderr b/testdata/p4_16_errors_outputs/multicast-bmv2.p4-stderr new file mode 100644 index 00000000000..835e09d908e --- /dev/null +++ b/testdata/p4_16_errors_outputs/multicast-bmv2.p4-stderr @@ -0,0 +1,6 @@ +multicast-bmv2.p4(87): [--Werror=duplicate] error: ._drop: conflicting control plane name: ._drop + @name("._drop") action _drop() { + ^^^^^ +multicast-bmv2.p4(62) + @name("._drop") action _drop() { + ^^^^^ diff --git a/testdata/p4_16_samples/multicast-bmv2.p4 b/testdata/p4_16_samples/multicast-bmv2.p4 index 808fefc0c3d..b912801932a 100644 --- a/testdata/p4_16_samples/multicast-bmv2.p4 +++ b/testdata/p4_16_samples/multicast-bmv2.p4 @@ -59,7 +59,7 @@ control egress(inout headers hdr, inout metadata meta, inout standard_metadata_t @name(".rewrite_mac") action rewrite_mac(bit<48> smac) { hdr.ethernet.srcAddr = smac; } - @name("._drop") action _drop() { + @name("_drop") action _drop() { mark_to_drop(standard_metadata); } @name("send_frame") table send_frame { @@ -84,7 +84,7 @@ control ingress(inout headers hdr, inout metadata meta, inout standard_metadata_ @name(".set_dmac") action set_dmac(bit<48> dmac) { hdr.ethernet.dstAddr = dmac; } - @name("._drop") action _drop() { + @name("_drop") action _drop() { mark_to_drop(standard_metadata); } @name(".set_nhop") action set_nhop(bit<32> nhop_ipv4, bit<9> port) { diff --git a/testdata/p4_16_samples/named_meter_1-bmv2.p4 b/testdata/p4_16_samples/named_meter_1-bmv2.p4 index 80c156d848e..1e2f85fe56f 100644 --- a/testdata/p4_16_samples/named_meter_1-bmv2.p4 +++ b/testdata/p4_16_samples/named_meter_1-bmv2.p4 @@ -76,11 +76,11 @@ control ingress(inout headers hdr, inout metadata meta, inout standard_metadata_ size = 16; default_action = NoAction(); } - @name("m_action") action m_action_0(bit<9> meter_idx) { + @name("m_action_2") action m_action_0(bit<9> meter_idx) { standard_metadata.egress_spec = meter_idx; my_meter.read(meta.meta.meter_tag); } - @name("_nop") action _nop_0() { + @name("_nop_2") action _nop_0() { my_meter.read(meta.meta.meter_tag); } @name("m_table") table m_table { diff --git a/testdata/p4_16_samples/named_meter_bmv2.p4 b/testdata/p4_16_samples/named_meter_bmv2.p4 index f420c6efeeb..1f0ecf84210 100644 --- a/testdata/p4_16_samples/named_meter_bmv2.p4 +++ b/testdata/p4_16_samples/named_meter_bmv2.p4 @@ -75,12 +75,12 @@ control ingress(inout headers hdr, inout metadata meta, inout standard_metadata_ size = 16; default_action = NoAction(); } - @name("m_action") action m_action_0(bit<9> meter_idx) { + @name("m_action_2") action m_action_0(bit<9> meter_idx) { standard_metadata.egress_spec = meter_idx; standard_metadata.egress_spec = 9w1; my_meter.read(meta.meta.meter_tag); } - @name("_nop") action _nop_0() { + @name("_nop_2") action _nop_0() { my_meter.read(meta.meta.meter_tag); } @name("m_table") table m_table { From 91166c897800d2bfecc1a414bfeae61a635d8552 Mon Sep 17 00:00:00 2001 From: Andy Fingerhut Date: Tue, 8 Oct 2024 21:53:09 -0700 Subject: [PATCH 10/48] Update expected output files for modified test programs Signed-off-by: Andy Fingerhut --- .../multicast-bmv2-first.p4 | 162 ++++++++++++++++++ .../p4_16_errors_outputs/multicast-bmv2.p4 | 154 +++++++++++++++++ testdata/p4_16_samples/issue1949-bmv2.p4 | 84 +++++++++ .../name_annotations_to_p4info-bmv2.p4 | 69 ++++++++ .../multicast-bmv2-first.p4 | 4 +- .../multicast-bmv2-frontend.p4 | 6 +- .../multicast-bmv2-midend.p4 | 6 +- .../p4_16_samples_outputs/multicast-bmv2.p4 | 4 +- .../multicast-bmv2.p4.p4info.txtpb | 19 +- .../named_meter_1-bmv2-first.p4 | 4 +- .../named_meter_1-bmv2-frontend.p4 | 4 +- .../named_meter_1-bmv2-midend.p4 | 4 +- .../named_meter_1-bmv2.p4 | 4 +- .../named_meter_1-bmv2.p4.p4info.txtpb | 17 +- .../named_meter_bmv2-first.p4 | 4 +- .../named_meter_bmv2-frontend.p4 | 4 +- .../named_meter_bmv2-midend.p4 | 4 +- .../p4_16_samples_outputs/named_meter_bmv2.p4 | 4 +- .../named_meter_bmv2.p4.p4info.txtpb | 17 +- 19 files changed, 532 insertions(+), 42 deletions(-) create mode 100644 testdata/p4_16_errors_outputs/multicast-bmv2-first.p4 create mode 100644 testdata/p4_16_errors_outputs/multicast-bmv2.p4 create mode 100644 testdata/p4_16_samples/issue1949-bmv2.p4 create mode 100644 testdata/p4_16_samples/name_annotations_to_p4info-bmv2.p4 diff --git a/testdata/p4_16_errors_outputs/multicast-bmv2-first.p4 b/testdata/p4_16_errors_outputs/multicast-bmv2-first.p4 new file mode 100644 index 00000000000..44fb930e118 --- /dev/null +++ b/testdata/p4_16_errors_outputs/multicast-bmv2-first.p4 @@ -0,0 +1,162 @@ +#include +#define V1MODEL_VERSION 20180101 +#include + +struct routing_metadata_t { + bit<32> nhop_ipv4; +} + +header ethernet_t { + bit<48> dstAddr; + bit<48> srcAddr; + bit<16> etherType; +} + +header ipv4_t { + bit<4> version; + bit<4> ihl; + bit<8> diffserv; + bit<16> totalLen; + bit<16> identification; + bit<3> flags; + bit<13> fragOffset; + bit<8> ttl; + bit<8> protocol; + bit<16> hdrChecksum; + bit<32> srcAddr; + bit<32> dstAddr; +} + +struct metadata { + @name("routing_metadata") + routing_metadata_t routing_metadata; +} + +struct headers { + @name("ethernet") + ethernet_t ethernet; + @name("ipv4") + ipv4_t ipv4; +} + +parser ParserImpl(packet_in packet, out headers hdr, inout metadata meta, inout standard_metadata_t standard_metadata) { + @name("parse_ethernet") state parse_ethernet { + packet.extract(hdr.ethernet); + transition select(hdr.ethernet.etherType) { + 16w0x800: parse_ipv4; + default: accept; + } + } + @name("parse_ipv4") state parse_ipv4 { + packet.extract(hdr.ipv4); + transition accept; + } + @name("start") state start { + transition parse_ethernet; + } +} + +control egress(inout headers hdr, inout metadata meta, inout standard_metadata_t standard_metadata) { + @name(".rewrite_mac") action rewrite_mac(bit<48> smac) { + hdr.ethernet.srcAddr = smac; + } + @name("._drop") action _drop() { + mark_to_drop(standard_metadata); + } + @name("send_frame") table send_frame { + actions = { + rewrite_mac(); + _drop(); + @defaultonly NoAction(); + } + key = { + standard_metadata.egress_port: exact @name("standard_metadata.egress_port"); + } + size = 256; + default_action = NoAction(); + } + apply { + send_frame.apply(); + } +} + +control ingress(inout headers hdr, inout metadata meta, inout standard_metadata_t standard_metadata) { + @name(".bcast") action bcast() { + standard_metadata.mcast_grp = 16w1; + } + @name(".set_dmac") action set_dmac(bit<48> dmac) { + hdr.ethernet.dstAddr = dmac; + } + @name("._drop") action _drop() { + mark_to_drop(standard_metadata); + } + @name(".set_nhop") action set_nhop(bit<32> nhop_ipv4, bit<9> port) { + meta.routing_metadata.nhop_ipv4 = nhop_ipv4; + standard_metadata.egress_spec = port; + hdr.ipv4.ttl = hdr.ipv4.ttl + 8w255; + } + @name(".set_nhop_redirect") action set_nhop_redirect(bit<32> nhop_ipv4, bit<9> port) { + meta.routing_metadata.nhop_ipv4 = nhop_ipv4; + standard_metadata.egress_spec = port; + hdr.ipv4.ttl = hdr.ipv4.ttl + 8w255; + } + @name("broadcast") table broadcast { + actions = { + bcast(); + @defaultonly NoAction(); + } + size = 1; + default_action = NoAction(); + } + @name("forward") table forward { + actions = { + set_dmac(); + _drop(); + @defaultonly NoAction(); + } + key = { + meta.routing_metadata.nhop_ipv4: exact @name("meta.routing_metadata.nhop_ipv4"); + } + size = 512; + default_action = NoAction(); + } + @name("ipv4_lpm") table ipv4_lpm { + actions = { + set_nhop(); + _drop(); + @defaultonly NoAction(); + } + key = { + hdr.ipv4.dstAddr: lpm @name("hdr.ipv4.dstAddr"); + } + size = 1024; + default_action = NoAction(); + } + apply { + if (hdr.ipv4.isValid()) { + ipv4_lpm.apply(); + forward.apply(); + } else { + broadcast.apply(); + } + } +} + +control DeparserImpl(packet_out packet, in headers hdr) { + apply { + packet.emit(hdr.ethernet); + packet.emit(hdr.ipv4); + } +} + +control verifyChecksum(inout headers hdr, inout metadata meta) { + apply { + } +} + +control computeChecksum(inout headers hdr, inout metadata meta) { + apply { + } +} + +V1Switch(ParserImpl(), verifyChecksum(), ingress(), egress(), computeChecksum(), DeparserImpl()) main; diff --git a/testdata/p4_16_errors_outputs/multicast-bmv2.p4 b/testdata/p4_16_errors_outputs/multicast-bmv2.p4 new file mode 100644 index 00000000000..fdf4220703c --- /dev/null +++ b/testdata/p4_16_errors_outputs/multicast-bmv2.p4 @@ -0,0 +1,154 @@ +#include +#define V1MODEL_VERSION 20180101 +#include + +struct routing_metadata_t { + bit<32> nhop_ipv4; +} + +header ethernet_t { + bit<48> dstAddr; + bit<48> srcAddr; + bit<16> etherType; +} + +header ipv4_t { + bit<4> version; + bit<4> ihl; + bit<8> diffserv; + bit<16> totalLen; + bit<16> identification; + bit<3> flags; + bit<13> fragOffset; + bit<8> ttl; + bit<8> protocol; + bit<16> hdrChecksum; + bit<32> srcAddr; + bit<32> dstAddr; +} + +struct metadata { + @name("routing_metadata") + routing_metadata_t routing_metadata; +} + +struct headers { + @name("ethernet") + ethernet_t ethernet; + @name("ipv4") + ipv4_t ipv4; +} + +parser ParserImpl(packet_in packet, out headers hdr, inout metadata meta, inout standard_metadata_t standard_metadata) { + @name("parse_ethernet") state parse_ethernet { + packet.extract(hdr.ethernet); + transition select(hdr.ethernet.etherType) { + 16w0x800: parse_ipv4; + default: accept; + } + } + @name("parse_ipv4") state parse_ipv4 { + packet.extract(hdr.ipv4); + transition accept; + } + @name("start") state start { + transition parse_ethernet; + } +} + +control egress(inout headers hdr, inout metadata meta, inout standard_metadata_t standard_metadata) { + @name(".rewrite_mac") action rewrite_mac(bit<48> smac) { + hdr.ethernet.srcAddr = smac; + } + @name("._drop") action _drop() { + mark_to_drop(standard_metadata); + } + @name("send_frame") table send_frame { + actions = { + rewrite_mac; + _drop; + } + key = { + standard_metadata.egress_port: exact; + } + size = 256; + } + apply { + send_frame.apply(); + } +} + +control ingress(inout headers hdr, inout metadata meta, inout standard_metadata_t standard_metadata) { + @name(".bcast") action bcast() { + standard_metadata.mcast_grp = 1; + } + @name(".set_dmac") action set_dmac(bit<48> dmac) { + hdr.ethernet.dstAddr = dmac; + } + @name("._drop") action _drop() { + mark_to_drop(standard_metadata); + } + @name(".set_nhop") action set_nhop(bit<32> nhop_ipv4, bit<9> port) { + meta.routing_metadata.nhop_ipv4 = nhop_ipv4; + standard_metadata.egress_spec = port; + hdr.ipv4.ttl = hdr.ipv4.ttl + 8w255; + } + @name(".set_nhop_redirect") action set_nhop_redirect(bit<32> nhop_ipv4, bit<9> port) { + meta.routing_metadata.nhop_ipv4 = nhop_ipv4; + standard_metadata.egress_spec = port; + hdr.ipv4.ttl = hdr.ipv4.ttl + 8w255; + } + @name("broadcast") table broadcast { + actions = { + bcast; + } + size = 1; + } + @name("forward") table forward { + actions = { + set_dmac; + _drop; + } + key = { + meta.routing_metadata.nhop_ipv4: exact; + } + size = 512; + } + @name("ipv4_lpm") table ipv4_lpm { + actions = { + set_nhop; + _drop; + } + key = { + hdr.ipv4.dstAddr: lpm; + } + size = 1024; + } + apply { + if (hdr.ipv4.isValid()) { + ipv4_lpm.apply(); + forward.apply(); + } else { + broadcast.apply(); + } + } +} + +control DeparserImpl(packet_out packet, in headers hdr) { + apply { + packet.emit(hdr.ethernet); + packet.emit(hdr.ipv4); + } +} + +control verifyChecksum(inout headers hdr, inout metadata meta) { + apply { + } +} + +control computeChecksum(inout headers hdr, inout metadata meta) { + apply { + } +} + +V1Switch(ParserImpl(), verifyChecksum(), ingress(), egress(), computeChecksum(), DeparserImpl()) main; diff --git a/testdata/p4_16_samples/issue1949-bmv2.p4 b/testdata/p4_16_samples/issue1949-bmv2.p4 new file mode 100644 index 00000000000..5eb22a115a7 --- /dev/null +++ b/testdata/p4_16_samples/issue1949-bmv2.p4 @@ -0,0 +1,84 @@ +/* +Copyright 2019 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 +#include + +header h1_t { + bit<8> f1; + bit<8> f2; +} + +struct headers_t { + h1_t h1; +} + +struct metadata_t { +} + +parser parserImpl(packet_in packet, + out headers_t hdr, + inout metadata_t meta, + inout standard_metadata_t stdmeta) +{ + state start { + packet.extract(hdr.h1); + transition accept; + } +} + +control ingressImpl(inout headers_t hdr, + inout metadata_t meta, + inout standard_metadata_t stdmeta) +{ + @name(".foo") action act1() { + hdr.h1.f1 = hdr.h1.f1 >> 2; + } + @name(".foo") action act2() { + hdr.h1.f1 = hdr.h1.f1 << 3; + } + table t1 { + key = { hdr.h1.f1 : exact; } + actions = { act1; act2; NoAction; } + const default_action = NoAction; + } + apply { + t1.apply(); + } +} + +control verifyChecksum(inout headers_t hdr, inout metadata_t meta) { + 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 packet, in headers_t hdr) { + apply { + } +} + +V1Switch(parserImpl(), verifyChecksum(), ingressImpl(), egressImpl(), updateChecksum(), deparserImpl()) main; + diff --git a/testdata/p4_16_samples/name_annotations_to_p4info-bmv2.p4 b/testdata/p4_16_samples/name_annotations_to_p4info-bmv2.p4 new file mode 100644 index 00000000000..bb1f7627cc0 --- /dev/null +++ b/testdata/p4_16_samples/name_annotations_to_p4info-bmv2.p4 @@ -0,0 +1,69 @@ +/* +Copyright 2021 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. +*/ + +#include +#include + +struct H { }; +struct M { + bit<32> hash1; +} + +parser ParserI(packet_in pk, out H hdr, inout M meta, inout standard_metadata_t smeta) { + state start { transition accept; } +} + +@name("altIngressName") +control IngressI(inout H hdr, inout M meta, inout standard_metadata_t smeta) { + @name("blue") + action foo(@name("orange") bit<9> port) { + smeta.egress_spec = port; + } + + @name("red") + table foo_table { + key = { + smeta.ingress_port : exact @name("green"); + } + actions = { + @name("yellow") foo; + @name("grey") NoAction; + } + const default_action = NoAction(); + } + apply { + foo_table.apply(); + } +}; + +control EgressI(inout H hdr, inout M meta, inout standard_metadata_t smeta) { + apply { } +}; + +control DeparserI(packet_out pk, in H hdr) { + apply { } +} + +control VerifyChecksumI(inout H hdr, inout M meta) { + apply { } +} + +control ComputeChecksumI(inout H hdr, inout M meta) { + apply { } +} + +V1Switch(ParserI(), VerifyChecksumI(), IngressI(), EgressI(), + ComputeChecksumI(), DeparserI()) main; diff --git a/testdata/p4_16_samples_outputs/multicast-bmv2-first.p4 b/testdata/p4_16_samples_outputs/multicast-bmv2-first.p4 index 44fb930e118..549f83c13a1 100644 --- a/testdata/p4_16_samples_outputs/multicast-bmv2-first.p4 +++ b/testdata/p4_16_samples_outputs/multicast-bmv2-first.p4 @@ -60,7 +60,7 @@ control egress(inout headers hdr, inout metadata meta, inout standard_metadata_t @name(".rewrite_mac") action rewrite_mac(bit<48> smac) { hdr.ethernet.srcAddr = smac; } - @name("._drop") action _drop() { + @name("_drop") action _drop() { mark_to_drop(standard_metadata); } @name("send_frame") table send_frame { @@ -87,7 +87,7 @@ control ingress(inout headers hdr, inout metadata meta, inout standard_metadata_ @name(".set_dmac") action set_dmac(bit<48> dmac) { hdr.ethernet.dstAddr = dmac; } - @name("._drop") action _drop() { + @name("_drop") action _drop() { mark_to_drop(standard_metadata); } @name(".set_nhop") action set_nhop(bit<32> nhop_ipv4, bit<9> port) { diff --git a/testdata/p4_16_samples_outputs/multicast-bmv2-frontend.p4 b/testdata/p4_16_samples_outputs/multicast-bmv2-frontend.p4 index 95148afd70f..25a2fed91b6 100644 --- a/testdata/p4_16_samples_outputs/multicast-bmv2-frontend.p4 +++ b/testdata/p4_16_samples_outputs/multicast-bmv2-frontend.p4 @@ -62,7 +62,7 @@ control egress(inout headers hdr, inout metadata meta, inout standard_metadata_t @name(".rewrite_mac") action rewrite_mac(@name("smac") bit<48> smac) { hdr.ethernet.srcAddr = smac; } - @name("._drop") action _drop() { + @name("egress._drop") action _drop() { mark_to_drop(standard_metadata); } @name("egress.send_frame") table send_frame_0 { @@ -95,10 +95,10 @@ control ingress(inout headers hdr, inout metadata meta, inout standard_metadata_ @name(".set_dmac") action set_dmac(@name("dmac") bit<48> dmac) { hdr.ethernet.dstAddr = dmac; } - @name("._drop") action _drop_2() { + @name("ingress._drop") action _drop_2() { mark_to_drop(standard_metadata); } - @name("._drop") action _drop_3() { + @name("ingress._drop") action _drop_3() { mark_to_drop(standard_metadata); } @name(".set_nhop") action set_nhop(@name("nhop_ipv4") bit<32> nhop_ipv4_1, @name("port") bit<9> port) { diff --git a/testdata/p4_16_samples_outputs/multicast-bmv2-midend.p4 b/testdata/p4_16_samples_outputs/multicast-bmv2-midend.p4 index 10afd1f4bb2..7845e575bc9 100644 --- a/testdata/p4_16_samples_outputs/multicast-bmv2-midend.p4 +++ b/testdata/p4_16_samples_outputs/multicast-bmv2-midend.p4 @@ -61,7 +61,7 @@ control egress(inout headers hdr, inout metadata meta, inout standard_metadata_t @name(".rewrite_mac") action rewrite_mac(@name("smac") bit<48> smac) { hdr.ethernet.srcAddr = smac; } - @name("._drop") action _drop() { + @name("egress._drop") action _drop() { mark_to_drop(standard_metadata); } @name("egress.send_frame") table send_frame_0 { @@ -94,10 +94,10 @@ control ingress(inout headers hdr, inout metadata meta, inout standard_metadata_ @name(".set_dmac") action set_dmac(@name("dmac") bit<48> dmac) { hdr.ethernet.dstAddr = dmac; } - @name("._drop") action _drop_2() { + @name("ingress._drop") action _drop_2() { mark_to_drop(standard_metadata); } - @name("._drop") action _drop_3() { + @name("ingress._drop") action _drop_3() { mark_to_drop(standard_metadata); } @name(".set_nhop") action set_nhop(@name("nhop_ipv4") bit<32> nhop_ipv4_1, @name("port") bit<9> port) { diff --git a/testdata/p4_16_samples_outputs/multicast-bmv2.p4 b/testdata/p4_16_samples_outputs/multicast-bmv2.p4 index fdf4220703c..f608cc80b0f 100644 --- a/testdata/p4_16_samples_outputs/multicast-bmv2.p4 +++ b/testdata/p4_16_samples_outputs/multicast-bmv2.p4 @@ -60,7 +60,7 @@ control egress(inout headers hdr, inout metadata meta, inout standard_metadata_t @name(".rewrite_mac") action rewrite_mac(bit<48> smac) { hdr.ethernet.srcAddr = smac; } - @name("._drop") action _drop() { + @name("_drop") action _drop() { mark_to_drop(standard_metadata); } @name("send_frame") table send_frame { @@ -85,7 +85,7 @@ control ingress(inout headers hdr, inout metadata meta, inout standard_metadata_ @name(".set_dmac") action set_dmac(bit<48> dmac) { hdr.ethernet.dstAddr = dmac; } - @name("._drop") action _drop() { + @name("_drop") action _drop() { mark_to_drop(standard_metadata); } @name(".set_nhop") action set_nhop(bit<32> nhop_ipv4, bit<9> port) { diff --git a/testdata/p4_16_samples_outputs/multicast-bmv2.p4.p4info.txtpb b/testdata/p4_16_samples_outputs/multicast-bmv2.p4.p4info.txtpb index eb21c0eabd8..c22ccb32dbb 100644 --- a/testdata/p4_16_samples_outputs/multicast-bmv2.p4.p4info.txtpb +++ b/testdata/p4_16_samples_outputs/multicast-bmv2.p4.p4info.txtpb @@ -39,7 +39,7 @@ tables { id: 25234447 } action_refs { - id: 19143480 + id: 19344232 } action_refs { id: 21257015 @@ -67,7 +67,7 @@ tables { id: 23300268 } action_refs { - id: 19143480 + id: 19344232 } action_refs { id: 21257015 @@ -95,7 +95,7 @@ tables { id: 31165400 } action_refs { - id: 19143480 + id: 25850727 } action_refs { id: 21257015 @@ -136,9 +136,9 @@ actions { } actions { preamble { - id: 19143480 - name: "_drop" - alias: "_drop" + id: 19344232 + name: "ingress._drop" + alias: "ingress._drop" } } actions { @@ -170,5 +170,12 @@ actions { bitwidth: 48 } } +actions { + preamble { + id: 25850727 + name: "egress._drop" + alias: "egress._drop" + } +} type_info { } diff --git a/testdata/p4_16_samples_outputs/named_meter_1-bmv2-first.p4 b/testdata/p4_16_samples_outputs/named_meter_1-bmv2-first.p4 index 2d70d5b316e..b4b618b7596 100644 --- a/testdata/p4_16_samples_outputs/named_meter_1-bmv2-first.p4 +++ b/testdata/p4_16_samples_outputs/named_meter_1-bmv2-first.p4 @@ -60,11 +60,11 @@ control ingress(inout headers hdr, inout metadata meta, inout standard_metadata_ size = 16; default_action = NoAction(); } - @name("m_action") action m_action_0(bit<9> meter_idx) { + @name("m_action_2") action m_action_0(bit<9> meter_idx) { standard_metadata.egress_spec = meter_idx; my_meter.read(meta.meta.meter_tag); } - @name("_nop") action _nop_0() { + @name("_nop_2") action _nop_0() { my_meter.read(meta.meta.meter_tag); } @name("m_table") table m_table { diff --git a/testdata/p4_16_samples_outputs/named_meter_1-bmv2-frontend.p4 b/testdata/p4_16_samples_outputs/named_meter_1-bmv2-frontend.p4 index 551a3b2c470..2d08c566a81 100644 --- a/testdata/p4_16_samples_outputs/named_meter_1-bmv2-frontend.p4 +++ b/testdata/p4_16_samples_outputs/named_meter_1-bmv2-frontend.p4 @@ -60,11 +60,11 @@ control ingress(inout headers hdr, inout metadata meta, inout standard_metadata_ size = 16; default_action = NoAction_1(); } - @name("ingress.m_action") action m_action_0(@name("meter_idx") bit<9> meter_idx) { + @name("ingress.m_action_2") action m_action_0(@name("meter_idx") bit<9> meter_idx) { standard_metadata.egress_spec = meter_idx; my_meter.read(meta.meta.meter_tag); } - @name("ingress._nop") action _nop_0() { + @name("ingress._nop_2") action _nop_0() { my_meter.read(meta.meta.meter_tag); } @name("ingress.m_table") table m_table_0 { diff --git a/testdata/p4_16_samples_outputs/named_meter_1-bmv2-midend.p4 b/testdata/p4_16_samples_outputs/named_meter_1-bmv2-midend.p4 index 716d31df623..fe5f2004863 100644 --- a/testdata/p4_16_samples_outputs/named_meter_1-bmv2-midend.p4 +++ b/testdata/p4_16_samples_outputs/named_meter_1-bmv2-midend.p4 @@ -59,11 +59,11 @@ control ingress(inout headers hdr, inout metadata meta, inout standard_metadata_ size = 16; default_action = NoAction_1(); } - @name("ingress.m_action") action m_action_0(@name("meter_idx") bit<9> meter_idx) { + @name("ingress.m_action_2") action m_action_0(@name("meter_idx") bit<9> meter_idx) { standard_metadata.egress_spec = meter_idx; my_meter.read(meta._meta_meter_tag0); } - @name("ingress._nop") action _nop_0() { + @name("ingress._nop_2") action _nop_0() { my_meter.read(meta._meta_meter_tag0); } @name("ingress.m_table") table m_table_0 { diff --git a/testdata/p4_16_samples_outputs/named_meter_1-bmv2.p4 b/testdata/p4_16_samples_outputs/named_meter_1-bmv2.p4 index a9544c63639..75cfab9d1a0 100644 --- a/testdata/p4_16_samples_outputs/named_meter_1-bmv2.p4 +++ b/testdata/p4_16_samples_outputs/named_meter_1-bmv2.p4 @@ -60,11 +60,11 @@ control ingress(inout headers hdr, inout metadata meta, inout standard_metadata_ size = 16; default_action = NoAction(); } - @name("m_action") action m_action_0(bit<9> meter_idx) { + @name("m_action_2") action m_action_0(bit<9> meter_idx) { standard_metadata.egress_spec = meter_idx; my_meter.read(meta.meta.meter_tag); } - @name("_nop") action _nop_0() { + @name("_nop_2") action _nop_0() { my_meter.read(meta.meta.meter_tag); } @name("m_table") table m_table { diff --git a/testdata/p4_16_samples_outputs/named_meter_1-bmv2.p4.p4info.txtpb b/testdata/p4_16_samples_outputs/named_meter_1-bmv2.p4.p4info.txtpb index 398c9670544..5fd57eb7b5b 100644 --- a/testdata/p4_16_samples_outputs/named_meter_1-bmv2.p4.p4info.txtpb +++ b/testdata/p4_16_samples_outputs/named_meter_1-bmv2.p4.p4info.txtpb @@ -43,10 +43,10 @@ tables { match_type: EXACT } action_refs { - id: 24512981 + id: 18971545 } action_refs { - id: 26939440 + id: 27693640 } action_refs { id: 21257015 @@ -81,9 +81,9 @@ actions { } actions { preamble { - id: 24512981 - name: "ingress.m_action" - alias: "m_action" + id: 18971545 + name: "ingress.m_action_2" + alias: "m_action_2" } params { id: 1 @@ -91,6 +91,13 @@ actions { bitwidth: 9 } } +actions { + preamble { + id: 27693640 + name: "ingress._nop_2" + alias: "_nop_2" + } +} direct_meters { preamble { id: 368209065 diff --git a/testdata/p4_16_samples_outputs/named_meter_bmv2-first.p4 b/testdata/p4_16_samples_outputs/named_meter_bmv2-first.p4 index 4234a05c5d6..08560c03e6f 100644 --- a/testdata/p4_16_samples_outputs/named_meter_bmv2-first.p4 +++ b/testdata/p4_16_samples_outputs/named_meter_bmv2-first.p4 @@ -60,12 +60,12 @@ control ingress(inout headers hdr, inout metadata meta, inout standard_metadata_ size = 16; default_action = NoAction(); } - @name("m_action") action m_action_0(bit<9> meter_idx) { + @name("m_action_2") action m_action_0(bit<9> meter_idx) { standard_metadata.egress_spec = meter_idx; standard_metadata.egress_spec = 9w1; my_meter.read(meta.meta.meter_tag); } - @name("_nop") action _nop_0() { + @name("_nop_2") action _nop_0() { my_meter.read(meta.meta.meter_tag); } @name("m_table") table m_table { diff --git a/testdata/p4_16_samples_outputs/named_meter_bmv2-frontend.p4 b/testdata/p4_16_samples_outputs/named_meter_bmv2-frontend.p4 index 8ca214c1191..49b7140c8f3 100644 --- a/testdata/p4_16_samples_outputs/named_meter_bmv2-frontend.p4 +++ b/testdata/p4_16_samples_outputs/named_meter_bmv2-frontend.p4 @@ -60,11 +60,11 @@ control ingress(inout headers hdr, inout metadata meta, inout standard_metadata_ size = 16; default_action = NoAction_1(); } - @name("ingress.m_action") action m_action_0(@name("meter_idx") bit<9> meter_idx) { + @name("ingress.m_action_2") action m_action_0(@name("meter_idx") bit<9> meter_idx) { standard_metadata.egress_spec = 9w1; my_meter_0.read(meta.meta.meter_tag); } - @name("ingress._nop") action _nop_0() { + @name("ingress._nop_2") action _nop_0() { my_meter_0.read(meta.meta.meter_tag); } @name("ingress.m_table") table m_table_0 { diff --git a/testdata/p4_16_samples_outputs/named_meter_bmv2-midend.p4 b/testdata/p4_16_samples_outputs/named_meter_bmv2-midend.p4 index 0bdd39f7a1c..7c2b71cd7a1 100644 --- a/testdata/p4_16_samples_outputs/named_meter_bmv2-midend.p4 +++ b/testdata/p4_16_samples_outputs/named_meter_bmv2-midend.p4 @@ -59,11 +59,11 @@ control ingress(inout headers hdr, inout metadata meta, inout standard_metadata_ size = 16; default_action = NoAction_1(); } - @name("ingress.m_action") action m_action_0(@name("meter_idx") bit<9> meter_idx) { + @name("ingress.m_action_2") action m_action_0(@name("meter_idx") bit<9> meter_idx) { standard_metadata.egress_spec = 9w1; my_meter_0.read(meta._meta_meter_tag0); } - @name("ingress._nop") action _nop_0() { + @name("ingress._nop_2") action _nop_0() { my_meter_0.read(meta._meta_meter_tag0); } @name("ingress.m_table") table m_table_0 { diff --git a/testdata/p4_16_samples_outputs/named_meter_bmv2.p4 b/testdata/p4_16_samples_outputs/named_meter_bmv2.p4 index 3f53812af01..c6f394e2cd4 100644 --- a/testdata/p4_16_samples_outputs/named_meter_bmv2.p4 +++ b/testdata/p4_16_samples_outputs/named_meter_bmv2.p4 @@ -60,12 +60,12 @@ control ingress(inout headers hdr, inout metadata meta, inout standard_metadata_ size = 16; default_action = NoAction(); } - @name("m_action") action m_action_0(bit<9> meter_idx) { + @name("m_action_2") action m_action_0(bit<9> meter_idx) { standard_metadata.egress_spec = meter_idx; standard_metadata.egress_spec = 9w1; my_meter.read(meta.meta.meter_tag); } - @name("_nop") action _nop_0() { + @name("_nop_2") action _nop_0() { my_meter.read(meta.meta.meter_tag); } @name("m_table") table m_table { diff --git a/testdata/p4_16_samples_outputs/named_meter_bmv2.p4.p4info.txtpb b/testdata/p4_16_samples_outputs/named_meter_bmv2.p4.p4info.txtpb index 30a57fc6528..c351804ae74 100644 --- a/testdata/p4_16_samples_outputs/named_meter_bmv2.p4.p4info.txtpb +++ b/testdata/p4_16_samples_outputs/named_meter_bmv2.p4.p4info.txtpb @@ -43,10 +43,10 @@ tables { match_type: EXACT } action_refs { - id: 24512981 + id: 18971545 } action_refs { - id: 26939440 + id: 27693640 } action_refs { id: 21257015 @@ -81,9 +81,9 @@ actions { } actions { preamble { - id: 24512981 - name: "ingress.m_action" - alias: "m_action" + id: 18971545 + name: "ingress.m_action_2" + alias: "m_action_2" } params { id: 1 @@ -91,6 +91,13 @@ actions { bitwidth: 9 } } +actions { + preamble { + id: 27693640 + name: "ingress._nop_2" + alias: "_nop_2" + } +} direct_meters { preamble { id: 354402025 From 9acb7e0a23a54c007f28f0bbaa59d1ac4565f7be Mon Sep 17 00:00:00 2001 From: Andy Fingerhut Date: Tue, 8 Oct 2024 22:01:00 -0700 Subject: [PATCH 11/48] Move new test program with error into p4_16_errors directory Signed-off-by: Andy Fingerhut --- testdata/{p4_16_samples => p4_16_errors}/issue1949-bmv2.p4 | 0 1 file changed, 0 insertions(+), 0 deletions(-) rename testdata/{p4_16_samples => p4_16_errors}/issue1949-bmv2.p4 (100%) diff --git a/testdata/p4_16_samples/issue1949-bmv2.p4 b/testdata/p4_16_errors/issue1949-bmv2.p4 similarity index 100% rename from testdata/p4_16_samples/issue1949-bmv2.p4 rename to testdata/p4_16_errors/issue1949-bmv2.p4 From b133b281f35d668bcec2ff4588782edcb8d15afa Mon Sep 17 00:00:00 2001 From: Andy Fingerhut Date: Tue, 8 Oct 2024 22:08:41 -0700 Subject: [PATCH 12/48] Add expected output files for new test programs Signed-off-by: Andy Fingerhut --- .../issue1949-bmv2-first.p4 | 67 +++++++++++++++++++ .../p4_16_errors_outputs/issue1949-bmv2.p4 | 67 +++++++++++++++++++ .../issue1949-bmv2.p4-stderr | 6 ++ .../name_annotations_to_p4info-bmv2-first.p4 | 57 ++++++++++++++++ ...ame_annotations_to_p4info-bmv2-frontend.p4 | 59 ++++++++++++++++ .../name_annotations_to_p4info-bmv2-midend.p4 | 59 ++++++++++++++++ .../name_annotations_to_p4info-bmv2.p4 | 57 ++++++++++++++++ .../name_annotations_to_p4info-bmv2.p4-stderr | 0 ...nnotations_to_p4info-bmv2.p4.entries.txtpb | 3 + ...annotations_to_p4info-bmv2.p4.p4info.txtpb | 52 ++++++++++++++ 10 files changed, 427 insertions(+) create mode 100644 testdata/p4_16_errors_outputs/issue1949-bmv2-first.p4 create mode 100644 testdata/p4_16_errors_outputs/issue1949-bmv2.p4 create mode 100644 testdata/p4_16_errors_outputs/issue1949-bmv2.p4-stderr create mode 100644 testdata/p4_16_samples_outputs/name_annotations_to_p4info-bmv2-first.p4 create mode 100644 testdata/p4_16_samples_outputs/name_annotations_to_p4info-bmv2-frontend.p4 create mode 100644 testdata/p4_16_samples_outputs/name_annotations_to_p4info-bmv2-midend.p4 create mode 100644 testdata/p4_16_samples_outputs/name_annotations_to_p4info-bmv2.p4 create mode 100644 testdata/p4_16_samples_outputs/name_annotations_to_p4info-bmv2.p4-stderr create mode 100644 testdata/p4_16_samples_outputs/name_annotations_to_p4info-bmv2.p4.entries.txtpb create mode 100644 testdata/p4_16_samples_outputs/name_annotations_to_p4info-bmv2.p4.p4info.txtpb diff --git a/testdata/p4_16_errors_outputs/issue1949-bmv2-first.p4 b/testdata/p4_16_errors_outputs/issue1949-bmv2-first.p4 new file mode 100644 index 00000000000..01dee47f960 --- /dev/null +++ b/testdata/p4_16_errors_outputs/issue1949-bmv2-first.p4 @@ -0,0 +1,67 @@ +#include +#define V1MODEL_VERSION 20180101 +#include + +header h1_t { + bit<8> f1; + bit<8> f2; +} + +struct headers_t { + h1_t h1; +} + +struct metadata_t { +} + +parser parserImpl(packet_in packet, out headers_t hdr, inout metadata_t meta, inout standard_metadata_t stdmeta) { + state start { + packet.extract(hdr.h1); + transition accept; + } +} + +control ingressImpl(inout headers_t hdr, inout metadata_t meta, inout standard_metadata_t stdmeta) { + @name(".foo") action act1() { + hdr.h1.f1 = hdr.h1.f1 >> 2; + } + @name(".foo") action act2() { + hdr.h1.f1 = hdr.h1.f1 << 3; + } + table t1 { + key = { + hdr.h1.f1: exact @name("hdr.h1.f1"); + } + actions = { + act1(); + act2(); + NoAction(); + } + const default_action = NoAction(); + } + apply { + t1.apply(); + } +} + +control verifyChecksum(inout headers_t hdr, inout metadata_t meta) { + 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 packet, in headers_t hdr) { + apply { + } +} + +V1Switch(parserImpl(), verifyChecksum(), ingressImpl(), egressImpl(), updateChecksum(), deparserImpl()) main; diff --git a/testdata/p4_16_errors_outputs/issue1949-bmv2.p4 b/testdata/p4_16_errors_outputs/issue1949-bmv2.p4 new file mode 100644 index 00000000000..3255df59142 --- /dev/null +++ b/testdata/p4_16_errors_outputs/issue1949-bmv2.p4 @@ -0,0 +1,67 @@ +#include +#define V1MODEL_VERSION 20180101 +#include + +header h1_t { + bit<8> f1; + bit<8> f2; +} + +struct headers_t { + h1_t h1; +} + +struct metadata_t { +} + +parser parserImpl(packet_in packet, out headers_t hdr, inout metadata_t meta, inout standard_metadata_t stdmeta) { + state start { + packet.extract(hdr.h1); + transition accept; + } +} + +control ingressImpl(inout headers_t hdr, inout metadata_t meta, inout standard_metadata_t stdmeta) { + @name(".foo") action act1() { + hdr.h1.f1 = hdr.h1.f1 >> 2; + } + @name(".foo") action act2() { + hdr.h1.f1 = hdr.h1.f1 << 3; + } + table t1 { + key = { + hdr.h1.f1: exact; + } + actions = { + act1; + act2; + NoAction; + } + const default_action = NoAction; + } + apply { + t1.apply(); + } +} + +control verifyChecksum(inout headers_t hdr, inout metadata_t meta) { + 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 packet, in headers_t hdr) { + apply { + } +} + +V1Switch(parserImpl(), verifyChecksum(), ingressImpl(), egressImpl(), updateChecksum(), deparserImpl()) main; diff --git a/testdata/p4_16_errors_outputs/issue1949-bmv2.p4-stderr b/testdata/p4_16_errors_outputs/issue1949-bmv2.p4-stderr new file mode 100644 index 00000000000..f0f3a46d7fb --- /dev/null +++ b/testdata/p4_16_errors_outputs/issue1949-bmv2.p4-stderr @@ -0,0 +1,6 @@ +issue1949-bmv2.p4(50): [--Werror=duplicate] error: .foo: conflicting control plane name: .foo + @name(".foo") action act2() { + ^^^^ +issue1949-bmv2.p4(47) + @name(".foo") action act1() { + ^^^^ diff --git a/testdata/p4_16_samples_outputs/name_annotations_to_p4info-bmv2-first.p4 b/testdata/p4_16_samples_outputs/name_annotations_to_p4info-bmv2-first.p4 new file mode 100644 index 00000000000..7e294ece67a --- /dev/null +++ b/testdata/p4_16_samples_outputs/name_annotations_to_p4info-bmv2-first.p4 @@ -0,0 +1,57 @@ +#include +#define V1MODEL_VERSION 20180101 +#include + +struct H { +} + +struct M { + bit<32> hash1; +} + +parser ParserI(packet_in pk, out H hdr, inout M meta, inout standard_metadata_t smeta) { + state start { + transition accept; + } +} + +@name("altIngressName") control IngressI(inout H hdr, inout M meta, inout standard_metadata_t smeta) { + @name("blue") action foo(@name("orange") bit<9> port) { + smeta.egress_spec = port; + } + @name("red") table foo_table { + key = { + smeta.ingress_port: exact @name("green"); + } + actions = { + @name("yellow") foo(); + @name("grey") NoAction(); + } + const default_action = NoAction(); + } + apply { + foo_table.apply(); + } +} + +control EgressI(inout H hdr, inout M meta, inout standard_metadata_t smeta) { + apply { + } +} + +control DeparserI(packet_out pk, in H hdr) { + apply { + } +} + +control VerifyChecksumI(inout H hdr, inout M meta) { + apply { + } +} + +control ComputeChecksumI(inout H hdr, inout M meta) { + apply { + } +} + +V1Switch(ParserI(), VerifyChecksumI(), IngressI(), EgressI(), ComputeChecksumI(), DeparserI()) main; diff --git a/testdata/p4_16_samples_outputs/name_annotations_to_p4info-bmv2-frontend.p4 b/testdata/p4_16_samples_outputs/name_annotations_to_p4info-bmv2-frontend.p4 new file mode 100644 index 00000000000..8b83caba873 --- /dev/null +++ b/testdata/p4_16_samples_outputs/name_annotations_to_p4info-bmv2-frontend.p4 @@ -0,0 +1,59 @@ +#include +#define V1MODEL_VERSION 20180101 +#include + +struct H { +} + +struct M { + bit<32> hash1; +} + +parser ParserI(packet_in pk, out H hdr, inout M meta, inout standard_metadata_t smeta) { + state start { + transition accept; + } +} + +@name("IngressI.altIngressName") control IngressI(inout H hdr, inout M meta, inout standard_metadata_t smeta) { + @noWarn("unused") @name(".NoAction") action NoAction_1() { + } + @name("IngressI.blue") action foo(@name("orange") bit<9> port) { + smeta.egress_spec = port; + } + @name("IngressI.red") table foo_table_0 { + key = { + smeta.ingress_port: exact @name("green"); + } + actions = { + @name("yellow") foo(); + @name("grey") NoAction_1(); + } + const default_action = NoAction_1(); + } + apply { + foo_table_0.apply(); + } +} + +control EgressI(inout H hdr, inout M meta, inout standard_metadata_t smeta) { + apply { + } +} + +control DeparserI(packet_out pk, in H hdr) { + apply { + } +} + +control VerifyChecksumI(inout H hdr, inout M meta) { + apply { + } +} + +control ComputeChecksumI(inout H hdr, inout M meta) { + apply { + } +} + +V1Switch(ParserI(), VerifyChecksumI(), IngressI(), EgressI(), ComputeChecksumI(), DeparserI()) main; diff --git a/testdata/p4_16_samples_outputs/name_annotations_to_p4info-bmv2-midend.p4 b/testdata/p4_16_samples_outputs/name_annotations_to_p4info-bmv2-midend.p4 new file mode 100644 index 00000000000..8b83caba873 --- /dev/null +++ b/testdata/p4_16_samples_outputs/name_annotations_to_p4info-bmv2-midend.p4 @@ -0,0 +1,59 @@ +#include +#define V1MODEL_VERSION 20180101 +#include + +struct H { +} + +struct M { + bit<32> hash1; +} + +parser ParserI(packet_in pk, out H hdr, inout M meta, inout standard_metadata_t smeta) { + state start { + transition accept; + } +} + +@name("IngressI.altIngressName") control IngressI(inout H hdr, inout M meta, inout standard_metadata_t smeta) { + @noWarn("unused") @name(".NoAction") action NoAction_1() { + } + @name("IngressI.blue") action foo(@name("orange") bit<9> port) { + smeta.egress_spec = port; + } + @name("IngressI.red") table foo_table_0 { + key = { + smeta.ingress_port: exact @name("green"); + } + actions = { + @name("yellow") foo(); + @name("grey") NoAction_1(); + } + const default_action = NoAction_1(); + } + apply { + foo_table_0.apply(); + } +} + +control EgressI(inout H hdr, inout M meta, inout standard_metadata_t smeta) { + apply { + } +} + +control DeparserI(packet_out pk, in H hdr) { + apply { + } +} + +control VerifyChecksumI(inout H hdr, inout M meta) { + apply { + } +} + +control ComputeChecksumI(inout H hdr, inout M meta) { + apply { + } +} + +V1Switch(ParserI(), VerifyChecksumI(), IngressI(), EgressI(), ComputeChecksumI(), DeparserI()) main; diff --git a/testdata/p4_16_samples_outputs/name_annotations_to_p4info-bmv2.p4 b/testdata/p4_16_samples_outputs/name_annotations_to_p4info-bmv2.p4 new file mode 100644 index 00000000000..1ef534477dd --- /dev/null +++ b/testdata/p4_16_samples_outputs/name_annotations_to_p4info-bmv2.p4 @@ -0,0 +1,57 @@ +#include +#define V1MODEL_VERSION 20180101 +#include + +struct H { +} + +struct M { + bit<32> hash1; +} + +parser ParserI(packet_in pk, out H hdr, inout M meta, inout standard_metadata_t smeta) { + state start { + transition accept; + } +} + +@name("altIngressName") control IngressI(inout H hdr, inout M meta, inout standard_metadata_t smeta) { + @name("blue") action foo(@name("orange") bit<9> port) { + smeta.egress_spec = port; + } + @name("red") table foo_table { + key = { + smeta.ingress_port: exact @name("green"); + } + actions = { + @name("yellow") foo; + @name("grey") NoAction; + } + const default_action = NoAction(); + } + apply { + foo_table.apply(); + } +} + +control EgressI(inout H hdr, inout M meta, inout standard_metadata_t smeta) { + apply { + } +} + +control DeparserI(packet_out pk, in H hdr) { + apply { + } +} + +control VerifyChecksumI(inout H hdr, inout M meta) { + apply { + } +} + +control ComputeChecksumI(inout H hdr, inout M meta) { + apply { + } +} + +V1Switch(ParserI(), VerifyChecksumI(), IngressI(), EgressI(), ComputeChecksumI(), DeparserI()) main; diff --git a/testdata/p4_16_samples_outputs/name_annotations_to_p4info-bmv2.p4-stderr b/testdata/p4_16_samples_outputs/name_annotations_to_p4info-bmv2.p4-stderr new file mode 100644 index 00000000000..e69de29bb2d diff --git a/testdata/p4_16_samples_outputs/name_annotations_to_p4info-bmv2.p4.entries.txtpb b/testdata/p4_16_samples_outputs/name_annotations_to_p4info-bmv2.p4.entries.txtpb new file mode 100644 index 00000000000..5cb9652623a --- /dev/null +++ b/testdata/p4_16_samples_outputs/name_annotations_to_p4info-bmv2.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/name_annotations_to_p4info-bmv2.p4.p4info.txtpb b/testdata/p4_16_samples_outputs/name_annotations_to_p4info-bmv2.p4.p4info.txtpb new file mode 100644 index 00000000000..f9668fdcbe1 --- /dev/null +++ b/testdata/p4_16_samples_outputs/name_annotations_to_p4info-bmv2.p4.p4info.txtpb @@ -0,0 +1,52 @@ +# proto-file: p4/config/v1/p4info.proto +# proto-message: p4.config.v1.P4Info + +pkg_info { + arch: "v1model" +} +tables { + preamble { + id: 49805857 + name: "IngressI.red" + alias: "red" + } + match_fields { + id: 1 + name: "green" + bitwidth: 9 + match_type: EXACT + } + action_refs { + id: 23079077 + } + action_refs { + id: 21257015 + } + const_default_action_id: 21257015 + initial_default_action { + action_id: 21257015 + } + size: 1024 +} +actions { + preamble { + id: 21257015 + name: "NoAction" + alias: "NoAction" + annotations: "@noWarn(\"unused\")" + } +} +actions { + preamble { + id: 23079077 + name: "IngressI.blue" + alias: "blue" + } + params { + id: 1 + name: "orange" + bitwidth: 9 + } +} +type_info { +} From 76bf24ff402469cbe607a2cf34641aca9ef4b867 Mon Sep 17 00:00:00 2001 From: Andy Fingerhut Date: Wed, 9 Oct 2024 07:57:18 -0700 Subject: [PATCH 13/48] More fine-tuning to enable tests to pass Signed-off-by: Andy Fingerhut --- frontends/p4/duplicateHierarchicalNameCheck.cpp | 6 +++++- test/gtest/parser_unroll.cpp | 2 ++ 2 files changed, 7 insertions(+), 1 deletion(-) diff --git a/frontends/p4/duplicateHierarchicalNameCheck.cpp b/frontends/p4/duplicateHierarchicalNameCheck.cpp index 8008c68e699..c6b8f63e6b9 100644 --- a/frontends/p4/duplicateHierarchicalNameCheck.cpp +++ b/frontends/p4/duplicateHierarchicalNameCheck.cpp @@ -38,7 +38,11 @@ const IR::Node *DuplicateHierarchicalNameCheck::postorder(IR::Annotation *annota CHECK_NULL(getContext()->parent); auto *annotatedNode = getContext()->parent->node; CHECK_NULL(annotatedNode); - if (annotatedNode->is()) { + // variable and struct declarations are fine if they have identical + // name annotations, and such name annotations can be synthesized by + // p4c before this pass. Ignore them. + if (annotatedNode->is() || + annotatedNode->is()) { return annotation; } bool foundDuplicate = false; diff --git a/test/gtest/parser_unroll.cpp b/test/gtest/parser_unroll.cpp index 53136ddd1d6..9acdb1a9e31 100644 --- a/test/gtest/parser_unroll.cpp +++ b/test/gtest/parser_unroll.cpp @@ -83,6 +83,8 @@ std::pair loadExample( const char *argv = "./gtestp4c"; options.process(1, (char *const *)&argv); options.langVersion = langVersion; + options.excludeFrontendPasses = true; + options.passesToExcludeFrontend.push_back(cstring("DuplicateHierarchicalNameCheck")); const IR::P4Program *program = load_model(file, options); if (!program) return std::make_pair(nullptr, nullptr); return rewriteParser(program, options); From f7ec2fe29b27ebfaaefd4c7180ca6879b424ad54 Mon Sep 17 00:00:00 2001 From: Andy Fingerhut Date: Wed, 9 Oct 2024 08:01:14 -0700 Subject: [PATCH 14/48] Fix lint error Signed-off-by: Andy Fingerhut --- frontends/p4/duplicateHierarchicalNameCheck.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/frontends/p4/duplicateHierarchicalNameCheck.cpp b/frontends/p4/duplicateHierarchicalNameCheck.cpp index c6b8f63e6b9..5c225b00940 100644 --- a/frontends/p4/duplicateHierarchicalNameCheck.cpp +++ b/frontends/p4/duplicateHierarchicalNameCheck.cpp @@ -42,7 +42,7 @@ const IR::Node *DuplicateHierarchicalNameCheck::postorder(IR::Annotation *annota // name annotations, and such name annotations can be synthesized by // p4c before this pass. Ignore them. if (annotatedNode->is() || - annotatedNode->is()) { + annotatedNode->is()) { return annotation; } bool foundDuplicate = false; From 1cc82246e58dee523ec72675505242d4ec9e49af Mon Sep 17 00:00:00 2001 From: Andy Fingerhut Date: Wed, 9 Oct 2024 08:07:05 -0700 Subject: [PATCH 15/48] Fix lint error Signed-off-by: Andy Fingerhut --- frontends/p4/duplicateHierarchicalNameCheck.cpp | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/frontends/p4/duplicateHierarchicalNameCheck.cpp b/frontends/p4/duplicateHierarchicalNameCheck.cpp index 5c225b00940..ee2139a736d 100644 --- a/frontends/p4/duplicateHierarchicalNameCheck.cpp +++ b/frontends/p4/duplicateHierarchicalNameCheck.cpp @@ -41,8 +41,7 @@ const IR::Node *DuplicateHierarchicalNameCheck::postorder(IR::Annotation *annota // variable and struct declarations are fine if they have identical // name annotations, and such name annotations can be synthesized by // p4c before this pass. Ignore them. - if (annotatedNode->is() || - annotatedNode->is()) { + if (annotatedNode->is() || annotatedNode->is()) { return annotation; } bool foundDuplicate = false; From 93daba582c0e70b8fc72536785be38604d81e5b4 Mon Sep 17 00:00:00 2001 From: Andy Fingerhut Date: Wed, 9 Oct 2024 09:44:38 -0700 Subject: [PATCH 16/48] Address some review comments Signed-off-by: Andy Fingerhut --- frontends/p4/duplicateHierarchicalNameCheck.cpp | 2 +- frontends/p4/duplicateHierarchicalNameCheck.h | 8 +++----- 2 files changed, 4 insertions(+), 6 deletions(-) diff --git a/frontends/p4/duplicateHierarchicalNameCheck.cpp b/frontends/p4/duplicateHierarchicalNameCheck.cpp index ee2139a736d..267e1e8c4d8 100644 --- a/frontends/p4/duplicateHierarchicalNameCheck.cpp +++ b/frontends/p4/duplicateHierarchicalNameCheck.cpp @@ -69,7 +69,7 @@ const IR::Node *DuplicateHierarchicalNameCheck::postorder(IR::Annotation *annota } } if (foundDuplicate) { - error(ErrorType::ERR_DUPLICATE, "%1%: " ERR_STR_DUPLICATED_NAME ": %2%", annotatedNode, + error(ErrorType::ERR_DUPLICATE, "%1%: conflicting control plane name: %2%", annotatedNode, otherNode); } return annotation; diff --git a/frontends/p4/duplicateHierarchicalNameCheck.h b/frontends/p4/duplicateHierarchicalNameCheck.h index 0b849e201b7..f5ae0c2e7a5 100644 --- a/frontends/p4/duplicateHierarchicalNameCheck.h +++ b/frontends/p4/duplicateHierarchicalNameCheck.h @@ -22,8 +22,6 @@ limitations under the License. namespace P4 { -#define ERR_STR_DUPLICATED_NAME "conflicting control plane name" - /** * 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 @@ -44,12 +42,12 @@ namespace P4 { class DuplicateHierarchicalNameCheck : public Transform { std::vector stack; /// Used for detection of conflicting control plane names among actions. - std::map annotatedActions; + string_map annotatedActions; /// Used for detection of conflicting control plane names among tables. - std::map annotatedTables; + string_map annotatedTables; /// Used for detection of conflicting control plane names among /// objects other than actions and tables. - std::map annotatedOthers; + string_map annotatedOthers; public: cstring getName(const IR::IDeclaration *decl); From 66a0a0ad027517852a1868cb00f7c3a5da45b233 Mon Sep 17 00:00:00 2001 From: Andy Fingerhut Date: Wed, 9 Oct 2024 15:01:13 -0700 Subject: [PATCH 17/48] Rewording documentation of new pass for clarity. Signed-off-by: Andy Fingerhut --- frontends/p4/duplicateHierarchicalNameCheck.h | 11 +++++------ 1 file changed, 5 insertions(+), 6 deletions(-) diff --git a/frontends/p4/duplicateHierarchicalNameCheck.h b/frontends/p4/duplicateHierarchicalNameCheck.h index f5ae0c2e7a5..284d5ff1669 100644 --- a/frontends/p4/duplicateHierarchicalNameCheck.h +++ b/frontends/p4/duplicateHierarchicalNameCheck.h @@ -31,13 +31,12 @@ namespace P4 { * in this duplicate check. * * See also the pass HierarchicalNames, on which the implementation of - * this pass is based. + * this pass was based. * - * This pass should be run before LocalizeAllActions, because that - * pass can create actions with duplicate names, by design, that were - * not created by the P4 developer, but generated internally by the - * compiler. We should not issue an error if that pass creates - * duplicate hierarchical names. + * 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 DuplicateHierarchicalNameCheck : public Transform { std::vector stack; From 99beb308caeefec9430b5693ecf58fba7f503b97 Mon Sep 17 00:00:00 2001 From: Andy Fingerhut Date: Sun, 20 Oct 2024 04:42:43 +0000 Subject: [PATCH 18/48] Enable the new pass only if P4Runtime control plane API gen is enabled Signed-off-by: Andy Fingerhut --- control-plane/p4RuntimeSerializer.cpp | 2 +- frontends/p4/frontend.cpp | 8 ++++++++ frontends/p4/frontend.h | 12 ++++++++++++ 3 files changed, 21 insertions(+), 1 deletion(-) diff --git a/control-plane/p4RuntimeSerializer.cpp b/control-plane/p4RuntimeSerializer.cpp index e6120d824ca..c520a5c8e4b 100644 --- a/control-plane/p4RuntimeSerializer.cpp +++ b/control-plane/p4RuntimeSerializer.cpp @@ -1584,7 +1584,7 @@ void P4RuntimeSerializer::serializeP4RuntimeIfRequired(const IR::P4Program *prog std::vector files; std::vector formats; - // only generate P4Info is required by use-provided options + // only generate P4Info is required by user-provided options if (options.p4RuntimeFile.isNullOrEmpty() && options.p4RuntimeFiles.isNullOrEmpty() && options.p4RuntimeEntriesFile.isNullOrEmpty() && options.p4RuntimeEntriesFiles.isNullOrEmpty()) { diff --git a/frontends/p4/frontend.cpp b/frontends/p4/frontend.cpp index 7fb2e02e277..c7b531695cd 100644 --- a/frontends/p4/frontend.cpp +++ b/frontends/p4/frontend.cpp @@ -239,7 +239,15 @@ const IR::P4Program *FrontEnd::run(const CompilerOptions &options, const IR::P4P if (policy->optimize(options)) { passes.addPasses({ new Inline(&refMap, &typeMap, evaluator, *policy, options.optimizeParserInlining), + }); + } + if (policy->controlPlaneAPIGenEnabled(options)) { + passes.addPasses({ new DuplicateHierarchicalNameCheck(), + }); + } + if (policy->optimize(options)) { + passes.addPasses({ new InlineActions(&refMap, &typeMap, *policy), new LocalizeAllActions(&refMap, *policy), new UniqueNames(), diff --git a/frontends/p4/frontend.h b/frontends/p4/frontend.h index 14ad2f345a1..0695ef38298 100644 --- a/frontends/p4/frontend.h +++ b/frontends/p4/frontend.h @@ -60,6 +60,18 @@ class FrontEndPolicy : public RemoveUnusedPolicy { return options.optimizationLevel > 0; } + /// 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 CompilerOptions &options) const { + if (options.p4RuntimeFile.isNullOrEmpty() && options.p4RuntimeFiles.isNullOrEmpty() && + options.p4RuntimeEntriesFile.isNullOrEmpty() && + options.p4RuntimeEntriesFiles.isNullOrEmpty()) { + return false; + } + return true; + } + /// Get policy for the constant folding pass. @see ConstantFoldingPolicy /// @returns Defaults to nullptr, which causes constant folding to use the default policy, which /// does not modify the pass defaults in any way. From d89f2146c073a2513eb7ee1f87c4df0dc7cdf2d9 Mon Sep 17 00:00:00 2001 From: Andy Fingerhut Date: Sun, 20 Oct 2024 03:12:16 -0400 Subject: [PATCH 19/48] Undo changes to Python test scripts Signed-off-by: Andy Fingerhut --- backends/bmv2/run-bmv2-test.py | 1 - backends/p4test/run-p4-sample.py | 13 ++++--------- 2 files changed, 4 insertions(+), 10 deletions(-) diff --git a/backends/bmv2/run-bmv2-test.py b/backends/bmv2/run-bmv2-test.py index ac6fc2169a5..94ec54d5dae 100755 --- a/backends/bmv2/run-bmv2-test.py +++ b/backends/bmv2/run-bmv2-test.py @@ -292,7 +292,6 @@ def process_file(options, argv): args = [binary, "-o", jsonfile] + options.compilerOptions if "p4_14" in options.p4filename or "v1_samples" in options.p4filename: - args.extend(["--excludeFrontendPasses", "DuplicateHierarchicalNameCheck"]) args.extend(["--std", "p4-14"]) args.append(options.p4filename) args.extend(argv) diff --git a/backends/p4test/run-p4-sample.py b/backends/p4test/run-p4-sample.py index 554583cca90..ae0a89e2d73 100755 --- a/backends/p4test/run-p4-sample.py +++ b/backends/p4test/run-p4-sample.py @@ -164,7 +164,7 @@ def compare_files(options, produced, expected, ignore_case): return FAILURE -def recompile_file(options, produced, mustBeIdentical, origFileP414=False): +def recompile_file(options, produced, mustBeIdentical): # Compile the generated file a second time secondFile = produced + "-x" args = [ @@ -174,10 +174,8 @@ def recompile_file(options, produced, mustBeIdentical, origFileP414=False): secondFile, "--std", "p4-16", - ] - if origFileP414 or ('-midend' in produced): - args += ["--excludeFrontendPasses", "DuplicateHierarchicalNameCheck"] - args += [produced] + options.compilerOptions + produced, + ] + options.compilerOptions if options.runDebugger: if options.runDebugger_skip > 0: options.runDebugger_skip = options.runDebugger_skip - 1 @@ -316,10 +314,7 @@ def getArch(path): args.extend(["--p4runtime-files", p4runtimeFile]) args.extend(["--p4runtime-entries-files", p4runtimeEntriesFile]) - origFileP414 = False if "p4_14" in options.p4filename or "v1_samples" in options.p4filename: - origFileP414 = True - args.extend(["--excludeFrontendPasses", "DuplicateHierarchicalNameCheck"]) args.extend(["--std", "p4-14"]) args.extend(argv) if options.runDebugger: @@ -362,7 +357,7 @@ def getArch(path): if result == SUCCESS: result = check_generated_files(options, tmpdir, expected_dirname) if (result == SUCCESS) and (not expected_error): - result = recompile_file(options, ppfile, False, origFileP414) + result = recompile_file(options, ppfile, False) if ( (result == SUCCESS) and (not expected_error) From 7424db312b7d2732bd463d0cc3790719a7ee7bd8 Mon Sep 17 00:00:00 2001 From: Andy Fingerhut Date: Sun, 20 Oct 2024 03:34:38 -0400 Subject: [PATCH 20/48] Undo changes to gtestp4c source Signed-off-by: Andy Fingerhut --- test/gtest/parser_unroll.cpp | 2 -- 1 file changed, 2 deletions(-) diff --git a/test/gtest/parser_unroll.cpp b/test/gtest/parser_unroll.cpp index 9acdb1a9e31..53136ddd1d6 100644 --- a/test/gtest/parser_unroll.cpp +++ b/test/gtest/parser_unroll.cpp @@ -83,8 +83,6 @@ std::pair loadExample( const char *argv = "./gtestp4c"; options.process(1, (char *const *)&argv); options.langVersion = langVersion; - options.excludeFrontendPasses = true; - options.passesToExcludeFrontend.push_back(cstring("DuplicateHierarchicalNameCheck")); const IR::P4Program *program = load_model(file, options); if (!program) return std::make_pair(nullptr, nullptr); return rewriteParser(program, options); From 799448a178763dead5c9d46e5a752e684ef6aa30 Mon Sep 17 00:00:00 2001 From: Andy Fingerhut Date: Sun, 20 Oct 2024 20:07:22 -0400 Subject: [PATCH 21/48] Use abseil StrCat and StrAppend instead of loop Signed-off-by: Andy Fingerhut --- frontends/p4/duplicateHierarchicalNameCheck.cpp | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/frontends/p4/duplicateHierarchicalNameCheck.cpp b/frontends/p4/duplicateHierarchicalNameCheck.cpp index 267e1e8c4d8..e2afdf7b59a 100644 --- a/frontends/p4/duplicateHierarchicalNameCheck.cpp +++ b/frontends/p4/duplicateHierarchicalNameCheck.cpp @@ -29,10 +29,10 @@ const IR::Node *DuplicateHierarchicalNameCheck::postorder(IR::Annotation *annota cstring name = annotation->getName(); if (!name.startsWith(".")) { - std::string newName = ""; - for (cstring s : stack) newName += s + "."; - newName += name; - name = newName; + if (!stack.empty()) { + name = absl::StrCat(absl::StrJoin(stack, ".", [](std::string *out, cstring s) { + absl::StrAppend(out, s.string_view()); }), ".", name.string_view()); + } } // The node the annotation belongs to CHECK_NULL(getContext()->parent); From 377f41a049669fb66be653a8d90d93954ba77364 Mon Sep 17 00:00:00 2001 From: Andy Fingerhut Date: Sun, 20 Oct 2024 20:15:38 -0400 Subject: [PATCH 22/48] Fix formatting with clang-format Signed-off-by: Andy Fingerhut --- frontends/p4/duplicateHierarchicalNameCheck.cpp | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/frontends/p4/duplicateHierarchicalNameCheck.cpp b/frontends/p4/duplicateHierarchicalNameCheck.cpp index e2afdf7b59a..a4a3f877e68 100644 --- a/frontends/p4/duplicateHierarchicalNameCheck.cpp +++ b/frontends/p4/duplicateHierarchicalNameCheck.cpp @@ -30,8 +30,11 @@ const IR::Node *DuplicateHierarchicalNameCheck::postorder(IR::Annotation *annota cstring name = annotation->getName(); if (!name.startsWith(".")) { if (!stack.empty()) { - name = absl::StrCat(absl::StrJoin(stack, ".", [](std::string *out, cstring s) { - absl::StrAppend(out, s.string_view()); }), ".", name.string_view()); + name = absl::StrCat(absl::StrJoin(stack, ".", + [](std::string *out, cstring s) { + absl::StrAppend(out, s.string_view()); + }), + ".", name.string_view()); } } // The node the annotation belongs to From 682564b1bf647500dccfa24f69b713d708802f69 Mon Sep 17 00:00:00 2001 From: Andy Fingerhut Date: Mon, 21 Oct 2024 13:56:36 -0400 Subject: [PATCH 23/48] Fix #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 --- frontends/p4/localizeActions.cpp | 22 +++- testdata/p4_16_samples/issue-4969-bmv2.p4 | 106 ++++++++++++++++++ .../issue-4969-bmv2-first.p4 | 69 ++++++++++++ .../issue-4969-bmv2-frontend.p4 | 71 ++++++++++++ .../issue-4969-bmv2-midend.p4 | 70 ++++++++++++ .../p4_16_samples_outputs/issue-4969-bmv2.p4 | 69 ++++++++++++ .../issue-4969-bmv2.p4-stderr | 0 .../issue-4969-bmv2.p4.entries.txtpb | 3 + .../issue-4969-bmv2.p4.p4info.txtpb | 56 +++++++++ 9 files changed, 463 insertions(+), 3 deletions(-) create mode 100644 testdata/p4_16_samples/issue-4969-bmv2.p4 create mode 100644 testdata/p4_16_samples_outputs/issue-4969-bmv2-first.p4 create mode 100644 testdata/p4_16_samples_outputs/issue-4969-bmv2-frontend.p4 create mode 100644 testdata/p4_16_samples_outputs/issue-4969-bmv2-midend.p4 create mode 100644 testdata/p4_16_samples_outputs/issue-4969-bmv2.p4 create mode 100644 testdata/p4_16_samples_outputs/issue-4969-bmv2.p4-stderr create mode 100644 testdata/p4_16_samples_outputs/issue-4969-bmv2.p4.entries.txtpb create mode 100644 testdata/p4_16_samples_outputs/issue-4969-bmv2.p4.p4info.txtpb diff --git a/frontends/p4/localizeActions.cpp b/frontends/p4/localizeActions.cpp index 3ae11913a68..16d907d0683 100644 --- a/frontends/p4/localizeActions.cpp +++ b/frontends/p4/localizeActions.cpp @@ -43,9 +43,25 @@ const IR::Node *TagGlobalActions::preorder(IR::P4Action *action) { if (findContext() == 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()->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 new file mode 100644 index 00000000000..b0b27d0dc08 --- /dev/null +++ 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 +#include + +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 new file mode 100644 index 00000000000..e489adb60a4 --- /dev/null +++ b/testdata/p4_16_samples_outputs/issue-4969-bmv2-first.p4 @@ -0,0 +1,69 @@ +#include +#define V1MODEL_VERSION 20180101 +#include + +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 @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(hdr.ethernet); + } +} + +V1Switch(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 new file mode 100644 index 00000000000..55db1cdb1a4 --- /dev/null +++ b/testdata/p4_16_samples_outputs/issue-4969-bmv2-frontend.p4 @@ -0,0 +1,71 @@ +#include +#define V1MODEL_VERSION 20180101 +#include + +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 { + } +} + +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(hdr.ethernet); + } +} + +V1Switch(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 new file mode 100644 index 00000000000..cf2c9409130 --- /dev/null +++ b/testdata/p4_16_samples_outputs/issue-4969-bmv2-midend.p4 @@ -0,0 +1,70 @@ +#include +#define V1MODEL_VERSION 20180101 +#include + +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(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(hdr.ethernet); + } +} + +V1Switch(parserImpl(), verifyChecksum(), ingressImpl(), egressImpl(), updateChecksum(), deparserImpl()) main; diff --git a/testdata/p4_16_samples_outputs/issue-4969-bmv2.p4 b/testdata/p4_16_samples_outputs/issue-4969-bmv2.p4 new file mode 100644 index 00000000000..561c5cfa59c --- /dev/null +++ b/testdata/p4_16_samples_outputs/issue-4969-bmv2.p4 @@ -0,0 +1,69 @@ +#include +#define V1MODEL_VERSION 20180101 +#include + +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.p4-stderr b/testdata/p4_16_samples_outputs/issue-4969-bmv2.p4-stderr new file mode 100644 index 00000000000..e69de29bb2d diff --git a/testdata/p4_16_samples_outputs/issue-4969-bmv2.p4.entries.txtpb b/testdata/p4_16_samples_outputs/issue-4969-bmv2.p4.entries.txtpb new file mode 100644 index 00000000000..5cb9652623a --- /dev/null +++ b/testdata/p4_16_samples_outputs/issue-4969-bmv2.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/issue-4969-bmv2.p4.p4info.txtpb b/testdata/p4_16_samples_outputs/issue-4969-bmv2.p4.p4info.txtpb new file mode 100644 index 00000000000..84addd50a84 --- /dev/null +++ b/testdata/p4_16_samples_outputs/issue-4969-bmv2.p4.p4info.txtpb @@ -0,0 +1,56 @@ +# proto-file: p4/config/v1/p4info.proto +# proto-message: p4.config.v1.P4Info + +pkg_info { + arch: "v1model" +} +tables { + preamble { + id: 49173205 + name: "ingressImpl.t1" + alias: "t1" + } + match_fields { + id: 1 + name: "hdr.ethernet.etherType" + bitwidth: 16 + match_type: EXACT + } + action_refs { + id: 21257015 + } + action_refs { + id: 25646030 + } + action_refs { + id: 21008649 + } + initial_default_action { + action_id: 21257015 + } + size: 512 +} +actions { + preamble { + id: 21257015 + name: "NoAction" + alias: "NoAction" + annotations: "@noWarn(\"unused\")" + } +} +actions { + preamble { + id: 25646030 + name: "foo1" + alias: "foo1" + } +} +actions { + preamble { + id: 21008649 + name: "bar" + alias: "bar" + } +} +type_info { +} From bb88b7dc072b97906f67310da4717ad339f93a78 Mon Sep 17 00:00:00 2001 From: Andy Fingerhut Date: Mon, 21 Oct 2024 14:05:18 -0400 Subject: [PATCH 24/48] Fix clang-format error Signed-off-by: Andy Fingerhut --- frontends/p4/localizeActions.cpp | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/frontends/p4/localizeActions.cpp b/frontends/p4/localizeActions.cpp index 16d907d0683..daf53527340 100644 --- a/frontends/p4/localizeActions.cpp +++ b/frontends/p4/localizeActions.cpp @@ -53,8 +53,7 @@ const IR::Node *TagGlobalActions::preorder(IR::P4Action *action) { if (!nameString.startsWith(".")) { nameString = "."_cs + nameString; auto newLit = new IR::StringLiteral(e0->srcInfo, nameString); - annos = annos->addOrReplace(IR::Annotation::nameAnnotation, - newLit); + annos = annos->addOrReplace(IR::Annotation::nameAnnotation, newLit); } } else { // Add new name annotation beginning with "." From d265e1c24ae5f156c2a1251e4d4bee8bd0cee4ea Mon Sep 17 00:00:00 2001 From: Andy Fingerhut Date: Mon, 21 Oct 2024 17:36:20 -0400 Subject: [PATCH 25/48] Also catch duplicates involving top-level actions without @name annotations Signed-off-by: Andy Fingerhut --- .../p4/duplicateHierarchicalNameCheck.cpp | 82 ++++++++++++------- frontends/p4/duplicateHierarchicalNameCheck.h | 4 + 2 files changed, 58 insertions(+), 28 deletions(-) diff --git a/frontends/p4/duplicateHierarchicalNameCheck.cpp b/frontends/p4/duplicateHierarchicalNameCheck.cpp index a4a3f877e68..a45e12832d9 100644 --- a/frontends/p4/duplicateHierarchicalNameCheck.cpp +++ b/frontends/p4/duplicateHierarchicalNameCheck.cpp @@ -24,6 +24,59 @@ cstring DuplicateHierarchicalNameCheck::getName(const IR::IDeclaration *decl) { return decl->getName(); } +void DuplicateHierarchicalNameCheck::checkForDuplicateName(cstring name, const IR::Node *node) { + bool foundDuplicate = false; + auto *otherNode = node; + if (node->is()) { + auto [it, inserted] = annotatedTables.insert(std::pair(name, node)); + if (!inserted) { + foundDuplicate = true; + otherNode = (*it).second; + } + } else if (node->is()) { + auto [it, inserted] = annotatedActions.insert(std::pair(name, node)); + if (!inserted) { + foundDuplicate = true; + otherNode = (*it).second; + } + } else { + auto [it, inserted] = annotatedOthers.insert(std::pair(name, node)); + if (!inserted) { + foundDuplicate = true; + otherNode = (*it).second; + } + } + if (foundDuplicate) { + error(ErrorType::ERR_DUPLICATE, "%1%: conflicting control plane name: %2%", node, + otherNode); + } +} + +const IR::Node *DuplicateHierarchicalNameCheck::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() == 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 *DuplicateHierarchicalNameCheck::postorder(IR::Annotation *annotation) { if (annotation->name != IR::Annotation::nameAnnotation) return annotation; @@ -47,34 +100,7 @@ const IR::Node *DuplicateHierarchicalNameCheck::postorder(IR::Annotation *annota if (annotatedNode->is() || annotatedNode->is()) { return annotation; } - bool foundDuplicate = false; - auto *otherNode = annotatedNode; - if (annotatedNode->is()) { - if (annotatedTables.count(name)) { - foundDuplicate = true; - otherNode = annotatedTables[name]; - } else { - annotatedTables[name] = annotatedNode; - } - } else if (annotatedNode->is()) { - if (annotatedActions.count(name)) { - foundDuplicate = true; - otherNode = annotatedActions[name]; - } else { - annotatedActions[name] = annotatedNode; - } - } else { - if (annotatedOthers.count(name)) { - foundDuplicate = true; - otherNode = annotatedOthers[name]; - } else { - annotatedOthers[name] = annotatedNode; - } - } - if (foundDuplicate) { - error(ErrorType::ERR_DUPLICATE, "%1%: conflicting control plane name: %2%", annotatedNode, - otherNode); - } + checkForDuplicateName(name, annotatedNode); return annotation; } diff --git a/frontends/p4/duplicateHierarchicalNameCheck.h b/frontends/p4/duplicateHierarchicalNameCheck.h index 284d5ff1669..7ac3fea27fa 100644 --- a/frontends/p4/duplicateHierarchicalNameCheck.h +++ b/frontends/p4/duplicateHierarchicalNameCheck.h @@ -79,6 +79,10 @@ class DuplicateHierarchicalNameCheck : public Transform { 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 { From 6bcb6147fe79bb03294db9855239b924fe49336e Mon Sep 17 00:00:00 2001 From: Andy Fingerhut Date: Mon, 21 Oct 2024 17:39:04 -0400 Subject: [PATCH 26/48] Add new test program to exercise latest code changes Signed-off-by: Andy Fingerhut --- ...ons-with-duplicate-control-plane-names1.p4 | 130 ++++++++++++++++++ ...th-duplicate-control-plane-names1-first.p4 | 84 +++++++++++ ...ons-with-duplicate-control-plane-names1.p4 | 84 +++++++++++ ...h-duplicate-control-plane-names1.p4-stderr | 6 + 4 files changed, 304 insertions(+) create mode 100644 testdata/p4_16_errors/actions-with-duplicate-control-plane-names1.p4 create mode 100644 testdata/p4_16_errors_outputs/actions-with-duplicate-control-plane-names1-first.p4 create mode 100644 testdata/p4_16_errors_outputs/actions-with-duplicate-control-plane-names1.p4 create mode 100644 testdata/p4_16_errors_outputs/actions-with-duplicate-control-plane-names1.p4-stderr diff --git a/testdata/p4_16_errors/actions-with-duplicate-control-plane-names1.p4 b/testdata/p4_16_errors/actions-with-duplicate-control-plane-names1.p4 new file mode 100644 index 00000000000..561174209c8 --- /dev/null +++ b/testdata/p4_16_errors/actions-with-duplicate-control-plane-names1.p4 @@ -0,0 +1,130 @@ +/* +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 +#include + +typedef bit<48> EthernetAddress; + +header ethernet_t { + EthernetAddress dstAddr; + EthernetAddress srcAddr; + bit<16> etherType; +} + +struct headers_t { + ethernet_t ethernet; +} + +struct metadata_t { +/* + bit<4> a; + bit<4> b; +*/ +} + +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("foo2") +action bar() { +} + +control ingressImpl( + inout headers_t hdr, + inout metadata_t meta, + inout standard_metadata_t stdmeta) + +{ + bit<8> tmp1; + bit<8> tmp2; + + // Action a1's control plane name should conflict with the name of + // top-level action foo1, and cause a compile-time error message, + // if you enable P4Info file generation. + @name(".foo1") action a1 (bit<8> x, bit<8> y) { tmp1 = x; tmp2 = y; } + + // Action a2's control plane name should conflict with the control + // plane name of top-level action bar, and cause a compile-time + // error message, if you enable P4Info file generation. + @name(".foo2") action a2 (bit<8> x, bit<8> y) { tmp1 = x; tmp2 = y; } + + table t1 { + actions = { NoAction; a1; a2; foo1; bar; } + key = { hdr.ethernet.etherType: exact; } + default_action = NoAction(); + size = 512; + } + apply { + tmp1 = hdr.ethernet.srcAddr[7:0]; + tmp2 = hdr.ethernet.dstAddr[7:0]; + t1.apply(); + // This is here simply to ensure that the compiler cannot + // optimize away the effects of t1 and t2, which can only + // assign values to variables tmp1 and tmp2. + hdr.ethernet.etherType = (bit<16>) (tmp1 - tmp2); + } +} + +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_errors_outputs/actions-with-duplicate-control-plane-names1-first.p4 b/testdata/p4_16_errors_outputs/actions-with-duplicate-control-plane-names1-first.p4 new file mode 100644 index 00000000000..483e7b604f8 --- /dev/null +++ b/testdata/p4_16_errors_outputs/actions-with-duplicate-control-plane-names1-first.p4 @@ -0,0 +1,84 @@ +#include +#define V1MODEL_VERSION 20180101 +#include + +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("foo2") action bar() { +} +control ingressImpl(inout headers_t hdr, inout metadata_t meta, inout standard_metadata_t stdmeta) { + bit<8> tmp1; + bit<8> tmp2; + @name(".foo1") action a1(bit<8> x, bit<8> y) { + tmp1 = x; + tmp2 = y; + } + @name(".foo2") action a2(bit<8> x, bit<8> y) { + tmp1 = x; + tmp2 = y; + } + table t1 { + actions = { + NoAction(); + a1(); + a2(); + foo1(); + bar(); + } + key = { + hdr.ethernet.etherType: exact @name("hdr.ethernet.etherType"); + } + default_action = NoAction(); + size = 512; + } + apply { + tmp1 = hdr.ethernet.srcAddr[7:0]; + tmp2 = hdr.ethernet.dstAddr[7:0]; + t1.apply(); + hdr.ethernet.etherType = (bit<16>)(tmp1 - tmp2); + } +} + +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_errors_outputs/actions-with-duplicate-control-plane-names1.p4 b/testdata/p4_16_errors_outputs/actions-with-duplicate-control-plane-names1.p4 new file mode 100644 index 00000000000..31e4f5d82f8 --- /dev/null +++ b/testdata/p4_16_errors_outputs/actions-with-duplicate-control-plane-names1.p4 @@ -0,0 +1,84 @@ +#include +#define V1MODEL_VERSION 20180101 +#include + +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("foo2") action bar() { +} +control ingressImpl(inout headers_t hdr, inout metadata_t meta, inout standard_metadata_t stdmeta) { + bit<8> tmp1; + bit<8> tmp2; + @name(".foo1") action a1(bit<8> x, bit<8> y) { + tmp1 = x; + tmp2 = y; + } + @name(".foo2") action a2(bit<8> x, bit<8> y) { + tmp1 = x; + tmp2 = y; + } + table t1 { + actions = { + NoAction; + a1; + a2; + foo1; + bar; + } + key = { + hdr.ethernet.etherType: exact; + } + default_action = NoAction(); + size = 512; + } + apply { + tmp1 = hdr.ethernet.srcAddr[7:0]; + tmp2 = hdr.ethernet.dstAddr[7:0]; + t1.apply(); + hdr.ethernet.etherType = (bit<16>)(tmp1 - tmp2); + } +} + +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_errors_outputs/actions-with-duplicate-control-plane-names1.p4-stderr b/testdata/p4_16_errors_outputs/actions-with-duplicate-control-plane-names1.p4-stderr new file mode 100644 index 00000000000..29506558ce9 --- /dev/null +++ b/testdata/p4_16_errors_outputs/actions-with-duplicate-control-plane-names1.p4-stderr @@ -0,0 +1,6 @@ +actions-with-duplicate-control-plane-names1.p4(77): [--Werror=duplicate] error: .foo1: conflicting control plane name: foo1 + @name(".foo1") action a1 (bit<8> x, bit<8> y) { tmp1 = x; tmp2 = y; } + ^^ +actions-with-duplicate-control-plane-names1.p4(58) +action foo1() { + ^^^^ From 55da2c37be7ba83a1b145256ae43f932cc63acd6 Mon Sep 17 00:00:00 2001 From: Andy Fingerhut Date: Mon, 21 Oct 2024 18:51:35 -0400 Subject: [PATCH 27/48] Change pass name to reflect only checking action control plane names Table and other P4 object control plane name conflicts are already checked for in the P4Info file generation code. Action control plane names are the only thing it does not do correctly. Signed-off-by: Andy Fingerhut --- frontends/CMakeLists.txt | 2 +- ... duplicateActionControlPlaneNameCheck.cpp} | 53 +++++++------------ ...=> duplicateActionControlPlaneNameCheck.h} | 19 +++---- frontends/p4/frontend.cpp | 4 +- .../issue1803_same_table_name.p4-stderr | 8 +-- 5 files changed, 31 insertions(+), 55 deletions(-) rename frontends/p4/{duplicateHierarchicalNameCheck.cpp => duplicateActionControlPlaneNameCheck.cpp} (66%) rename frontends/p4/{duplicateHierarchicalNameCheck.h => duplicateActionControlPlaneNameCheck.h} (79%) diff --git a/frontends/CMakeLists.txt b/frontends/CMakeLists.txt index db6b590d3d1..a44f4b1e192 100644 --- a/frontends/CMakeLists.txt +++ b/frontends/CMakeLists.txt @@ -33,7 +33,7 @@ set (P4_FRONTEND_SRCS p4/frontend.cpp p4/functionsInlining.cpp p4/hierarchicalNames.cpp - p4/duplicateHierarchicalNameCheck.cpp + p4/duplicateActionControlPlaneNameCheck.cpp p4/inlining.cpp p4/localizeActions.cpp p4/methodInstance.cpp diff --git a/frontends/p4/duplicateHierarchicalNameCheck.cpp b/frontends/p4/duplicateActionControlPlaneNameCheck.cpp similarity index 66% rename from frontends/p4/duplicateHierarchicalNameCheck.cpp rename to frontends/p4/duplicateActionControlPlaneNameCheck.cpp index a45e12832d9..5c921f091f8 100644 --- a/frontends/p4/duplicateHierarchicalNameCheck.cpp +++ b/frontends/p4/duplicateActionControlPlaneNameCheck.cpp @@ -14,37 +14,23 @@ See the License for the specific language governing permissions and limitations under the License. */ -#include "duplicateHierarchicalNameCheck.h" +#include "duplicateActionControlPlaneNameCheck.h" #include "lib/log.h" namespace P4 { -cstring DuplicateHierarchicalNameCheck::getName(const IR::IDeclaration *decl) { +cstring DuplicateActionControlPlaneNameCheck::getName(const IR::IDeclaration *decl) { return decl->getName(); } -void DuplicateHierarchicalNameCheck::checkForDuplicateName(cstring name, const IR::Node *node) { +void DuplicateActionControlPlaneNameCheck::checkForDuplicateName(cstring name, const IR::Node *node) { bool foundDuplicate = false; auto *otherNode = node; - if (node->is()) { - auto [it, inserted] = annotatedTables.insert(std::pair(name, node)); - if (!inserted) { - foundDuplicate = true; - otherNode = (*it).second; - } - } else if (node->is()) { - auto [it, inserted] = annotatedActions.insert(std::pair(name, node)); - if (!inserted) { - foundDuplicate = true; - otherNode = (*it).second; - } - } else { - auto [it, inserted] = annotatedOthers.insert(std::pair(name, node)); - if (!inserted) { - foundDuplicate = true; - otherNode = (*it).second; - } + auto [it, inserted] = actions.insert(std::pair(name, node)); + if (!inserted) { + foundDuplicate = true; + otherNode = (*it).second; } if (foundDuplicate) { error(ErrorType::ERR_DUPLICATE, "%1%: conflicting control plane name: %2%", node, @@ -52,7 +38,7 @@ void DuplicateHierarchicalNameCheck::checkForDuplicateName(cstring name, const I } } -const IR::Node *DuplicateHierarchicalNameCheck::postorder(IR::P4Action *action) { +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 @@ -77,9 +63,18 @@ const IR::Node *DuplicateHierarchicalNameCheck::postorder(IR::P4Action *action) return action; } -const IR::Node *DuplicateHierarchicalNameCheck::postorder(IR::Annotation *annotation) { +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()) { + return annotation; + } cstring name = annotation->getName(); if (!name.startsWith(".")) { if (!stack.empty()) { @@ -90,16 +85,6 @@ const IR::Node *DuplicateHierarchicalNameCheck::postorder(IR::Annotation *annota ".", name.string_view()); } } - // The node the annotation belongs to - CHECK_NULL(getContext()->parent); - auto *annotatedNode = getContext()->parent->node; - CHECK_NULL(annotatedNode); - // variable and struct declarations are fine if they have identical - // name annotations, and such name annotations can be synthesized by - // p4c before this pass. Ignore them. - if (annotatedNode->is() || annotatedNode->is()) { - return annotation; - } checkForDuplicateName(name, annotatedNode); return annotation; } diff --git a/frontends/p4/duplicateHierarchicalNameCheck.h b/frontends/p4/duplicateActionControlPlaneNameCheck.h similarity index 79% rename from frontends/p4/duplicateHierarchicalNameCheck.h rename to frontends/p4/duplicateActionControlPlaneNameCheck.h index 7ac3fea27fa..18156cc1ace 100644 --- a/frontends/p4/duplicateHierarchicalNameCheck.h +++ b/frontends/p4/duplicateActionControlPlaneNameCheck.h @@ -14,8 +14,8 @@ See the License for the specific language governing permissions and limitations under the License. */ -#ifndef FRONTENDS_P4_DUPLICATEHIERARCHICALNAMECHECK_H_ -#define FRONTENDS_P4_DUPLICATEHIERARCHICALNAMECHECK_H_ +#ifndef FRONTENDS_P4_DUPLICATEACTIONCONTROLPLANENAMECHECK_H_ +#define FRONTENDS_P4_DUPLICATEACTIONCONTROLPLANENAMECHECK_H_ #include "ir/ir.h" #include "ir/visitor.h" @@ -38,21 +38,16 @@ namespace P4 { * design, that were not created by the P4 developer. We should not * issue an error if that pass creates duplicate hierarchical names. */ -class DuplicateHierarchicalNameCheck : public Transform { +class DuplicateActionControlPlaneNameCheck : public Transform { std::vector stack; /// Used for detection of conflicting control plane names among actions. - string_map annotatedActions; - /// Used for detection of conflicting control plane names among tables. - string_map annotatedTables; - /// Used for detection of conflicting control plane names among - /// objects other than actions and tables. - string_map annotatedOthers; + string_map actions; public: cstring getName(const IR::IDeclaration *decl); - DuplicateHierarchicalNameCheck() { - setName("DuplicateHierarchicalNameCheck"); + DuplicateActionControlPlaneNameCheck() { + setName("DuplicateActionControlPlaneNameCheck"); visitDagOnce = false; } const IR::Node *preorder(IR::P4Parser *parser) override { @@ -93,4 +88,4 @@ class DuplicateHierarchicalNameCheck : public Transform { } // namespace P4 -#endif /* FRONTENDS_P4_DUPLICATEHIERARCHICALNAMECHECK_H_ */ +#endif /* FRONTENDS_P4_DUPLICATEACTIONCONTROLPLANENAMECHECK_H_ */ diff --git a/frontends/p4/frontend.cpp b/frontends/p4/frontend.cpp index 8a116c3827d..0fd1da169c6 100644 --- a/frontends/p4/frontend.cpp +++ b/frontends/p4/frontend.cpp @@ -35,7 +35,7 @@ limitations under the License. #include "deprecated.h" #include "directCalls.h" #include "dontcareArgs.h" -#include "duplicateHierarchicalNameCheck.h" +#include "duplicateActionControlPlaneNameCheck.h" #include "entryPriorities.h" #include "evaluator/evaluator.h" #include "frontends/common/constantFolding.h" @@ -239,7 +239,7 @@ const IR::P4Program *FrontEnd::run(const CompilerOptions &options, const IR::P4P } if (policy->controlPlaneAPIGenEnabled(options)) { passes.addPasses({ - new DuplicateHierarchicalNameCheck(), + new DuplicateActionControlPlaneNameCheck(), }); } if (policy->optimize(options)) { diff --git a/testdata/p4_16_errors_outputs/issue1803_same_table_name.p4-stderr b/testdata/p4_16_errors_outputs/issue1803_same_table_name.p4-stderr index 1128bce3b15..d6929e2b197 100644 --- a/testdata/p4_16_errors_outputs/issue1803_same_table_name.p4-stderr +++ b/testdata/p4_16_errors_outputs/issue1803_same_table_name.p4-stderr @@ -1,6 +1,2 @@ -issue1803_same_table_name.p4(19): [--Werror=duplicate] error: .t0: conflicting control plane name: .t0 - table t0 { - ^^ -issue1803_same_table_name.p4(19) - table t0 { - ^^ +[--Werror=duplicate] error: Name 't0' is used for multiple table objects in the P4Info message +[--Werror=duplicate] error: Found 1 duplicate name(s) in the P4Info From d67df833fae18ee1bda90223e2dab83685ecdcfd Mon Sep 17 00:00:00 2001 From: Andy Fingerhut Date: Tue, 22 Oct 2024 00:35:04 -0400 Subject: [PATCH 28/48] Fix lint warning, and add one more successful test program Signed-off-by: Andy Fingerhut --- .../duplicateActionControlPlaneNameCheck.cpp | 3 +- .../actions-almost-duplicate-names1.p4 | 152 ++++++++++++++++++ .../actions-almost-duplicate-names1-first.p4 | 97 +++++++++++ ...ctions-almost-duplicate-names1-frontend.p4 | 99 ++++++++++++ .../actions-almost-duplicate-names1-midend.p4 | 116 +++++++++++++ .../actions-almost-duplicate-names1.p4 | 97 +++++++++++ .../actions-almost-duplicate-names1.p4-stderr | 0 ...s-almost-duplicate-names1.p4.entries.txtpb | 3 + ...ns-almost-duplicate-names1.p4.p4info.txtpb | 146 +++++++++++++++++ 9 files changed, 712 insertions(+), 1 deletion(-) create mode 100644 testdata/p4_16_samples/actions-almost-duplicate-names1.p4 create mode 100644 testdata/p4_16_samples_outputs/actions-almost-duplicate-names1-first.p4 create mode 100644 testdata/p4_16_samples_outputs/actions-almost-duplicate-names1-frontend.p4 create mode 100644 testdata/p4_16_samples_outputs/actions-almost-duplicate-names1-midend.p4 create mode 100644 testdata/p4_16_samples_outputs/actions-almost-duplicate-names1.p4 create mode 100644 testdata/p4_16_samples_outputs/actions-almost-duplicate-names1.p4-stderr create mode 100644 testdata/p4_16_samples_outputs/actions-almost-duplicate-names1.p4.entries.txtpb create mode 100644 testdata/p4_16_samples_outputs/actions-almost-duplicate-names1.p4.p4info.txtpb diff --git a/frontends/p4/duplicateActionControlPlaneNameCheck.cpp b/frontends/p4/duplicateActionControlPlaneNameCheck.cpp index 5c921f091f8..13dd2b1c1e1 100644 --- a/frontends/p4/duplicateActionControlPlaneNameCheck.cpp +++ b/frontends/p4/duplicateActionControlPlaneNameCheck.cpp @@ -24,7 +24,8 @@ cstring DuplicateActionControlPlaneNameCheck::getName(const IR::IDeclaration *de return decl->getName(); } -void DuplicateActionControlPlaneNameCheck::checkForDuplicateName(cstring name, const IR::Node *node) { +void DuplicateActionControlPlaneNameCheck::checkForDuplicateName(cstring name, + const IR::Node *node) { bool foundDuplicate = false; auto *otherNode = node; auto [it, inserted] = actions.insert(std::pair(name, node)); diff --git a/testdata/p4_16_samples/actions-almost-duplicate-names1.p4 b/testdata/p4_16_samples/actions-almost-duplicate-names1.p4 new file mode 100644 index 00000000000..15084de9388 --- /dev/null +++ b/testdata/p4_16_samples/actions-almost-duplicate-names1.p4 @@ -0,0 +1,152 @@ +/* +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 +#include + +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("foo2") +action foo2() { +} + +@name(".baz") +action foo3() { +} + +control ingressImpl( + inout headers_t hdr, + inout metadata_t meta, + inout standard_metadata_t stdmeta) +{ + bit<8> tmp1; + bit<8> tmp2; + + // This is not a name conflict with foo1, because it has a top + // level name ".foo1", but a1 will have hierarchical name + // "ingressImpl.foo1". + @name("foo1") action a1 (bit<8> x, bit<8> y) { tmp1 = x >> 1; tmp2 = y; } + + // This should not be a name conflict with foo2, because it has a + // top level name ".foo2", but a2 will have hierarchical name + // "ingressImpl.foo2". + // TODO: However, it is a @name conflict in the current p4c + // implementation until a PR like #4970 is merged in. + //@name("foo2") action a2 (bit<8> x, bit<8> y) { tmp1 = x >> 2; tmp2 = y; } + + @name(".bar") action a3 (bit<8> x, bit<8> y) { tmp1 = x >> 3; tmp2 = y; } + // This is not a name conflict with a3, because it has a top level + // name ".bar", but a4 will have hierarchical name + // "ingressImpl.bar". + @name("bar") action a4 (bit<8> x, bit<8> y) { tmp1 = x >> 4; tmp2 = y; } + + // This is not a name conflict with foo3, because it has a top + // level name ".baz", but a5 will have hierarchical name + // "ingressImpl.baz". + @name("baz") action a5 (bit<8> x, bit<8> y) { tmp1 = x >> 5; tmp2 = y; } + + table t1 { + actions = { + NoAction; + a1; + //a2; + a3; + a4; + a5; + foo1; + foo2; + foo3; + } + key = { hdr.ethernet.etherType: exact; } + default_action = NoAction(); + size = 512; + } + apply { + tmp1 = hdr.ethernet.srcAddr[7:0]; + tmp2 = hdr.ethernet.dstAddr[7:0]; + t1.apply(); + // This is here simply to ensure that the compiler cannot + // optimize away the effects of t1 and t2, which can only + // assign values to variables tmp1 and tmp2. + hdr.ethernet.etherType = (bit<16>) (tmp1 - tmp2); + } +} + +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/actions-almost-duplicate-names1-first.p4 b/testdata/p4_16_samples_outputs/actions-almost-duplicate-names1-first.p4 new file mode 100644 index 00000000000..d606f50225e --- /dev/null +++ b/testdata/p4_16_samples_outputs/actions-almost-duplicate-names1-first.p4 @@ -0,0 +1,97 @@ +#include +#define V1MODEL_VERSION 20180101 +#include + +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("foo2") action foo2() { +} +@name(".baz") action foo3() { +} +control ingressImpl(inout headers_t hdr, inout metadata_t meta, inout standard_metadata_t stdmeta) { + bit<8> tmp1; + bit<8> tmp2; + @name("foo1") action a1(bit<8> x, bit<8> y) { + tmp1 = x >> 1; + tmp2 = y; + } + @name(".bar") action a3(bit<8> x, bit<8> y) { + tmp1 = x >> 3; + tmp2 = y; + } + @name("bar") action a4(bit<8> x, bit<8> y) { + tmp1 = x >> 4; + tmp2 = y; + } + @name("baz") action a5(bit<8> x, bit<8> y) { + tmp1 = x >> 5; + tmp2 = y; + } + table t1 { + actions = { + NoAction(); + a1(); + a3(); + a4(); + a5(); + foo1(); + foo2(); + foo3(); + } + key = { + hdr.ethernet.etherType: exact @name("hdr.ethernet.etherType"); + } + default_action = NoAction(); + size = 512; + } + apply { + tmp1 = hdr.ethernet.srcAddr[7:0]; + tmp2 = hdr.ethernet.dstAddr[7:0]; + t1.apply(); + hdr.ethernet.etherType = (bit<16>)(tmp1 - tmp2); + } +} + +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/actions-almost-duplicate-names1-frontend.p4 b/testdata/p4_16_samples_outputs/actions-almost-duplicate-names1-frontend.p4 new file mode 100644 index 00000000000..71981918730 --- /dev/null +++ b/testdata/p4_16_samples_outputs/actions-almost-duplicate-names1-frontend.p4 @@ -0,0 +1,99 @@ +#include +#define V1MODEL_VERSION 20180101 +#include + +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 { + } +} + +control ingressImpl(inout headers_t hdr, inout metadata_t meta, inout standard_metadata_t stdmeta) { + @name("ingressImpl.tmp1") bit<8> tmp1_0; + @name("ingressImpl.tmp2") bit<8> tmp2_0; + @noWarn("unused") @name(".NoAction") action NoAction_1() { + } + @name(".foo1") action foo1_0() { + } + @name("ingressImpl.foo2") action foo2_0() { + } + @name(".baz") action foo3_0() { + } + @name("ingressImpl.foo1") action a1(@name("x") bit<8> x, @name("y") bit<8> y) { + tmp1_0 = x >> 1; + tmp2_0 = y; + } + @name(".bar") action a3(@name("x") bit<8> x_4, @name("y") bit<8> y_4) { + tmp1_0 = x_4 >> 3; + tmp2_0 = y_4; + } + @name("ingressImpl.bar") action a4(@name("x") bit<8> x_5, @name("y") bit<8> y_5) { + tmp1_0 = x_5 >> 4; + tmp2_0 = y_5; + } + @name("ingressImpl.baz") action a5(@name("x") bit<8> x_6, @name("y") bit<8> y_6) { + tmp1_0 = x_6 >> 5; + tmp2_0 = y_6; + } + @name("ingressImpl.t1") table t1_0 { + actions = { + NoAction_1(); + a1(); + a3(); + a4(); + a5(); + foo1_0(); + foo2_0(); + foo3_0(); + } + key = { + hdr.ethernet.etherType: exact @name("hdr.ethernet.etherType"); + } + default_action = NoAction_1(); + size = 512; + } + apply { + tmp1_0 = hdr.ethernet.srcAddr[7:0]; + tmp2_0 = hdr.ethernet.dstAddr[7:0]; + t1_0.apply(); + hdr.ethernet.etherType = (bit<16>)(tmp1_0 - tmp2_0); + } +} + +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/actions-almost-duplicate-names1-midend.p4 b/testdata/p4_16_samples_outputs/actions-almost-duplicate-names1-midend.p4 new file mode 100644 index 00000000000..d1cca333c64 --- /dev/null +++ b/testdata/p4_16_samples_outputs/actions-almost-duplicate-names1-midend.p4 @@ -0,0 +1,116 @@ +#include +#define V1MODEL_VERSION 20180101 +#include + +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(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) { + @name("ingressImpl.tmp1") bit<8> tmp1_0; + @name("ingressImpl.tmp2") bit<8> tmp2_0; + @noWarn("unused") @name(".NoAction") action NoAction_1() { + } + @name(".foo1") action foo1_0() { + } + @name("ingressImpl.foo2") action foo2_0() { + } + @name(".baz") action foo3_0() { + } + @name("ingressImpl.foo1") action a1(@name("x") bit<8> x, @name("y") bit<8> y) { + tmp1_0 = x >> 1; + tmp2_0 = y; + } + @name(".bar") action a3(@name("x") bit<8> x_4, @name("y") bit<8> y_4) { + tmp1_0 = x_4 >> 3; + tmp2_0 = y_4; + } + @name("ingressImpl.bar") action a4(@name("x") bit<8> x_5, @name("y") bit<8> y_5) { + tmp1_0 = x_5 >> 4; + tmp2_0 = y_5; + } + @name("ingressImpl.baz") action a5(@name("x") bit<8> x_6, @name("y") bit<8> y_6) { + tmp1_0 = x_6 >> 5; + tmp2_0 = y_6; + } + @name("ingressImpl.t1") table t1_0 { + actions = { + NoAction_1(); + a1(); + a3(); + a4(); + a5(); + foo1_0(); + foo2_0(); + foo3_0(); + } + key = { + hdr.ethernet.etherType: exact @name("hdr.ethernet.etherType"); + } + default_action = NoAction_1(); + size = 512; + } + @hidden action actionsalmostduplicatenames1l113() { + tmp1_0 = hdr.ethernet.srcAddr[7:0]; + tmp2_0 = hdr.ethernet.dstAddr[7:0]; + } + @hidden action actionsalmostduplicatenames1l119() { + hdr.ethernet.etherType = (bit<16>)(tmp1_0 - tmp2_0); + } + @hidden table tbl_actionsalmostduplicatenames1l113 { + actions = { + actionsalmostduplicatenames1l113(); + } + const default_action = actionsalmostduplicatenames1l113(); + } + @hidden table tbl_actionsalmostduplicatenames1l119 { + actions = { + actionsalmostduplicatenames1l119(); + } + const default_action = actionsalmostduplicatenames1l119(); + } + apply { + tbl_actionsalmostduplicatenames1l113.apply(); + t1_0.apply(); + tbl_actionsalmostduplicatenames1l119.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/actions-almost-duplicate-names1.p4 b/testdata/p4_16_samples_outputs/actions-almost-duplicate-names1.p4 new file mode 100644 index 00000000000..a85e79f001b --- /dev/null +++ b/testdata/p4_16_samples_outputs/actions-almost-duplicate-names1.p4 @@ -0,0 +1,97 @@ +#include +#define V1MODEL_VERSION 20180101 +#include + +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("foo2") action foo2() { +} +@name(".baz") action foo3() { +} +control ingressImpl(inout headers_t hdr, inout metadata_t meta, inout standard_metadata_t stdmeta) { + bit<8> tmp1; + bit<8> tmp2; + @name("foo1") action a1(bit<8> x, bit<8> y) { + tmp1 = x >> 1; + tmp2 = y; + } + @name(".bar") action a3(bit<8> x, bit<8> y) { + tmp1 = x >> 3; + tmp2 = y; + } + @name("bar") action a4(bit<8> x, bit<8> y) { + tmp1 = x >> 4; + tmp2 = y; + } + @name("baz") action a5(bit<8> x, bit<8> y) { + tmp1 = x >> 5; + tmp2 = y; + } + table t1 { + actions = { + NoAction; + a1; + a3; + a4; + a5; + foo1; + foo2; + foo3; + } + key = { + hdr.ethernet.etherType: exact; + } + default_action = NoAction(); + size = 512; + } + apply { + tmp1 = hdr.ethernet.srcAddr[7:0]; + tmp2 = hdr.ethernet.dstAddr[7:0]; + t1.apply(); + hdr.ethernet.etherType = (bit<16>)(tmp1 - tmp2); + } +} + +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/actions-almost-duplicate-names1.p4-stderr b/testdata/p4_16_samples_outputs/actions-almost-duplicate-names1.p4-stderr new file mode 100644 index 00000000000..e69de29bb2d diff --git a/testdata/p4_16_samples_outputs/actions-almost-duplicate-names1.p4.entries.txtpb b/testdata/p4_16_samples_outputs/actions-almost-duplicate-names1.p4.entries.txtpb new file mode 100644 index 00000000000..5cb9652623a --- /dev/null +++ b/testdata/p4_16_samples_outputs/actions-almost-duplicate-names1.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/actions-almost-duplicate-names1.p4.p4info.txtpb b/testdata/p4_16_samples_outputs/actions-almost-duplicate-names1.p4.p4info.txtpb new file mode 100644 index 00000000000..76b13ff0639 --- /dev/null +++ b/testdata/p4_16_samples_outputs/actions-almost-duplicate-names1.p4.p4info.txtpb @@ -0,0 +1,146 @@ +# proto-file: p4/config/v1/p4info.proto +# proto-message: p4.config.v1.P4Info + +pkg_info { + arch: "v1model" +} +tables { + preamble { + id: 49173205 + name: "ingressImpl.t1" + alias: "t1" + } + match_fields { + id: 1 + name: "hdr.ethernet.etherType" + bitwidth: 16 + match_type: EXACT + } + action_refs { + id: 21257015 + } + action_refs { + id: 30176274 + } + action_refs { + id: 21008649 + } + action_refs { + id: 27458909 + } + action_refs { + id: 24058105 + } + action_refs { + id: 25646030 + } + action_refs { + id: 17174663 + } + action_refs { + id: 28708850 + } + initial_default_action { + action_id: 21257015 + } + size: 512 +} +actions { + preamble { + id: 21257015 + name: "NoAction" + alias: "NoAction" + annotations: "@noWarn(\"unused\")" + } +} +actions { + preamble { + id: 25646030 + name: "foo1" + alias: "foo1" + } +} +actions { + preamble { + id: 17174663 + name: "ingressImpl.foo2" + alias: "foo2" + } +} +actions { + preamble { + id: 28708850 + name: "baz" + alias: "baz" + } +} +actions { + preamble { + id: 30176274 + name: "ingressImpl.foo1" + alias: "ingressImpl.foo1" + } + params { + id: 1 + name: "x" + bitwidth: 8 + } + params { + id: 2 + name: "y" + bitwidth: 8 + } +} +actions { + preamble { + id: 21008649 + name: "bar" + alias: "bar" + } + params { + id: 1 + name: "x" + bitwidth: 8 + } + params { + id: 2 + name: "y" + bitwidth: 8 + } +} +actions { + preamble { + id: 27458909 + name: "ingressImpl.bar" + alias: "ingressImpl.bar" + } + params { + id: 1 + name: "x" + bitwidth: 8 + } + params { + id: 2 + name: "y" + bitwidth: 8 + } +} +actions { + preamble { + id: 24058105 + name: "ingressImpl.baz" + alias: "ingressImpl.baz" + } + params { + id: 1 + name: "x" + bitwidth: 8 + } + params { + id: 2 + name: "y" + bitwidth: 8 + } +} +type_info { +} From 95dc4e2b68e4b7256e6c316ab9d1a274c9fc4001 Mon Sep 17 00:00:00 2001 From: Andy Fingerhut Date: Tue, 22 Oct 2024 16:47:01 -0400 Subject: [PATCH 29/48] A few more fixes and test cases for duplicate action name checks Signed-off-by: Andy Fingerhut --- .../duplicateActionControlPlaneNameCheck.cpp | 15 ++++--- ...ons-with-duplicate-control-plane-names1.p4 | 41 +++++++++++++----- ...th-duplicate-control-plane-names1-first.p4 | 22 ++++++++-- ...ons-with-duplicate-control-plane-names1.p4 | 22 ++++++++-- ...h-duplicate-control-plane-names1.p4-stderr | 24 +++++++++-- .../actions-almost-duplicate-names1.p4 | 6 +-- .../actions-almost-duplicate-names1-first.p4 | 5 +++ ...ctions-almost-duplicate-names1-frontend.p4 | 23 ++++++---- .../actions-almost-duplicate-names1-midend.p4 | 43 +++++++++++-------- .../actions-almost-duplicate-names1.p4 | 5 +++ ...ns-almost-duplicate-names1.p4.p4info.txtpb | 26 +++++++++-- tools/ptf/base_test.py | 23 ++++++++-- 12 files changed, 190 insertions(+), 65 deletions(-) diff --git a/frontends/p4/duplicateActionControlPlaneNameCheck.cpp b/frontends/p4/duplicateActionControlPlaneNameCheck.cpp index 13dd2b1c1e1..e891afd5648 100644 --- a/frontends/p4/duplicateActionControlPlaneNameCheck.cpp +++ b/frontends/p4/duplicateActionControlPlaneNameCheck.cpp @@ -78,11 +78,16 @@ const IR::Node *DuplicateActionControlPlaneNameCheck::postorder(IR::Annotation * } cstring name = annotation->getName(); if (!name.startsWith(".")) { - if (!stack.empty()) { - name = absl::StrCat(absl::StrJoin(stack, ".", - [](std::string *out, cstring s) { - absl::StrAppend(out, s.string_view()); - }), + // 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()); } } diff --git a/testdata/p4_16_errors/actions-with-duplicate-control-plane-names1.p4 b/testdata/p4_16_errors/actions-with-duplicate-control-plane-names1.p4 index 561174209c8..d8825d54042 100644 --- a/testdata/p4_16_errors/actions-with-duplicate-control-plane-names1.p4 +++ b/testdata/p4_16_errors/actions-with-duplicate-control-plane-names1.p4 @@ -55,12 +55,18 @@ control verifyChecksum( apply { } } -action foo1() { -} +action foo1() { } -@name("foo2") -action bar() { -} +@name("foo2") action topa2() { } + +@name("foo3") action topa3() { } + +// Action topa4's control plane name should conflict with the control +// plane name of action topa3, and cause a compile-time error message, +// if you enable P4Info file generation. +@name(".foo3") action topa4() { } + +@name(".ingressImpl.foo5") action topa5() { } control ingressImpl( inout headers_t hdr, @@ -74,15 +80,30 @@ control ingressImpl( // Action a1's control plane name should conflict with the name of // top-level action foo1, and cause a compile-time error message, // if you enable P4Info file generation. - @name(".foo1") action a1 (bit<8> x, bit<8> y) { tmp1 = x; tmp2 = y; } + @name(".foo1") action a1 (bit<8> x, bit<8> y) { tmp1 = x >> 1; tmp2 = y; } // Action a2's control plane name should conflict with the control - // plane name of top-level action bar, and cause a compile-time - // error message, if you enable P4Info file generation. - @name(".foo2") action a2 (bit<8> x, bit<8> y) { tmp1 = x; tmp2 = y; } + // plane name of topa2, and cause a compile-time error message, if + // you enable P4Info file generation. + @name(".foo2") action a2 (bit<8> x, bit<8> y) { tmp1 = x >> 2; tmp2 = y; } + + // Action a5's control plane name should conflict with the control + // plane name of topa5, and cause a compile-time error message, if + // you enable P4Info file generation. + @name("foo5") action a5 (bit<8> x, bit<8> y) { tmp1 = x >> 4; tmp2 = y; } table t1 { - actions = { NoAction; a1; a2; foo1; bar; } + actions = { + NoAction; + a1; + a2; + a5; + foo1; + topa2; + topa3; + topa4; + topa5; + } key = { hdr.ethernet.etherType: exact; } default_action = NoAction(); size = 512; diff --git a/testdata/p4_16_errors_outputs/actions-with-duplicate-control-plane-names1-first.p4 b/testdata/p4_16_errors_outputs/actions-with-duplicate-control-plane-names1-first.p4 index 483e7b604f8..ba44d2e99ba 100644 --- a/testdata/p4_16_errors_outputs/actions-with-duplicate-control-plane-names1-first.p4 +++ b/testdata/p4_16_errors_outputs/actions-with-duplicate-control-plane-names1-first.p4 @@ -30,17 +30,27 @@ control verifyChecksum(inout headers_t hdr, inout metadata_t meta) { action foo1() { } -@name("foo2") action bar() { +@name("foo2") action topa2() { +} +@name("foo3") action topa3() { +} +@name(".foo3") action topa4() { +} +@name(".ingressImpl.foo5") action topa5() { } control ingressImpl(inout headers_t hdr, inout metadata_t meta, inout standard_metadata_t stdmeta) { bit<8> tmp1; bit<8> tmp2; @name(".foo1") action a1(bit<8> x, bit<8> y) { - tmp1 = x; + tmp1 = x >> 1; tmp2 = y; } @name(".foo2") action a2(bit<8> x, bit<8> y) { - tmp1 = x; + tmp1 = x >> 2; + tmp2 = y; + } + @name("foo5") action a5(bit<8> x, bit<8> y) { + tmp1 = x >> 4; tmp2 = y; } table t1 { @@ -48,8 +58,12 @@ control ingressImpl(inout headers_t hdr, inout metadata_t meta, inout standard_m NoAction(); a1(); a2(); + a5(); foo1(); - bar(); + topa2(); + topa3(); + topa4(); + topa5(); } key = { hdr.ethernet.etherType: exact @name("hdr.ethernet.etherType"); diff --git a/testdata/p4_16_errors_outputs/actions-with-duplicate-control-plane-names1.p4 b/testdata/p4_16_errors_outputs/actions-with-duplicate-control-plane-names1.p4 index 31e4f5d82f8..98ded802664 100644 --- a/testdata/p4_16_errors_outputs/actions-with-duplicate-control-plane-names1.p4 +++ b/testdata/p4_16_errors_outputs/actions-with-duplicate-control-plane-names1.p4 @@ -30,17 +30,27 @@ control verifyChecksum(inout headers_t hdr, inout metadata_t meta) { action foo1() { } -@name("foo2") action bar() { +@name("foo2") action topa2() { +} +@name("foo3") action topa3() { +} +@name(".foo3") action topa4() { +} +@name(".ingressImpl.foo5") action topa5() { } control ingressImpl(inout headers_t hdr, inout metadata_t meta, inout standard_metadata_t stdmeta) { bit<8> tmp1; bit<8> tmp2; @name(".foo1") action a1(bit<8> x, bit<8> y) { - tmp1 = x; + tmp1 = x >> 1; tmp2 = y; } @name(".foo2") action a2(bit<8> x, bit<8> y) { - tmp1 = x; + tmp1 = x >> 2; + tmp2 = y; + } + @name("foo5") action a5(bit<8> x, bit<8> y) { + tmp1 = x >> 4; tmp2 = y; } table t1 { @@ -48,8 +58,12 @@ control ingressImpl(inout headers_t hdr, inout metadata_t meta, inout standard_m NoAction; a1; a2; + a5; foo1; - bar; + topa2; + topa3; + topa4; + topa5; } key = { hdr.ethernet.etherType: exact; diff --git a/testdata/p4_16_errors_outputs/actions-with-duplicate-control-plane-names1.p4-stderr b/testdata/p4_16_errors_outputs/actions-with-duplicate-control-plane-names1.p4-stderr index 29506558ce9..e9fdab29466 100644 --- a/testdata/p4_16_errors_outputs/actions-with-duplicate-control-plane-names1.p4-stderr +++ b/testdata/p4_16_errors_outputs/actions-with-duplicate-control-plane-names1.p4-stderr @@ -1,6 +1,24 @@ -actions-with-duplicate-control-plane-names1.p4(77): [--Werror=duplicate] error: .foo1: conflicting control plane name: foo1 - @name(".foo1") action a1 (bit<8> x, bit<8> y) { tmp1 = x; tmp2 = y; } +actions-with-duplicate-control-plane-names1.p4(67): [--Werror=duplicate] error: .foo3: conflicting control plane name: foo3 +@name(".foo3") action topa4() { } + ^^^^^ +actions-with-duplicate-control-plane-names1.p4(62) +@name("foo3") action topa3() { } + ^^^^^ +actions-with-duplicate-control-plane-names1.p4(83): [--Werror=duplicate] error: .foo1: conflicting control plane name: foo1 + @name(".foo1") action a1 (bit<8> x, bit<8> y) { tmp1 = x >> 1; tmp2 = y; } ^^ actions-with-duplicate-control-plane-names1.p4(58) -action foo1() { +action foo1() { } ^^^^ +actions-with-duplicate-control-plane-names1.p4(88): [--Werror=duplicate] error: .foo2: conflicting control plane name: foo2 + @name(".foo2") action a2 (bit<8> x, bit<8> y) { tmp1 = x >> 2; tmp2 = y; } + ^^ +actions-with-duplicate-control-plane-names1.p4(60) +@name("foo2") action topa2() { } + ^^^^^ +actions-with-duplicate-control-plane-names1.p4(93): [--Werror=duplicate] error: foo5: conflicting control plane name: .ingressImpl.foo5 + @name("foo5") action a5 (bit<8> x, bit<8> y) { tmp1 = x >> 4; tmp2 = y; } + ^^ +actions-with-duplicate-control-plane-names1.p4(69) +@name(".ingressImpl.foo5") action topa5() { } + ^^^^^ diff --git a/testdata/p4_16_samples/actions-almost-duplicate-names1.p4 b/testdata/p4_16_samples/actions-almost-duplicate-names1.p4 index 15084de9388..52a4ae6eced 100644 --- a/testdata/p4_16_samples/actions-almost-duplicate-names1.p4 +++ b/testdata/p4_16_samples/actions-almost-duplicate-names1.p4 @@ -78,9 +78,7 @@ control ingressImpl( // This should not be a name conflict with foo2, because it has a // top level name ".foo2", but a2 will have hierarchical name // "ingressImpl.foo2". - // TODO: However, it is a @name conflict in the current p4c - // implementation until a PR like #4970 is merged in. - //@name("foo2") action a2 (bit<8> x, bit<8> y) { tmp1 = x >> 2; tmp2 = y; } + @name("foo2") action a2 (bit<8> x, bit<8> y) { tmp1 = x >> 2; tmp2 = y; } @name(".bar") action a3 (bit<8> x, bit<8> y) { tmp1 = x >> 3; tmp2 = y; } // This is not a name conflict with a3, because it has a top level @@ -97,7 +95,7 @@ control ingressImpl( actions = { NoAction; a1; - //a2; + a2; a3; a4; a5; diff --git a/testdata/p4_16_samples_outputs/actions-almost-duplicate-names1-first.p4 b/testdata/p4_16_samples_outputs/actions-almost-duplicate-names1-first.p4 index d606f50225e..af35a852843 100644 --- a/testdata/p4_16_samples_outputs/actions-almost-duplicate-names1-first.p4 +++ b/testdata/p4_16_samples_outputs/actions-almost-duplicate-names1-first.p4 @@ -41,6 +41,10 @@ control ingressImpl(inout headers_t hdr, inout metadata_t meta, inout standard_m tmp1 = x >> 1; tmp2 = y; } + @name("foo2") action a2(bit<8> x, bit<8> y) { + tmp1 = x >> 2; + tmp2 = y; + } @name(".bar") action a3(bit<8> x, bit<8> y) { tmp1 = x >> 3; tmp2 = y; @@ -57,6 +61,7 @@ control ingressImpl(inout headers_t hdr, inout metadata_t meta, inout standard_m actions = { NoAction(); a1(); + a2(); a3(); a4(); a5(); diff --git a/testdata/p4_16_samples_outputs/actions-almost-duplicate-names1-frontend.p4 b/testdata/p4_16_samples_outputs/actions-almost-duplicate-names1-frontend.p4 index 71981918730..b39c443220a 100644 --- a/testdata/p4_16_samples_outputs/actions-almost-duplicate-names1-frontend.p4 +++ b/testdata/p4_16_samples_outputs/actions-almost-duplicate-names1-frontend.p4 @@ -35,7 +35,7 @@ control ingressImpl(inout headers_t hdr, inout metadata_t meta, inout standard_m } @name(".foo1") action foo1_0() { } - @name("ingressImpl.foo2") action foo2_0() { + @name(".foo2") action foo2_0() { } @name(".baz") action foo3_0() { } @@ -43,22 +43,27 @@ control ingressImpl(inout headers_t hdr, inout metadata_t meta, inout standard_m tmp1_0 = x >> 1; tmp2_0 = y; } - @name(".bar") action a3(@name("x") bit<8> x_4, @name("y") bit<8> y_4) { - tmp1_0 = x_4 >> 3; - tmp2_0 = y_4; - } - @name("ingressImpl.bar") action a4(@name("x") bit<8> x_5, @name("y") bit<8> y_5) { - tmp1_0 = x_5 >> 4; + @name("ingressImpl.foo2") action a2(@name("x") bit<8> x_5, @name("y") bit<8> y_5) { + tmp1_0 = x_5 >> 2; tmp2_0 = y_5; } - @name("ingressImpl.baz") action a5(@name("x") bit<8> x_6, @name("y") bit<8> y_6) { - tmp1_0 = x_6 >> 5; + @name(".bar") action a3(@name("x") bit<8> x_6, @name("y") bit<8> y_6) { + tmp1_0 = x_6 >> 3; tmp2_0 = y_6; } + @name("ingressImpl.bar") action a4(@name("x") bit<8> x_7, @name("y") bit<8> y_7) { + tmp1_0 = x_7 >> 4; + tmp2_0 = y_7; + } + @name("ingressImpl.baz") action a5(@name("x") bit<8> x_8, @name("y") bit<8> y_8) { + tmp1_0 = x_8 >> 5; + tmp2_0 = y_8; + } @name("ingressImpl.t1") table t1_0 { actions = { NoAction_1(); a1(); + a2(); a3(); a4(); a5(); diff --git a/testdata/p4_16_samples_outputs/actions-almost-duplicate-names1-midend.p4 b/testdata/p4_16_samples_outputs/actions-almost-duplicate-names1-midend.p4 index d1cca333c64..d8e76d20851 100644 --- a/testdata/p4_16_samples_outputs/actions-almost-duplicate-names1-midend.p4 +++ b/testdata/p4_16_samples_outputs/actions-almost-duplicate-names1-midend.p4 @@ -34,7 +34,7 @@ control ingressImpl(inout headers_t hdr, inout metadata_t meta, inout standard_m } @name(".foo1") action foo1_0() { } - @name("ingressImpl.foo2") action foo2_0() { + @name(".foo2") action foo2_0() { } @name(".baz") action foo3_0() { } @@ -42,22 +42,27 @@ control ingressImpl(inout headers_t hdr, inout metadata_t meta, inout standard_m tmp1_0 = x >> 1; tmp2_0 = y; } - @name(".bar") action a3(@name("x") bit<8> x_4, @name("y") bit<8> y_4) { - tmp1_0 = x_4 >> 3; - tmp2_0 = y_4; - } - @name("ingressImpl.bar") action a4(@name("x") bit<8> x_5, @name("y") bit<8> y_5) { - tmp1_0 = x_5 >> 4; + @name("ingressImpl.foo2") action a2(@name("x") bit<8> x_5, @name("y") bit<8> y_5) { + tmp1_0 = x_5 >> 2; tmp2_0 = y_5; } - @name("ingressImpl.baz") action a5(@name("x") bit<8> x_6, @name("y") bit<8> y_6) { - tmp1_0 = x_6 >> 5; + @name(".bar") action a3(@name("x") bit<8> x_6, @name("y") bit<8> y_6) { + tmp1_0 = x_6 >> 3; tmp2_0 = y_6; } + @name("ingressImpl.bar") action a4(@name("x") bit<8> x_7, @name("y") bit<8> y_7) { + tmp1_0 = x_7 >> 4; + tmp2_0 = y_7; + } + @name("ingressImpl.baz") action a5(@name("x") bit<8> x_8, @name("y") bit<8> y_8) { + tmp1_0 = x_8 >> 5; + tmp2_0 = y_8; + } @name("ingressImpl.t1") table t1_0 { actions = { NoAction_1(); a1(); + a2(); a3(); a4(); a5(); @@ -71,29 +76,29 @@ control ingressImpl(inout headers_t hdr, inout metadata_t meta, inout standard_m default_action = NoAction_1(); size = 512; } - @hidden action actionsalmostduplicatenames1l113() { + @hidden action actionsalmostduplicatenames1l111() { tmp1_0 = hdr.ethernet.srcAddr[7:0]; tmp2_0 = hdr.ethernet.dstAddr[7:0]; } - @hidden action actionsalmostduplicatenames1l119() { + @hidden action actionsalmostduplicatenames1l117() { hdr.ethernet.etherType = (bit<16>)(tmp1_0 - tmp2_0); } - @hidden table tbl_actionsalmostduplicatenames1l113 { + @hidden table tbl_actionsalmostduplicatenames1l111 { actions = { - actionsalmostduplicatenames1l113(); + actionsalmostduplicatenames1l111(); } - const default_action = actionsalmostduplicatenames1l113(); + const default_action = actionsalmostduplicatenames1l111(); } - @hidden table tbl_actionsalmostduplicatenames1l119 { + @hidden table tbl_actionsalmostduplicatenames1l117 { actions = { - actionsalmostduplicatenames1l119(); + actionsalmostduplicatenames1l117(); } - const default_action = actionsalmostduplicatenames1l119(); + const default_action = actionsalmostduplicatenames1l117(); } apply { - tbl_actionsalmostduplicatenames1l113.apply(); + tbl_actionsalmostduplicatenames1l111.apply(); t1_0.apply(); - tbl_actionsalmostduplicatenames1l119.apply(); + tbl_actionsalmostduplicatenames1l117.apply(); } } diff --git a/testdata/p4_16_samples_outputs/actions-almost-duplicate-names1.p4 b/testdata/p4_16_samples_outputs/actions-almost-duplicate-names1.p4 index a85e79f001b..97b85ede8dc 100644 --- a/testdata/p4_16_samples_outputs/actions-almost-duplicate-names1.p4 +++ b/testdata/p4_16_samples_outputs/actions-almost-duplicate-names1.p4 @@ -41,6 +41,10 @@ control ingressImpl(inout headers_t hdr, inout metadata_t meta, inout standard_m tmp1 = x >> 1; tmp2 = y; } + @name("foo2") action a2(bit<8> x, bit<8> y) { + tmp1 = x >> 2; + tmp2 = y; + } @name(".bar") action a3(bit<8> x, bit<8> y) { tmp1 = x >> 3; tmp2 = y; @@ -57,6 +61,7 @@ control ingressImpl(inout headers_t hdr, inout metadata_t meta, inout standard_m actions = { NoAction; a1; + a2; a3; a4; a5; diff --git a/testdata/p4_16_samples_outputs/actions-almost-duplicate-names1.p4.p4info.txtpb b/testdata/p4_16_samples_outputs/actions-almost-duplicate-names1.p4.p4info.txtpb index 76b13ff0639..184fa761217 100644 --- a/testdata/p4_16_samples_outputs/actions-almost-duplicate-names1.p4.p4info.txtpb +++ b/testdata/p4_16_samples_outputs/actions-almost-duplicate-names1.p4.p4info.txtpb @@ -22,6 +22,9 @@ tables { action_refs { id: 30176274 } + action_refs { + id: 17174663 + } action_refs { id: 21008649 } @@ -35,7 +38,7 @@ tables { id: 25646030 } action_refs { - id: 17174663 + id: 24901046 } action_refs { id: 28708850 @@ -62,8 +65,8 @@ actions { } actions { preamble { - id: 17174663 - name: "ingressImpl.foo2" + id: 24901046 + name: "foo2" alias: "foo2" } } @@ -91,6 +94,23 @@ actions { bitwidth: 8 } } +actions { + preamble { + id: 17174663 + name: "ingressImpl.foo2" + alias: "ingressImpl.foo2" + } + params { + id: 1 + name: "x" + bitwidth: 8 + } + params { + id: 2 + name: "y" + bitwidth: 8 + } +} actions { preamble { id: 21008649 diff --git a/tools/ptf/base_test.py b/tools/ptf/base_test.py index 79bf2c483cc..708f55a1abd 100644 --- a/tools/ptf/base_test.py +++ b/tools/ptf/base_test.py @@ -286,14 +286,30 @@ def import_p4info_names(self): "meters", "direct_meters", ]: + # First add the full hierarchical names of all objects. + full_names = {} + for obj in getattr(self.p4info, obj_type): + key = (obj_type, obj.preamble.name) + self.p4info_obj_map[key] = obj + full_names[key] = True + # Now add suffixes of hierarchical names, but only if they + # are not equal to any full name. + # Example: If we have two objects, obj1 with name + # "foo.bar.baz" and obj2 with name "baz", at the end we + # should have the name "baz" referring to obj2 and both + # the full name "foo.bar.baz" and the suffix "bar.baz" + # referring to obj1. for obj in getattr(self.p4info, obj_type): pre = obj.preamble suffix = None for s in reversed(pre.name.split(".")): suffix = s if suffix is None else s + "." + suffix key = (obj_type, suffix) - self.p4info_obj_map[key] = obj - suffix_count[key] += 1 + if key not in full_names: + self.p4info_obj_map[key] = obj + suffix_count[key] += 1 + # Now remove any suffixes that are ambiguous, because they are + # equal to some other suffix. for key, c in list(suffix_count.items()): if c > 1: del self.p4info_obj_map[key] @@ -631,8 +647,7 @@ def set_action(self, action, a_name, params): action_id = self.get_action_id(a_name) if action_id is None: self.fail( - "Failed to get id of action '{}' - perhaps the action name is misspelled?" - ).format(a_name) + "Failed to get id of action '{}' - perhaps the action name is misspelled?".format(a_name)) action.action_id = action_id for p_name, v in params: param = action.params.add() From efd0d5201fdfa93f0f2c7504969fdecf82ea7ae5 Mon Sep 17 00:00:00 2001 From: Andy Fingerhut Date: Tue, 22 Oct 2024 16:52:29 -0400 Subject: [PATCH 30/48] Fix code formatting to pass lint check Signed-off-by: Andy Fingerhut --- frontends/p4/duplicateActionControlPlaneNameCheck.cpp | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/frontends/p4/duplicateActionControlPlaneNameCheck.cpp b/frontends/p4/duplicateActionControlPlaneNameCheck.cpp index e891afd5648..a2856eda43b 100644 --- a/frontends/p4/duplicateActionControlPlaneNameCheck.cpp +++ b/frontends/p4/duplicateActionControlPlaneNameCheck.cpp @@ -84,10 +84,11 @@ const IR::Node *DuplicateActionControlPlaneNameCheck::postorder(IR::Annotation * 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 = absl::StrCat(".", + absl::StrJoin(stack, ".", + [](std::string *out, cstring s) { + absl::StrAppend(out, s.string_view()); + }), ".", name.string_view()); } } From 4540f2a6bf7e4c53fd43b26c7dd1615fbdbdeadd Mon Sep 17 00:00:00 2001 From: Andy Fingerhut Date: Tue, 22 Oct 2024 19:17:56 -0400 Subject: [PATCH 31/48] Fix lint warning for Python code Signed-off-by: Andy Fingerhut --- tools/ptf/base_test.py | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/tools/ptf/base_test.py b/tools/ptf/base_test.py index 708f55a1abd..55323eb237f 100644 --- a/tools/ptf/base_test.py +++ b/tools/ptf/base_test.py @@ -647,7 +647,10 @@ def set_action(self, action, a_name, params): action_id = self.get_action_id(a_name) if action_id is None: self.fail( - "Failed to get id of action '{}' - perhaps the action name is misspelled?".format(a_name)) + "Failed to get id of action '{}' - perhaps the action name is misspelled?".format( + a_name + ) + ) action.action_id = action_id for p_name, v in params: param = action.params.add() From 39a9251af13a237212f5e9f24d91b4dd71c27288 Mon Sep 17 00:00:00 2001 From: Andy Fingerhut Date: Wed, 23 Oct 2024 12:45:46 -0400 Subject: [PATCH 32/48] Update old file name in CMakeLists.txt Signed-off-by: Andy Fingerhut --- frontends/CMakeLists.txt | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/frontends/CMakeLists.txt b/frontends/CMakeLists.txt index a44f4b1e192..dfaee0b517e 100644 --- a/frontends/CMakeLists.txt +++ b/frontends/CMakeLists.txt @@ -111,7 +111,7 @@ set (P4_FRONTEND_HDRS p4/frontend.h p4/functionsInlining.h p4/hierarchicalNames.h - p4/duplicateHierarchicalNameCheck.h + p4/duplicateActionControlPlaneNameCheck.h p4/inlining.h p4/localizeActions.h p4/methodInstance.h From b614ff97b0b46e78f3b79b9c7368e7ab198b61f2 Mon Sep 17 00:00:00 2001 From: Andy Fingerhut Date: Thu, 24 Oct 2024 20:32:46 -0400 Subject: [PATCH 33/48] Add another case of conflicting action names between control and sub-control Signed-off-by: Andy Fingerhut --- ...ons-with-duplicate-control-plane-names1.p4 | 40 ++++++++++++++++++- ...th-duplicate-control-plane-names1-first.p4 | 40 ++++++++++++++++++- ...ons-with-duplicate-control-plane-names1.p4 | 37 ++++++++++++++++- ...h-duplicate-control-plane-names1.p4-stderr | 26 ++++++++++-- 4 files changed, 136 insertions(+), 7 deletions(-) diff --git a/testdata/p4_16_errors/actions-with-duplicate-control-plane-names1.p4 b/testdata/p4_16_errors/actions-with-duplicate-control-plane-names1.p4 index d8825d54042..451af01cb68 100644 --- a/testdata/p4_16_errors/actions-with-duplicate-control-plane-names1.p4 +++ b/testdata/p4_16_errors/actions-with-duplicate-control-plane-names1.p4 @@ -68,6 +68,37 @@ action foo1() { } @name(".ingressImpl.foo5") action topa5() { } +control c1( + inout headers_t hdr, + inout bit<8> tmp1, + inout bit<8> tmp2) +{ + // Action suba1's control plane name should conflict with the name + // of top-level action foo1, and cause a compile-time error + // message, if you enable P4Info file generation. + @name(".foo1") action suba1 (bit<8> x, bit<8> y) { tmp1 = x; tmp2 = y >> 1; } + + // Action suba2's control plane name should conflict with the + // control plane name of topa2, and cause a compile-time error + // message, if you enable P4Info file generation. + @name(".foo2") action suba2 (bit<8> x, bit<8> y) { tmp1 = x; tmp2 = y >> 2; } + + @name("bar1") action suba3 (bit<8> x, bit<8> y) { tmp1 = x; tmp2 = y >> 3; } + + table t2 { + actions = { + suba1; + suba2; + suba3; + } + key = { hdr.ethernet.srcAddr: exact; } + size = 32; + } + apply { + t2.apply(); + } +} + control ingressImpl( inout headers_t hdr, inout metadata_t meta, @@ -90,7 +121,12 @@ control ingressImpl( // Action a5's control plane name should conflict with the control // plane name of topa5, and cause a compile-time error message, if // you enable P4Info file generation. - @name("foo5") action a5 (bit<8> x, bit<8> y) { tmp1 = x >> 4; tmp2 = y; } + @name("foo5") action a5 (bit<8> x, bit<8> y) { tmp1 = x >> 5; tmp2 = y; } + + // Action a6's control plane name should conflict with the + // control plane name of suba3, and cause a compile-time error + // message, if you enable P4Info file generation. + @name("c1.bar1") action a6 (bit<8> x, bit<8> y) { tmp1 = x >> 6; tmp2 = y; } table t1 { actions = { @@ -98,6 +134,7 @@ control ingressImpl( a1; a2; a5; + a6; foo1; topa2; topa3; @@ -111,6 +148,7 @@ control ingressImpl( apply { tmp1 = hdr.ethernet.srcAddr[7:0]; tmp2 = hdr.ethernet.dstAddr[7:0]; + c1.apply(hdr, tmp1, tmp2); t1.apply(); // This is here simply to ensure that the compiler cannot // optimize away the effects of t1 and t2, which can only diff --git a/testdata/p4_16_errors_outputs/actions-with-duplicate-control-plane-names1-first.p4 b/testdata/p4_16_errors_outputs/actions-with-duplicate-control-plane-names1-first.p4 index ba44d2e99ba..476197adb45 100644 --- a/testdata/p4_16_errors_outputs/actions-with-duplicate-control-plane-names1-first.p4 +++ b/testdata/p4_16_errors_outputs/actions-with-duplicate-control-plane-names1-first.p4 @@ -38,6 +38,37 @@ action foo1() { } @name(".ingressImpl.foo5") action topa5() { } +control c1(inout headers_t hdr, inout bit<8> tmp1, inout bit<8> tmp2) { + @name(".foo1") action suba1(bit<8> x, bit<8> y) { + tmp1 = x; + tmp2 = y >> 1; + } + @name(".foo2") action suba2(bit<8> x, bit<8> y) { + tmp1 = x; + tmp2 = y >> 2; + } + @name("bar1") action suba3(bit<8> x, bit<8> y) { + tmp1 = x; + tmp2 = y >> 3; + } + table t2 { + actions = { + suba1(); + suba2(); + suba3(); + @defaultonly NoAction(); + } + key = { + hdr.ethernet.srcAddr: exact @name("hdr.ethernet.srcAddr"); + } + size = 32; + default_action = NoAction(); + } + apply { + t2.apply(); + } +} + control ingressImpl(inout headers_t hdr, inout metadata_t meta, inout standard_metadata_t stdmeta) { bit<8> tmp1; bit<8> tmp2; @@ -50,7 +81,11 @@ control ingressImpl(inout headers_t hdr, inout metadata_t meta, inout standard_m tmp2 = y; } @name("foo5") action a5(bit<8> x, bit<8> y) { - tmp1 = x >> 4; + tmp1 = x >> 5; + tmp2 = y; + } + @name("c1.bar1") action a6(bit<8> x, bit<8> y) { + tmp1 = x >> 6; tmp2 = y; } table t1 { @@ -59,6 +94,7 @@ control ingressImpl(inout headers_t hdr, inout metadata_t meta, inout standard_m a1(); a2(); a5(); + a6(); foo1(); topa2(); topa3(); @@ -71,9 +107,11 @@ control ingressImpl(inout headers_t hdr, inout metadata_t meta, inout standard_m default_action = NoAction(); size = 512; } + @name("c1") c1() c1_inst; apply { tmp1 = hdr.ethernet.srcAddr[7:0]; tmp2 = hdr.ethernet.dstAddr[7:0]; + c1_inst.apply(hdr, tmp1, tmp2); t1.apply(); hdr.ethernet.etherType = (bit<16>)(tmp1 - tmp2); } diff --git a/testdata/p4_16_errors_outputs/actions-with-duplicate-control-plane-names1.p4 b/testdata/p4_16_errors_outputs/actions-with-duplicate-control-plane-names1.p4 index 98ded802664..e1e81b15afd 100644 --- a/testdata/p4_16_errors_outputs/actions-with-duplicate-control-plane-names1.p4 +++ b/testdata/p4_16_errors_outputs/actions-with-duplicate-control-plane-names1.p4 @@ -38,6 +38,35 @@ action foo1() { } @name(".ingressImpl.foo5") action topa5() { } +control c1(inout headers_t hdr, inout bit<8> tmp1, inout bit<8> tmp2) { + @name(".foo1") action suba1(bit<8> x, bit<8> y) { + tmp1 = x; + tmp2 = y >> 1; + } + @name(".foo2") action suba2(bit<8> x, bit<8> y) { + tmp1 = x; + tmp2 = y >> 2; + } + @name("bar1") action suba3(bit<8> x, bit<8> y) { + tmp1 = x; + tmp2 = y >> 3; + } + table t2 { + actions = { + suba1; + suba2; + suba3; + } + key = { + hdr.ethernet.srcAddr: exact; + } + size = 32; + } + apply { + t2.apply(); + } +} + control ingressImpl(inout headers_t hdr, inout metadata_t meta, inout standard_metadata_t stdmeta) { bit<8> tmp1; bit<8> tmp2; @@ -50,7 +79,11 @@ control ingressImpl(inout headers_t hdr, inout metadata_t meta, inout standard_m tmp2 = y; } @name("foo5") action a5(bit<8> x, bit<8> y) { - tmp1 = x >> 4; + tmp1 = x >> 5; + tmp2 = y; + } + @name("c1.bar1") action a6(bit<8> x, bit<8> y) { + tmp1 = x >> 6; tmp2 = y; } table t1 { @@ -59,6 +92,7 @@ control ingressImpl(inout headers_t hdr, inout metadata_t meta, inout standard_m a1; a2; a5; + a6; foo1; topa2; topa3; @@ -74,6 +108,7 @@ control ingressImpl(inout headers_t hdr, inout metadata_t meta, inout standard_m apply { tmp1 = hdr.ethernet.srcAddr[7:0]; tmp2 = hdr.ethernet.dstAddr[7:0]; + c1.apply(hdr, tmp1, tmp2); t1.apply(); hdr.ethernet.etherType = (bit<16>)(tmp1 - tmp2); } diff --git a/testdata/p4_16_errors_outputs/actions-with-duplicate-control-plane-names1.p4-stderr b/testdata/p4_16_errors_outputs/actions-with-duplicate-control-plane-names1.p4-stderr index e9fdab29466..e1ee3df39d6 100644 --- a/testdata/p4_16_errors_outputs/actions-with-duplicate-control-plane-names1.p4-stderr +++ b/testdata/p4_16_errors_outputs/actions-with-duplicate-control-plane-names1.p4-stderr @@ -4,21 +4,39 @@ actions-with-duplicate-control-plane-names1.p4(67): [--Werror=duplicate] error: actions-with-duplicate-control-plane-names1.p4(62) @name("foo3") action topa3() { } ^^^^^ -actions-with-duplicate-control-plane-names1.p4(83): [--Werror=duplicate] error: .foo1: conflicting control plane name: foo1 +actions-with-duplicate-control-plane-names1.p4(114): [--Werror=duplicate] error: .foo1: conflicting control plane name: foo1 @name(".foo1") action a1 (bit<8> x, bit<8> y) { tmp1 = x >> 1; tmp2 = y; } ^^ actions-with-duplicate-control-plane-names1.p4(58) action foo1() { } ^^^^ -actions-with-duplicate-control-plane-names1.p4(88): [--Werror=duplicate] error: .foo2: conflicting control plane name: foo2 +actions-with-duplicate-control-plane-names1.p4(119): [--Werror=duplicate] error: .foo2: conflicting control plane name: foo2 @name(".foo2") action a2 (bit<8> x, bit<8> y) { tmp1 = x >> 2; tmp2 = y; } ^^ actions-with-duplicate-control-plane-names1.p4(60) @name("foo2") action topa2() { } ^^^^^ -actions-with-duplicate-control-plane-names1.p4(93): [--Werror=duplicate] error: foo5: conflicting control plane name: .ingressImpl.foo5 - @name("foo5") action a5 (bit<8> x, bit<8> y) { tmp1 = x >> 4; tmp2 = y; } +actions-with-duplicate-control-plane-names1.p4(124): [--Werror=duplicate] error: foo5: conflicting control plane name: .ingressImpl.foo5 + @name("foo5") action a5 (bit<8> x, bit<8> y) { tmp1 = x >> 5; tmp2 = y; } ^^ actions-with-duplicate-control-plane-names1.p4(69) @name(".ingressImpl.foo5") action topa5() { } ^^^^^ +actions-with-duplicate-control-plane-names1.p4(79): [--Werror=duplicate] error: .foo1: conflicting control plane name: foo1 + @name(".foo1") action suba1 (bit<8> x, bit<8> y) { tmp1 = x; tmp2 = y >> 1; } + ^^^^^ +actions-with-duplicate-control-plane-names1.p4(58) +action foo1() { } + ^^^^ +actions-with-duplicate-control-plane-names1.p4(84): [--Werror=duplicate] error: .foo2: conflicting control plane name: foo2 + @name(".foo2") action suba2 (bit<8> x, bit<8> y) { tmp1 = x; tmp2 = y >> 2; } + ^^^^^ +actions-with-duplicate-control-plane-names1.p4(60) +@name("foo2") action topa2() { } + ^^^^^ +actions-with-duplicate-control-plane-names1.p4(86): [--Werror=duplicate] error: c1.bar1: conflicting control plane name: c1.bar1 + @name("bar1") action suba3 (bit<8> x, bit<8> y) { tmp1 = x; tmp2 = y >> 3; } + ^^^^^ +actions-with-duplicate-control-plane-names1.p4(129) + @name("c1.bar1") action a6 (bit<8> x, bit<8> y) { tmp1 = x >> 6; tmp2 = y; } + ^^ From a2e913b49455f2010e32eb0a5d80d21560aba581 Mon Sep 17 00:00:00 2001 From: Andy Fingerhut Date: Thu, 24 Oct 2024 20:56:52 -0400 Subject: [PATCH 34/48] Add another correct case of not-conflicting @name annotations involving sub-ctrl Signed-off-by: Andy Fingerhut --- .../actions-almost-duplicate-names1.p4 | 23 ++++++++ .../actions-almost-duplicate-names1-first.p4 | 23 ++++++++ ...ctions-almost-duplicate-names1-frontend.p4 | 38 ++++++++---- .../actions-almost-duplicate-names1-midend.p4 | 58 ++++++++++++------- .../actions-almost-duplicate-names1.p4 | 20 +++++++ ...ns-almost-duplicate-names1.p4.p4info.txtpb | 42 ++++++++++++++ 6 files changed, 174 insertions(+), 30 deletions(-) diff --git a/testdata/p4_16_samples/actions-almost-duplicate-names1.p4 b/testdata/p4_16_samples/actions-almost-duplicate-names1.p4 index 52a4ae6eced..dc28e6ad341 100644 --- a/testdata/p4_16_samples/actions-almost-duplicate-names1.p4 +++ b/testdata/p4_16_samples/actions-almost-duplicate-names1.p4 @@ -62,6 +62,28 @@ action foo2() { action foo3() { } +control c1( + inout headers_t hdr, + inout bit<8> tmp1, + inout bit<8> tmp2) +{ + // This is not a name conflict with a4 in control ingressImpl, + // because it has a hierarchical name "ingressImpl.bar", but suba1 + // will have hierarchical name "ingressImpl.c1.bar". + @name("bar") action suba1 (bit<8> x, bit<8> y) { tmp1 = x; tmp2 = y >> 3; } + + table t2 { + actions = { + suba1; + } + key = { hdr.ethernet.srcAddr: exact; } + size = 32; + } + apply { + t2.apply(); + } +} + control ingressImpl( inout headers_t hdr, inout metadata_t meta, @@ -110,6 +132,7 @@ control ingressImpl( apply { tmp1 = hdr.ethernet.srcAddr[7:0]; tmp2 = hdr.ethernet.dstAddr[7:0]; + c1.apply(hdr, tmp1, tmp2); t1.apply(); // This is here simply to ensure that the compiler cannot // optimize away the effects of t1 and t2, which can only diff --git a/testdata/p4_16_samples_outputs/actions-almost-duplicate-names1-first.p4 b/testdata/p4_16_samples_outputs/actions-almost-duplicate-names1-first.p4 index af35a852843..fffbfa68874 100644 --- a/testdata/p4_16_samples_outputs/actions-almost-duplicate-names1-first.p4 +++ b/testdata/p4_16_samples_outputs/actions-almost-duplicate-names1-first.p4 @@ -34,6 +34,27 @@ action foo1() { } @name(".baz") action foo3() { } +control c1(inout headers_t hdr, inout bit<8> tmp1, inout bit<8> tmp2) { + @name("bar") action suba1(bit<8> x, bit<8> y) { + tmp1 = x; + tmp2 = y >> 3; + } + table t2 { + actions = { + suba1(); + @defaultonly NoAction(); + } + key = { + hdr.ethernet.srcAddr: exact @name("hdr.ethernet.srcAddr"); + } + size = 32; + default_action = NoAction(); + } + apply { + t2.apply(); + } +} + control ingressImpl(inout headers_t hdr, inout metadata_t meta, inout standard_metadata_t stdmeta) { bit<8> tmp1; bit<8> tmp2; @@ -75,9 +96,11 @@ control ingressImpl(inout headers_t hdr, inout metadata_t meta, inout standard_m default_action = NoAction(); size = 512; } + @name("c1") c1() c1_inst; apply { tmp1 = hdr.ethernet.srcAddr[7:0]; tmp2 = hdr.ethernet.dstAddr[7:0]; + c1_inst.apply(hdr, tmp1, tmp2); t1.apply(); hdr.ethernet.etherType = (bit<16>)(tmp1 - tmp2); } diff --git a/testdata/p4_16_samples_outputs/actions-almost-duplicate-names1-frontend.p4 b/testdata/p4_16_samples_outputs/actions-almost-duplicate-names1-frontend.p4 index b39c443220a..2d368f983ba 100644 --- a/testdata/p4_16_samples_outputs/actions-almost-duplicate-names1-frontend.p4 +++ b/testdata/p4_16_samples_outputs/actions-almost-duplicate-names1-frontend.p4 @@ -33,6 +33,8 @@ control ingressImpl(inout headers_t hdr, inout metadata_t meta, inout standard_m @name("ingressImpl.tmp2") bit<8> tmp2_0; @noWarn("unused") @name(".NoAction") action NoAction_1() { } + @noWarn("unused") @name(".NoAction") action NoAction_2() { + } @name(".foo1") action foo1_0() { } @name(".foo2") action foo2_0() { @@ -43,22 +45,22 @@ control ingressImpl(inout headers_t hdr, inout metadata_t meta, inout standard_m tmp1_0 = x >> 1; tmp2_0 = y; } - @name("ingressImpl.foo2") action a2(@name("x") bit<8> x_5, @name("y") bit<8> y_5) { - tmp1_0 = x_5 >> 2; - tmp2_0 = y_5; - } - @name(".bar") action a3(@name("x") bit<8> x_6, @name("y") bit<8> y_6) { - tmp1_0 = x_6 >> 3; + @name("ingressImpl.foo2") action a2(@name("x") bit<8> x_6, @name("y") bit<8> y_6) { + tmp1_0 = x_6 >> 2; tmp2_0 = y_6; } - @name("ingressImpl.bar") action a4(@name("x") bit<8> x_7, @name("y") bit<8> y_7) { - tmp1_0 = x_7 >> 4; + @name(".bar") action a3(@name("x") bit<8> x_7, @name("y") bit<8> y_7) { + tmp1_0 = x_7 >> 3; tmp2_0 = y_7; } - @name("ingressImpl.baz") action a5(@name("x") bit<8> x_8, @name("y") bit<8> y_8) { - tmp1_0 = x_8 >> 5; + @name("ingressImpl.bar") action a4(@name("x") bit<8> x_8, @name("y") bit<8> y_8) { + tmp1_0 = x_8 >> 4; tmp2_0 = y_8; } + @name("ingressImpl.baz") action a5(@name("x") bit<8> x_9, @name("y") bit<8> y_9) { + tmp1_0 = x_9 >> 5; + tmp2_0 = y_9; + } @name("ingressImpl.t1") table t1_0 { actions = { NoAction_1(); @@ -77,9 +79,25 @@ control ingressImpl(inout headers_t hdr, inout metadata_t meta, inout standard_m default_action = NoAction_1(); size = 512; } + @name("ingressImpl.c1.bar") action c1_bar_0(@name("x") bit<8> x_10, @name("y") bit<8> y_10) { + tmp1_0 = x_10; + tmp2_0 = y_10 >> 3; + } + @name("ingressImpl.c1.t2") table c1_t2 { + actions = { + c1_bar_0(); + @defaultonly NoAction_2(); + } + key = { + hdr.ethernet.srcAddr: exact @name("hdr.ethernet.srcAddr"); + } + size = 32; + default_action = NoAction_2(); + } apply { tmp1_0 = hdr.ethernet.srcAddr[7:0]; tmp2_0 = hdr.ethernet.dstAddr[7:0]; + c1_t2.apply(); t1_0.apply(); hdr.ethernet.etherType = (bit<16>)(tmp1_0 - tmp2_0); } diff --git a/testdata/p4_16_samples_outputs/actions-almost-duplicate-names1-midend.p4 b/testdata/p4_16_samples_outputs/actions-almost-duplicate-names1-midend.p4 index d8e76d20851..766dfcb7c78 100644 --- a/testdata/p4_16_samples_outputs/actions-almost-duplicate-names1-midend.p4 +++ b/testdata/p4_16_samples_outputs/actions-almost-duplicate-names1-midend.p4 @@ -32,6 +32,8 @@ control ingressImpl(inout headers_t hdr, inout metadata_t meta, inout standard_m @name("ingressImpl.tmp2") bit<8> tmp2_0; @noWarn("unused") @name(".NoAction") action NoAction_1() { } + @noWarn("unused") @name(".NoAction") action NoAction_2() { + } @name(".foo1") action foo1_0() { } @name(".foo2") action foo2_0() { @@ -42,22 +44,22 @@ control ingressImpl(inout headers_t hdr, inout metadata_t meta, inout standard_m tmp1_0 = x >> 1; tmp2_0 = y; } - @name("ingressImpl.foo2") action a2(@name("x") bit<8> x_5, @name("y") bit<8> y_5) { - tmp1_0 = x_5 >> 2; - tmp2_0 = y_5; - } - @name(".bar") action a3(@name("x") bit<8> x_6, @name("y") bit<8> y_6) { - tmp1_0 = x_6 >> 3; + @name("ingressImpl.foo2") action a2(@name("x") bit<8> x_6, @name("y") bit<8> y_6) { + tmp1_0 = x_6 >> 2; tmp2_0 = y_6; } - @name("ingressImpl.bar") action a4(@name("x") bit<8> x_7, @name("y") bit<8> y_7) { - tmp1_0 = x_7 >> 4; + @name(".bar") action a3(@name("x") bit<8> x_7, @name("y") bit<8> y_7) { + tmp1_0 = x_7 >> 3; tmp2_0 = y_7; } - @name("ingressImpl.baz") action a5(@name("x") bit<8> x_8, @name("y") bit<8> y_8) { - tmp1_0 = x_8 >> 5; + @name("ingressImpl.bar") action a4(@name("x") bit<8> x_8, @name("y") bit<8> y_8) { + tmp1_0 = x_8 >> 4; tmp2_0 = y_8; } + @name("ingressImpl.baz") action a5(@name("x") bit<8> x_9, @name("y") bit<8> y_9) { + tmp1_0 = x_9 >> 5; + tmp2_0 = y_9; + } @name("ingressImpl.t1") table t1_0 { actions = { NoAction_1(); @@ -76,29 +78,45 @@ control ingressImpl(inout headers_t hdr, inout metadata_t meta, inout standard_m default_action = NoAction_1(); size = 512; } - @hidden action actionsalmostduplicatenames1l111() { + @name("ingressImpl.c1.bar") action c1_bar_0(@name("x") bit<8> x_10, @name("y") bit<8> y_10) { + tmp1_0 = x_10; + tmp2_0 = y_10 >> 3; + } + @name("ingressImpl.c1.t2") table c1_t2 { + actions = { + c1_bar_0(); + @defaultonly NoAction_2(); + } + key = { + hdr.ethernet.srcAddr: exact @name("hdr.ethernet.srcAddr"); + } + size = 32; + default_action = NoAction_2(); + } + @hidden action actionsalmostduplicatenames1l133() { tmp1_0 = hdr.ethernet.srcAddr[7:0]; tmp2_0 = hdr.ethernet.dstAddr[7:0]; } - @hidden action actionsalmostduplicatenames1l117() { + @hidden action actionsalmostduplicatenames1l140() { hdr.ethernet.etherType = (bit<16>)(tmp1_0 - tmp2_0); } - @hidden table tbl_actionsalmostduplicatenames1l111 { + @hidden table tbl_actionsalmostduplicatenames1l133 { actions = { - actionsalmostduplicatenames1l111(); + actionsalmostduplicatenames1l133(); } - const default_action = actionsalmostduplicatenames1l111(); + const default_action = actionsalmostduplicatenames1l133(); } - @hidden table tbl_actionsalmostduplicatenames1l117 { + @hidden table tbl_actionsalmostduplicatenames1l140 { actions = { - actionsalmostduplicatenames1l117(); + actionsalmostduplicatenames1l140(); } - const default_action = actionsalmostduplicatenames1l117(); + const default_action = actionsalmostduplicatenames1l140(); } apply { - tbl_actionsalmostduplicatenames1l111.apply(); + tbl_actionsalmostduplicatenames1l133.apply(); + c1_t2.apply(); t1_0.apply(); - tbl_actionsalmostduplicatenames1l117.apply(); + tbl_actionsalmostduplicatenames1l140.apply(); } } diff --git a/testdata/p4_16_samples_outputs/actions-almost-duplicate-names1.p4 b/testdata/p4_16_samples_outputs/actions-almost-duplicate-names1.p4 index 97b85ede8dc..28cb84a938f 100644 --- a/testdata/p4_16_samples_outputs/actions-almost-duplicate-names1.p4 +++ b/testdata/p4_16_samples_outputs/actions-almost-duplicate-names1.p4 @@ -34,6 +34,25 @@ action foo1() { } @name(".baz") action foo3() { } +control c1(inout headers_t hdr, inout bit<8> tmp1, inout bit<8> tmp2) { + @name("bar") action suba1(bit<8> x, bit<8> y) { + tmp1 = x; + tmp2 = y >> 3; + } + table t2 { + actions = { + suba1; + } + key = { + hdr.ethernet.srcAddr: exact; + } + size = 32; + } + apply { + t2.apply(); + } +} + control ingressImpl(inout headers_t hdr, inout metadata_t meta, inout standard_metadata_t stdmeta) { bit<8> tmp1; bit<8> tmp2; @@ -78,6 +97,7 @@ control ingressImpl(inout headers_t hdr, inout metadata_t meta, inout standard_m apply { tmp1 = hdr.ethernet.srcAddr[7:0]; tmp2 = hdr.ethernet.dstAddr[7:0]; + c1.apply(hdr, tmp1, tmp2); t1.apply(); hdr.ethernet.etherType = (bit<16>)(tmp1 - tmp2); } diff --git a/testdata/p4_16_samples_outputs/actions-almost-duplicate-names1.p4.p4info.txtpb b/testdata/p4_16_samples_outputs/actions-almost-duplicate-names1.p4.p4info.txtpb index 184fa761217..c6e0319c1e3 100644 --- a/testdata/p4_16_samples_outputs/actions-almost-duplicate-names1.p4.p4info.txtpb +++ b/testdata/p4_16_samples_outputs/actions-almost-duplicate-names1.p4.p4info.txtpb @@ -48,6 +48,31 @@ tables { } size: 512 } +tables { + preamble { + id: 43840731 + name: "ingressImpl.c1.t2" + alias: "t2" + } + match_fields { + id: 1 + name: "hdr.ethernet.srcAddr" + bitwidth: 48 + match_type: EXACT + } + action_refs { + id: 25482417 + } + action_refs { + id: 21257015 + annotations: "@defaultonly" + scope: DEFAULT_ONLY + } + initial_default_action { + action_id: 21257015 + } + size: 32 +} actions { preamble { id: 21257015 @@ -162,5 +187,22 @@ actions { bitwidth: 8 } } +actions { + preamble { + id: 25482417 + name: "ingressImpl.c1.bar" + alias: "c1.bar" + } + params { + id: 1 + name: "x" + bitwidth: 8 + } + params { + id: 2 + name: "y" + bitwidth: 8 + } +} type_info { } From 72827dd1a455a7304cb09bc67188930ea61121ac Mon Sep 17 00:00:00 2001 From: Andy Fingerhut Date: Wed, 30 Oct 2024 13:33:32 -0400 Subject: [PATCH 35/48] Fix comment, and remove unnecessary comment in P4 test program Signed-off-by: Andy Fingerhut --- control-plane/p4RuntimeSerializer.cpp | 2 +- ...ons-with-duplicate-control-plane-names1.p4 | 4 --- ...h-duplicate-control-plane-names1.p4-stderr | 28 +++++++++---------- 3 files changed, 15 insertions(+), 19 deletions(-) diff --git a/control-plane/p4RuntimeSerializer.cpp b/control-plane/p4RuntimeSerializer.cpp index c520a5c8e4b..6c74f928fd9 100644 --- a/control-plane/p4RuntimeSerializer.cpp +++ b/control-plane/p4RuntimeSerializer.cpp @@ -1584,7 +1584,7 @@ void P4RuntimeSerializer::serializeP4RuntimeIfRequired(const IR::P4Program *prog std::vector files; std::vector formats; - // only generate P4Info is required by user-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/testdata/p4_16_errors/actions-with-duplicate-control-plane-names1.p4 b/testdata/p4_16_errors/actions-with-duplicate-control-plane-names1.p4 index 451af01cb68..b0d1704d2c5 100644 --- a/testdata/p4_16_errors/actions-with-duplicate-control-plane-names1.p4 +++ b/testdata/p4_16_errors/actions-with-duplicate-control-plane-names1.p4 @@ -30,10 +30,6 @@ struct headers_t { } struct metadata_t { -/* - bit<4> a; - bit<4> b; -*/ } parser parserImpl( diff --git a/testdata/p4_16_errors_outputs/actions-with-duplicate-control-plane-names1.p4-stderr b/testdata/p4_16_errors_outputs/actions-with-duplicate-control-plane-names1.p4-stderr index e1ee3df39d6..e80f43d918f 100644 --- a/testdata/p4_16_errors_outputs/actions-with-duplicate-control-plane-names1.p4-stderr +++ b/testdata/p4_16_errors_outputs/actions-with-duplicate-control-plane-names1.p4-stderr @@ -1,42 +1,42 @@ -actions-with-duplicate-control-plane-names1.p4(67): [--Werror=duplicate] error: .foo3: conflicting control plane name: foo3 +actions-with-duplicate-control-plane-names1.p4(63): [--Werror=duplicate] error: .foo3: conflicting control plane name: foo3 @name(".foo3") action topa4() { } ^^^^^ -actions-with-duplicate-control-plane-names1.p4(62) +actions-with-duplicate-control-plane-names1.p4(58) @name("foo3") action topa3() { } ^^^^^ -actions-with-duplicate-control-plane-names1.p4(114): [--Werror=duplicate] error: .foo1: conflicting control plane name: foo1 +actions-with-duplicate-control-plane-names1.p4(110): [--Werror=duplicate] error: .foo1: conflicting control plane name: foo1 @name(".foo1") action a1 (bit<8> x, bit<8> y) { tmp1 = x >> 1; tmp2 = y; } ^^ -actions-with-duplicate-control-plane-names1.p4(58) +actions-with-duplicate-control-plane-names1.p4(54) action foo1() { } ^^^^ -actions-with-duplicate-control-plane-names1.p4(119): [--Werror=duplicate] error: .foo2: conflicting control plane name: foo2 +actions-with-duplicate-control-plane-names1.p4(115): [--Werror=duplicate] error: .foo2: conflicting control plane name: foo2 @name(".foo2") action a2 (bit<8> x, bit<8> y) { tmp1 = x >> 2; tmp2 = y; } ^^ -actions-with-duplicate-control-plane-names1.p4(60) +actions-with-duplicate-control-plane-names1.p4(56) @name("foo2") action topa2() { } ^^^^^ -actions-with-duplicate-control-plane-names1.p4(124): [--Werror=duplicate] error: foo5: conflicting control plane name: .ingressImpl.foo5 +actions-with-duplicate-control-plane-names1.p4(120): [--Werror=duplicate] error: foo5: conflicting control plane name: .ingressImpl.foo5 @name("foo5") action a5 (bit<8> x, bit<8> y) { tmp1 = x >> 5; tmp2 = y; } ^^ -actions-with-duplicate-control-plane-names1.p4(69) +actions-with-duplicate-control-plane-names1.p4(65) @name(".ingressImpl.foo5") action topa5() { } ^^^^^ -actions-with-duplicate-control-plane-names1.p4(79): [--Werror=duplicate] error: .foo1: conflicting control plane name: foo1 +actions-with-duplicate-control-plane-names1.p4(75): [--Werror=duplicate] error: .foo1: conflicting control plane name: foo1 @name(".foo1") action suba1 (bit<8> x, bit<8> y) { tmp1 = x; tmp2 = y >> 1; } ^^^^^ -actions-with-duplicate-control-plane-names1.p4(58) +actions-with-duplicate-control-plane-names1.p4(54) action foo1() { } ^^^^ -actions-with-duplicate-control-plane-names1.p4(84): [--Werror=duplicate] error: .foo2: conflicting control plane name: foo2 +actions-with-duplicate-control-plane-names1.p4(80): [--Werror=duplicate] error: .foo2: conflicting control plane name: foo2 @name(".foo2") action suba2 (bit<8> x, bit<8> y) { tmp1 = x; tmp2 = y >> 2; } ^^^^^ -actions-with-duplicate-control-plane-names1.p4(60) +actions-with-duplicate-control-plane-names1.p4(56) @name("foo2") action topa2() { } ^^^^^ -actions-with-duplicate-control-plane-names1.p4(86): [--Werror=duplicate] error: c1.bar1: conflicting control plane name: c1.bar1 +actions-with-duplicate-control-plane-names1.p4(82): [--Werror=duplicate] error: c1.bar1: conflicting control plane name: c1.bar1 @name("bar1") action suba3 (bit<8> x, bit<8> y) { tmp1 = x; tmp2 = y >> 3; } ^^^^^ -actions-with-duplicate-control-plane-names1.p4(129) +actions-with-duplicate-control-plane-names1.p4(125) @name("c1.bar1") action a6 (bit<8> x, bit<8> y) { tmp1 = x >> 6; tmp2 = y; } ^^ From bb50cc6475d4b3f5c44c5491cf62338ebcdbcd45 Mon Sep 17 00:00:00 2001 From: Andy Fingerhut Date: Tue, 5 Nov 2024 06:11:53 +0000 Subject: [PATCH 36/48] Put method controlPlaneAPIGenEnabled in class CompilerOptions Signed-off-by: Andy Fingerhut --- frontends/common/options.h | 12 ++++++++++++ frontends/p4/frontend.h | 12 ------------ 2 files changed, 12 insertions(+), 12 deletions(-) diff --git a/frontends/common/options.h b/frontends/common/options.h index a383c7ca272..4a7284fa5c8 100644 --- a/frontends/common/options.h +++ b/frontends/common/options.h @@ -87,6 +87,18 @@ 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 CompilerOptions &options) const { + if (options.p4RuntimeFile.isNullOrEmpty() && options.p4RuntimeFiles.isNullOrEmpty() && + options.p4RuntimeEntriesFile.isNullOrEmpty() && + options.p4RuntimeEntriesFiles.isNullOrEmpty()) { + return false; + } + return true; + } }; } // namespace P4 diff --git a/frontends/p4/frontend.h b/frontends/p4/frontend.h index 0695ef38298..14ad2f345a1 100644 --- a/frontends/p4/frontend.h +++ b/frontends/p4/frontend.h @@ -60,18 +60,6 @@ class FrontEndPolicy : public RemoveUnusedPolicy { return options.optimizationLevel > 0; } - /// 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 CompilerOptions &options) const { - if (options.p4RuntimeFile.isNullOrEmpty() && options.p4RuntimeFiles.isNullOrEmpty() && - options.p4RuntimeEntriesFile.isNullOrEmpty() && - options.p4RuntimeEntriesFiles.isNullOrEmpty()) { - return false; - } - return true; - } - /// Get policy for the constant folding pass. @see ConstantFoldingPolicy /// @returns Defaults to nullptr, which causes constant folding to use the default policy, which /// does not modify the pass defaults in any way. From a8401636f9c4fdcff4267c028bf093b29ce5aa39 Mon Sep 17 00:00:00 2001 From: Andy Fingerhut Date: Tue, 5 Nov 2024 06:19:23 +0000 Subject: [PATCH 37/48] Fixes for incorrect previous commit. Signed-off-by: Andy Fingerhut --- frontends/common/options.h | 8 ++++---- frontends/p4/frontend.cpp | 2 +- 2 files changed, 5 insertions(+), 5 deletions(-) diff --git a/frontends/common/options.h b/frontends/common/options.h index 4a7284fa5c8..8a157ddb7c3 100644 --- a/frontends/common/options.h +++ b/frontends/common/options.h @@ -91,10 +91,10 @@ class CompilerOptions : public ParserOptions { /// 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 CompilerOptions &options) const { - if (options.p4RuntimeFile.isNullOrEmpty() && options.p4RuntimeFiles.isNullOrEmpty() && - options.p4RuntimeEntriesFile.isNullOrEmpty() && - options.p4RuntimeEntriesFiles.isNullOrEmpty()) { + virtual bool controlPlaneAPIGenEnabled() const { + if (p4RuntimeFile.isNullOrEmpty() && p4RuntimeFiles.isNullOrEmpty() && + p4RuntimeEntriesFile.isNullOrEmpty() && + p4RuntimeEntriesFiles.isNullOrEmpty()) { return false; } return true; diff --git a/frontends/p4/frontend.cpp b/frontends/p4/frontend.cpp index 0fd1da169c6..b2bd068ccfa 100644 --- a/frontends/p4/frontend.cpp +++ b/frontends/p4/frontend.cpp @@ -237,7 +237,7 @@ const IR::P4Program *FrontEnd::run(const CompilerOptions &options, const IR::P4P new Inline(&typeMap, *policy, options.optimizeParserInlining), }); } - if (policy->controlPlaneAPIGenEnabled(options)) { + if (options.controlPlaneAPIGenEnabled()) { passes.addPasses({ new DuplicateActionControlPlaneNameCheck(), }); From 19956fc6dae940ad1068abc164848706bb548d98 Mon Sep 17 00:00:00 2001 From: Andy Fingerhut Date: Tue, 5 Nov 2024 06:25:17 +0000 Subject: [PATCH 38/48] Fix cpplint warning from clang-format Signed-off-by: Andy Fingerhut --- frontends/common/options.h | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/frontends/common/options.h b/frontends/common/options.h index 8a157ddb7c3..422eeb1c6cb 100644 --- a/frontends/common/options.h +++ b/frontends/common/options.h @@ -93,8 +93,7 @@ class CompilerOptions : public ParserOptions { /// given explicitly enabling control plane API generation. virtual bool controlPlaneAPIGenEnabled() const { if (p4RuntimeFile.isNullOrEmpty() && p4RuntimeFiles.isNullOrEmpty() && - p4RuntimeEntriesFile.isNullOrEmpty() && - p4RuntimeEntriesFiles.isNullOrEmpty()) { + p4RuntimeEntriesFile.isNullOrEmpty() && p4RuntimeEntriesFiles.isNullOrEmpty()) { return false; } return true; From 62ee2c13093f0dab2f9540549ec93525a051a58d Mon Sep 17 00:00:00 2001 From: Andy Fingerhut Date: Sat, 9 Nov 2024 22:11:50 +0000 Subject: [PATCH 39/48] Updates due to recent changes in annotation APIs Signed-off-by: Andy Fingerhut --- frontends/p4/duplicateActionControlPlaneNameCheck.cpp | 11 ++--------- frontends/p4/localizeActions.cpp | 7 ++----- 2 files changed, 4 insertions(+), 14 deletions(-) diff --git a/frontends/p4/duplicateActionControlPlaneNameCheck.cpp b/frontends/p4/duplicateActionControlPlaneNameCheck.cpp index a2856eda43b..888ccd547b7 100644 --- a/frontends/p4/duplicateActionControlPlaneNameCheck.cpp +++ b/frontends/p4/duplicateActionControlPlaneNameCheck.cpp @@ -46,15 +46,8 @@ const IR::Node *DuplicateActionControlPlaneNameCheck::postorder(IR::P4Action *ac // by the time this pass executes, so this method will handle // such actions. if (findContext() == nullptr) { - auto annos = action->annotations; - bool hasNameAnnotation = false; - if (annos != nullptr) { - auto nameAnno = annos->getSingle(IR::Annotation::nameAnnotation); - if (nameAnno) { - hasNameAnnotation = true; - } - } - if (!hasNameAnnotation) { + auto nameAnno = action->getAnnotation(IR::Annotation::nameAnnotation); + if (!nameAnno) { // name is what this top-level action's @name annotation // will be, after LocalizeAllActions pass adds one. cstring name = "." + action->name; diff --git a/frontends/p4/localizeActions.cpp b/frontends/p4/localizeActions.cpp index 7683bdf1a4d..a95641d0cbe 100644 --- a/frontends/p4/localizeActions.cpp +++ b/frontends/p4/localizeActions.cpp @@ -42,9 +42,7 @@ class ParamCloner : public CloneExpressions { const IR::Node *TagGlobalActions::preorder(IR::P4Action *action) { if (findContext() == nullptr) { - auto annos = action->annotations; - if (annos == nullptr) annos = IR::Annotations::empty; - auto nameAnno = annos->getSingle(IR::Annotation::nameAnnotation); + auto nameAnno = action->getAnnotation(IR::Annotation::nameAnnotation); if (nameAnno) { // If the value of the existing name annotation does not // begin with ".", prepend "." so that the name remains @@ -54,9 +52,8 @@ const IR::Node *TagGlobalActions::preorder(IR::P4Action *action) { if (!nameString.startsWith(".")) { nameString = "."_cs + nameString; auto newLit = new IR::StringLiteral(e0->srcInfo, nameString); - annos = annos->addOrReplace(IR::Annotation::nameAnnotation, newLit); + action->addOrReplaceAnnotation(IR::Annotation::nameAnnotation, newLit); } - action->annotations = annos; } else { cstring name = absl::StrCat(".", action->name); action->addAnnotationIfNew(IR::Annotation::nameAnnotation, new IR::StringLiteral(name), From c3feb19495c661a060f9daaf88ca7aa38dced88b Mon Sep 17 00:00:00 2001 From: Andy Fingerhut Date: Sat, 9 Nov 2024 23:20:18 +0000 Subject: [PATCH 40/48] More modifications required to make the code work with ... ... the recent annotations infra changes Signed-off-by: Andy Fingerhut --- .../duplicateActionControlPlaneNameCheck.cpp | 75 ++++++++----------- .../p4/duplicateActionControlPlaneNameCheck.h | 9 +-- 2 files changed, 32 insertions(+), 52 deletions(-) diff --git a/frontends/p4/duplicateActionControlPlaneNameCheck.cpp b/frontends/p4/duplicateActionControlPlaneNameCheck.cpp index 888ccd547b7..403cbf0742f 100644 --- a/frontends/p4/duplicateActionControlPlaneNameCheck.cpp +++ b/frontends/p4/duplicateActionControlPlaneNameCheck.cpp @@ -40,53 +40,40 @@ void DuplicateActionControlPlaneNameCheck::checkForDuplicateName(cstring name, } 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() == nullptr) { - auto nameAnno = action->getAnnotation(IR::Annotation::nameAnnotation); - if (!nameAnno) { - // 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; -} + bool topLevel = findContext() == nullptr; + auto nameAnno = action->getAnnotation(IR::Annotation::nameAnnotation); + if (!nameAnno && topLevel) { + // 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. -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()) { - 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()); + // name is what this top-level action's @name annotation + // will be, after LocalizeAllActions pass adds one. + cstring name = "." + action->name; + checkForDuplicateName(name, action); + } else { + const auto *e0 = nameAnno->expr.at(0); + cstring name = e0->to()->value; + 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, action); } - checkForDuplicateName(name, annotatedNode); - return annotation; + return action; } } // namespace P4 diff --git a/frontends/p4/duplicateActionControlPlaneNameCheck.h b/frontends/p4/duplicateActionControlPlaneNameCheck.h index 18156cc1ace..964e4f0101a 100644 --- a/frontends/p4/duplicateActionControlPlaneNameCheck.h +++ b/frontends/p4/duplicateActionControlPlaneNameCheck.h @@ -24,7 +24,7 @@ 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 + * error if it finds two actions 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 @@ -77,13 +77,6 @@ class DuplicateActionControlPlaneNameCheck : public Transform { 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 From 31f0b10a3122ebfcfec70ba7da2c29f7315df2b8 Mon Sep 17 00:00:00 2001 From: Andy Fingerhut Date: Sun, 10 Nov 2024 11:11:57 +0000 Subject: [PATCH 41/48] Simplify string manipulation code using latest abseil changes Signed-off-by: Andy Fingerhut --- .../p4/duplicateActionControlPlaneNameCheck.cpp | 12 ++++-------- 1 file changed, 4 insertions(+), 8 deletions(-) diff --git a/frontends/p4/duplicateActionControlPlaneNameCheck.cpp b/frontends/p4/duplicateActionControlPlaneNameCheck.cpp index 403cbf0742f..6ee18601609 100644 --- a/frontends/p4/duplicateActionControlPlaneNameCheck.cpp +++ b/frontends/p4/duplicateActionControlPlaneNameCheck.cpp @@ -51,7 +51,7 @@ const IR::Node *DuplicateActionControlPlaneNameCheck::postorder(IR::P4Action *ac // name is what this top-level action's @name annotation // will be, after LocalizeAllActions pass adds one. - cstring name = "." + action->name; + cstring name = absl::StrCat(".", action->name); checkForDuplicateName(name, action); } else { const auto *e0 = nameAnno->expr.at(0); @@ -61,14 +61,10 @@ const IR::Node *DuplicateActionControlPlaneNameCheck::postorder(IR::P4Action *ac // can compare it against any other @name annotation value // that begins with "." and is equal. if (stack.empty()) { - name = absl::StrCat(".", name.string_view()); + name = absl::StrCat(".", name); } else { - name = absl::StrCat(".", - absl::StrJoin(stack, ".", - [](std::string *out, cstring s) { - absl::StrAppend(out, s.string_view()); - }), - ".", name.string_view()); + name = absl::StrCat(".", absl::StrJoin(stack, "."), + ".", name); } } checkForDuplicateName(name, action); From bd31e6b0c9cf86a766ce52ebc7b9042032bea28c Mon Sep 17 00:00:00 2001 From: Andy Fingerhut Date: Sun, 10 Nov 2024 11:20:59 +0000 Subject: [PATCH 42/48] Fix clang-format warning Signed-off-by: Andy Fingerhut --- frontends/p4/duplicateActionControlPlaneNameCheck.cpp | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/frontends/p4/duplicateActionControlPlaneNameCheck.cpp b/frontends/p4/duplicateActionControlPlaneNameCheck.cpp index 6ee18601609..8f46488f5e8 100644 --- a/frontends/p4/duplicateActionControlPlaneNameCheck.cpp +++ b/frontends/p4/duplicateActionControlPlaneNameCheck.cpp @@ -63,8 +63,7 @@ const IR::Node *DuplicateActionControlPlaneNameCheck::postorder(IR::P4Action *ac if (stack.empty()) { name = absl::StrCat(".", name); } else { - name = absl::StrCat(".", absl::StrJoin(stack, "."), - ".", name); + name = absl::StrCat(".", absl::StrJoin(stack, "."), ".", name); } } checkForDuplicateName(name, action); From c111ab14292655691b89947f55f458ff58aa1fb8 Mon Sep 17 00:00:00 2001 From: Andy Fingerhut Date: Mon, 2 Dec 2024 15:51:36 +0000 Subject: [PATCH 43/48] Updates required to work with recent changes to annotation implementation Signed-off-by: Andy Fingerhut --- frontends/p4/duplicateActionControlPlaneNameCheck.cpp | 2 +- frontends/p4/localizeActions.cpp | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/frontends/p4/duplicateActionControlPlaneNameCheck.cpp b/frontends/p4/duplicateActionControlPlaneNameCheck.cpp index 8f46488f5e8..756308047f0 100644 --- a/frontends/p4/duplicateActionControlPlaneNameCheck.cpp +++ b/frontends/p4/duplicateActionControlPlaneNameCheck.cpp @@ -54,7 +54,7 @@ const IR::Node *DuplicateActionControlPlaneNameCheck::postorder(IR::P4Action *ac cstring name = absl::StrCat(".", action->name); checkForDuplicateName(name, action); } else { - const auto *e0 = nameAnno->expr.at(0); + const auto *e0 = nameAnno->getExpr(0); cstring name = e0->to()->value; if (!name.startsWith(".")) { // Create a fully hierarchical name beginning with ".", so we diff --git a/frontends/p4/localizeActions.cpp b/frontends/p4/localizeActions.cpp index a95641d0cbe..31aa2ba13dd 100644 --- a/frontends/p4/localizeActions.cpp +++ b/frontends/p4/localizeActions.cpp @@ -47,7 +47,7 @@ const IR::Node *TagGlobalActions::preorder(IR::P4Action *action) { // 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->getExpr(0); auto nameString = e0->to()->value; if (!nameString.startsWith(".")) { nameString = "."_cs + nameString; From b542540ce87587cec3d27ada7172c4d3f5fc0153 Mon Sep 17 00:00:00 2001 From: Andy Fingerhut Date: Mon, 2 Dec 2024 16:23:46 +0000 Subject: [PATCH 44/48] Address review comments Signed-off-by: Andy Fingerhut --- .../p4/duplicateActionControlPlaneNameCheck.cpp | 11 ++++------- .../p4/duplicateActionControlPlaneNameCheck.h | 14 +++++++------- frontends/p4/localizeActions.cpp | 6 ++---- 3 files changed, 13 insertions(+), 18 deletions(-) diff --git a/frontends/p4/duplicateActionControlPlaneNameCheck.cpp b/frontends/p4/duplicateActionControlPlaneNameCheck.cpp index 756308047f0..3f3d11afedd 100644 --- a/frontends/p4/duplicateActionControlPlaneNameCheck.cpp +++ b/frontends/p4/duplicateActionControlPlaneNameCheck.cpp @@ -26,21 +26,18 @@ cstring DuplicateActionControlPlaneNameCheck::getName(const IR::IDeclaration *de 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); if (!inserted) { - foundDuplicate = true; - otherNode = (*it).second; - } - if (foundDuplicate) { + otherNode = it->second; error(ErrorType::ERR_DUPLICATE, "%1%: conflicting control plane name: %2%", node, otherNode); } } const IR::Node *DuplicateActionControlPlaneNameCheck::postorder(IR::P4Action *action) { - bool topLevel = findContext() == nullptr; + bool topLevel = stack.empty(); auto nameAnno = action->getAnnotation(IR::Annotation::nameAnnotation); if (!nameAnno && topLevel) { // Actions declared at the top level that the developer did not diff --git a/frontends/p4/duplicateActionControlPlaneNameCheck.h b/frontends/p4/duplicateActionControlPlaneNameCheck.h index 964e4f0101a..b36401aa1d7 100644 --- a/frontends/p4/duplicateActionControlPlaneNameCheck.h +++ b/frontends/p4/duplicateActionControlPlaneNameCheck.h @@ -17,6 +17,7 @@ limitations under the License. #ifndef FRONTENDS_P4_DUPLICATEACTIONCONTROLPLANENAMECHECK_H_ #define FRONTENDS_P4_DUPLICATEACTIONCONTROLPLANENAMECHECK_H_ +#include #include "ir/ir.h" #include "ir/visitor.h" @@ -41,7 +42,7 @@ namespace P4 { class DuplicateActionControlPlaneNameCheck : public Transform { std::vector stack; /// Used for detection of conflicting control plane names among actions. - string_map actions; + absl::flat_hash_map actions; public: cstring getName(const IR::IDeclaration *decl); @@ -51,11 +52,9 @@ class DuplicateActionControlPlaneNameCheck : public Transform { 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(); + // There cannot be any action definitions inside a parser, so + // do not traverse through its nodes. + prune(); return parser; } @@ -69,7 +68,8 @@ class DuplicateActionControlPlaneNameCheck : public Transform { } const IR::Node *preorder(IR::P4Table *table) override { - visit(table->annotations); + // There cannot be any action definitions inside a table, so + // do not traverse through its nodes. prune(); return table; } diff --git a/frontends/p4/localizeActions.cpp b/frontends/p4/localizeActions.cpp index 31aa2ba13dd..c68f344cf23 100644 --- a/frontends/p4/localizeActions.cpp +++ b/frontends/p4/localizeActions.cpp @@ -42,8 +42,7 @@ class ParamCloner : public CloneExpressions { const IR::Node *TagGlobalActions::preorder(IR::P4Action *action) { if (findContext() == nullptr) { - auto nameAnno = action->getAnnotation(IR::Annotation::nameAnnotation); - if (nameAnno) { + if (const auto *nameAnno = action->getAnnotation(IR::Annotation::nameAnnotation)) { // 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. @@ -56,8 +55,7 @@ const IR::Node *TagGlobalActions::preorder(IR::P4Action *action) { } } else { cstring name = absl::StrCat(".", action->name); - action->addAnnotationIfNew(IR::Annotation::nameAnnotation, new IR::StringLiteral(name), - false); + action->addAnnotationIfNew(IR::Annotation::nameAnnotation, new IR::StringLiteral(name)); } } prune(); From beb87b0487d1fbf5b183d48418fb15afc7915d96 Mon Sep 17 00:00:00 2001 From: Andy Fingerhut Date: Mon, 2 Dec 2024 16:31:49 +0000 Subject: [PATCH 45/48] Removed an old line of commented-out code. Signed-off-by: Andy Fingerhut --- frontends/p4/duplicateActionControlPlaneNameCheck.cpp | 1 - 1 file changed, 1 deletion(-) diff --git a/frontends/p4/duplicateActionControlPlaneNameCheck.cpp b/frontends/p4/duplicateActionControlPlaneNameCheck.cpp index 3f3d11afedd..f43176cf830 100644 --- a/frontends/p4/duplicateActionControlPlaneNameCheck.cpp +++ b/frontends/p4/duplicateActionControlPlaneNameCheck.cpp @@ -27,7 +27,6 @@ cstring DuplicateActionControlPlaneNameCheck::getName(const IR::IDeclaration *de void DuplicateActionControlPlaneNameCheck::checkForDuplicateName(cstring name, const IR::Node *node) { auto *otherNode = node; - //auto [it, inserted] = actions.insert(std::pair(name, node)); auto [it, inserted] = actions.emplace(name, node); if (!inserted) { otherNode = it->second; From 87feba69f2d5b4a70e069f6a5b84905617f13e2d Mon Sep 17 00:00:00 2001 From: Andy Fingerhut Date: Mon, 2 Dec 2024 16:36:21 +0000 Subject: [PATCH 46/48] Reword some comments for clarity. Signed-off-by: Andy Fingerhut --- frontends/p4/duplicateActionControlPlaneNameCheck.cpp | 11 ++++++----- 1 file changed, 6 insertions(+), 5 deletions(-) diff --git a/frontends/p4/duplicateActionControlPlaneNameCheck.cpp b/frontends/p4/duplicateActionControlPlaneNameCheck.cpp index f43176cf830..536cb33f8e7 100644 --- a/frontends/p4/duplicateActionControlPlaneNameCheck.cpp +++ b/frontends/p4/duplicateActionControlPlaneNameCheck.cpp @@ -42,9 +42,9 @@ const IR::Node *DuplicateActionControlPlaneNameCheck::postorder(IR::P4Action *ac // 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 + // by the time this pass executes, so this case will handle // such actions. - + // // name is what this top-level action's @name annotation // will be, after LocalizeAllActions pass adds one. cstring name = absl::StrCat(".", action->name); @@ -53,9 +53,10 @@ const IR::Node *DuplicateActionControlPlaneNameCheck::postorder(IR::P4Action *ac const auto *e0 = nameAnno->getExpr(0); cstring name = e0->to()->value; 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. + // Create an absolute hierarchical name (one beginning + // with a "."), so we can compare it against any other + // @name annotation value that begins with "." and is + // equal. if (stack.empty()) { name = absl::StrCat(".", name); } else { From e586849fa544c709490b352281655068d8baac1f Mon Sep 17 00:00:00 2001 From: Andy Fingerhut Date: Mon, 2 Dec 2024 17:51:00 +0000 Subject: [PATCH 47/48] Fix lint warning Signed-off-by: Andy Fingerhut --- frontends/p4/duplicateActionControlPlaneNameCheck.h | 1 + 1 file changed, 1 insertion(+) diff --git a/frontends/p4/duplicateActionControlPlaneNameCheck.h b/frontends/p4/duplicateActionControlPlaneNameCheck.h index b36401aa1d7..e21f9480fb7 100644 --- a/frontends/p4/duplicateActionControlPlaneNameCheck.h +++ b/frontends/p4/duplicateActionControlPlaneNameCheck.h @@ -18,6 +18,7 @@ limitations under the License. #define FRONTENDS_P4_DUPLICATEACTIONCONTROLPLANENAMECHECK_H_ #include + #include "ir/ir.h" #include "ir/visitor.h" From 381ec486db521b8d7313abaf3a67d228720a2a8e Mon Sep 17 00:00:00 2001 From: Andy Fingerhut Date: Tue, 3 Dec 2024 03:06:25 +0000 Subject: [PATCH 48/48] Address review comment Signed-off-by: Andy Fingerhut --- frontends/p4/duplicateActionControlPlaneNameCheck.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/frontends/p4/duplicateActionControlPlaneNameCheck.cpp b/frontends/p4/duplicateActionControlPlaneNameCheck.cpp index 536cb33f8e7..2359c5681ea 100644 --- a/frontends/p4/duplicateActionControlPlaneNameCheck.cpp +++ b/frontends/p4/duplicateActionControlPlaneNameCheck.cpp @@ -57,7 +57,7 @@ const IR::Node *DuplicateActionControlPlaneNameCheck::postorder(IR::P4Action *ac // with a "."), so we can compare it against any other // @name annotation value that begins with "." and is // equal. - if (stack.empty()) { + if (topLevel) { name = absl::StrCat(".", name); } else { name = absl::StrCat(".", absl::StrJoin(stack, "."), ".", name);