From 6fe40113e35420e316009b8e6b612acbb8ba9876 Mon Sep 17 00:00:00 2001 From: Mihai Budiu Date: Fri, 15 Mar 2024 11:29:46 -0700 Subject: [PATCH 1/3] Make the tree cloner also clone Member(TypeNameExpression) to ensure the result is a DAG Signed-off-by: Mihai Budiu --- frontends/p4/cloner.h | 8 ++++++ testdata/p4_16_samples/issue4500.p4 | 23 +++++++++++++++++ .../p4_16_samples_outputs/issue4500-first.p4 | 21 ++++++++++++++++ .../issue4500-frontend.p4 | 18 +++++++++++++ .../p4_16_samples_outputs/issue4500-midend.p4 | 25 +++++++++++++++++++ testdata/p4_16_samples_outputs/issue4500.p4 | 21 ++++++++++++++++ .../p4_16_samples_outputs/issue4500.p4-stderr | 0 7 files changed, 116 insertions(+) create mode 100644 testdata/p4_16_samples/issue4500.p4 create mode 100644 testdata/p4_16_samples_outputs/issue4500-first.p4 create mode 100644 testdata/p4_16_samples_outputs/issue4500-frontend.p4 create mode 100644 testdata/p4_16_samples_outputs/issue4500-midend.p4 create mode 100644 testdata/p4_16_samples_outputs/issue4500.p4 create mode 100644 testdata/p4_16_samples_outputs/issue4500.p4-stderr diff --git a/frontends/p4/cloner.h b/frontends/p4/cloner.h index c32ed097cb..be8e1d1eed 100644 --- a/frontends/p4/cloner.h +++ b/frontends/p4/cloner.h @@ -34,6 +34,14 @@ class ClonePathExpressions : public Transform { return path; } + // Clone expressions of the form Member(TypeNameExpression) + const IR::Node *postorder(IR::Member* member) override { + if (member->expr->is()) { + return new IR::Member(member->expr->clone(), member->member); + } + return member; + } + template const T *clone(const IR::Node *node) { return node->apply(*this)->to(); diff --git a/testdata/p4_16_samples/issue4500.p4 b/testdata/p4_16_samples/issue4500.p4 new file mode 100644 index 0000000000..7865dc7f42 --- /dev/null +++ b/testdata/p4_16_samples/issue4500.p4 @@ -0,0 +1,23 @@ +enum E { + e0 +} + +extern void bar(E x); + +extern void baz(); + +void foo(E y) { + bar(y); + if (y == E.e0) baz(); +} + +control c() { + apply { + foo(E.e0); + } +} + +control proto(); +package top(proto p); + +top(c()) main; \ No newline at end of file diff --git a/testdata/p4_16_samples_outputs/issue4500-first.p4 b/testdata/p4_16_samples_outputs/issue4500-first.p4 new file mode 100644 index 0000000000..6786e84861 --- /dev/null +++ b/testdata/p4_16_samples_outputs/issue4500-first.p4 @@ -0,0 +1,21 @@ +enum E { + e0 +} + +extern void bar(E x); +extern void baz(); +void foo(E y) { + bar(y); + if (y == E.e0) { + baz(); + } +} +control c() { + apply { + foo(E.e0); + } +} + +control proto(); +package top(proto p); +top(c()) main; diff --git a/testdata/p4_16_samples_outputs/issue4500-frontend.p4 b/testdata/p4_16_samples_outputs/issue4500-frontend.p4 new file mode 100644 index 0000000000..1a02e7c129 --- /dev/null +++ b/testdata/p4_16_samples_outputs/issue4500-frontend.p4 @@ -0,0 +1,18 @@ +enum E { + e0 +} + +extern void bar(E x); +extern void baz(); +control c() { + apply { + bar(E.e0); + if (E.e0 == E.e0) { + baz(); + } + } +} + +control proto(); +package top(proto p); +top(c()) main; diff --git a/testdata/p4_16_samples_outputs/issue4500-midend.p4 b/testdata/p4_16_samples_outputs/issue4500-midend.p4 new file mode 100644 index 0000000000..6bf56910c3 --- /dev/null +++ b/testdata/p4_16_samples_outputs/issue4500-midend.p4 @@ -0,0 +1,25 @@ +enum E { + e0 +} + +extern void bar(E x); +extern void baz(); +control c() { + @hidden action issue4500l10() { + bar(E.e0); + baz(); + } + @hidden table tbl_issue4500l10 { + actions = { + issue4500l10(); + } + const default_action = issue4500l10(); + } + apply { + tbl_issue4500l10.apply(); + } +} + +control proto(); +package top(proto p); +top(c()) main; diff --git a/testdata/p4_16_samples_outputs/issue4500.p4 b/testdata/p4_16_samples_outputs/issue4500.p4 new file mode 100644 index 0000000000..6786e84861 --- /dev/null +++ b/testdata/p4_16_samples_outputs/issue4500.p4 @@ -0,0 +1,21 @@ +enum E { + e0 +} + +extern void bar(E x); +extern void baz(); +void foo(E y) { + bar(y); + if (y == E.e0) { + baz(); + } +} +control c() { + apply { + foo(E.e0); + } +} + +control proto(); +package top(proto p); +top(c()) main; diff --git a/testdata/p4_16_samples_outputs/issue4500.p4-stderr b/testdata/p4_16_samples_outputs/issue4500.p4-stderr new file mode 100644 index 0000000000..e69de29bb2 From 43072e1f7a304eb2c9add1762049228ce9dde532 Mon Sep 17 00:00:00 2001 From: Mihai Budiu Date: Fri, 15 Mar 2024 11:35:49 -0700 Subject: [PATCH 2/3] clang-format Signed-off-by: Mihai Budiu --- frontends/p4/cloner.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/frontends/p4/cloner.h b/frontends/p4/cloner.h index be8e1d1eed..78ebb43abf 100644 --- a/frontends/p4/cloner.h +++ b/frontends/p4/cloner.h @@ -35,7 +35,7 @@ class ClonePathExpressions : public Transform { } // Clone expressions of the form Member(TypeNameExpression) - const IR::Node *postorder(IR::Member* member) override { + const IR::Node *postorder(IR::Member *member) override { if (member->expr->is()) { return new IR::Member(member->expr->clone(), member->member); } From d1b406a12605a6ecef78b74a144fb0db9b38943b Mon Sep 17 00:00:00 2001 From: kfcripps Date: Tue, 19 Mar 2024 12:04:43 -0700 Subject: [PATCH 3/3] Rename ClonePathExpressions -> CloneExpressions --- backends/bmv2/psa_switch/psaSwitch.cpp | 2 +- backends/bmv2/simple_switch/simpleSwitch.cpp | 2 +- frontends/p4/cloner.h | 8 ++++---- frontends/p4/localizeActions.cpp | 2 +- frontends/p4/sideEffects.cpp | 2 +- frontends/p4/simplifyDefUse.h | 4 ++-- midend/predication.cpp | 4 ++-- 7 files changed, 12 insertions(+), 12 deletions(-) diff --git a/backends/bmv2/psa_switch/psaSwitch.cpp b/backends/bmv2/psa_switch/psaSwitch.cpp index 785ce6144a..33833c6660 100644 --- a/backends/bmv2/psa_switch/psaSwitch.cpp +++ b/backends/bmv2/psa_switch/psaSwitch.cpp @@ -284,7 +284,7 @@ void PsaSwitchBackend::convert(const IR::ToplevelBlock *tlb) { new P4::RemoveAllUnusedDeclarations(refMap), // Converts the DAG into a TREE (at least for expressions) // This is important later for conversion to JSON. - new P4::ClonePathExpressions(), + new P4::CloneExpressions(), new P4::ClearTypeMap(typeMap), evaluator, [this, evaluator, structure]() { toplevel = evaluator->getToplevelBlock(); }, diff --git a/backends/bmv2/simple_switch/simpleSwitch.cpp b/backends/bmv2/simple_switch/simpleSwitch.cpp index 5cbb1202aa..3fc2751bd6 100644 --- a/backends/bmv2/simple_switch/simpleSwitch.cpp +++ b/backends/bmv2/simple_switch/simpleSwitch.cpp @@ -1186,7 +1186,7 @@ void SimpleSwitchBackend::convert(const IR::ToplevelBlock *tlb) { new P4::FlattenLogMsg(refMap, typeMap), // Converts the DAG into a TREE (at least for expressions) // This is important later for conversion to JSON. - new P4::ClonePathExpressions(), + new P4::CloneExpressions(), new P4::ClearTypeMap(typeMap), new P4::TypeChecking(refMap, typeMap, true), evaluator, diff --git a/frontends/p4/cloner.h b/frontends/p4/cloner.h index 78ebb43abf..bfac6274e9 100644 --- a/frontends/p4/cloner.h +++ b/frontends/p4/cloner.h @@ -21,13 +21,13 @@ limitations under the License. namespace P4 { -/// This transform converts identical PathExpression nodes in a DAG +/// This transform converts identical PathExpression or Member nodes in a DAG /// into distinct nodes. -class ClonePathExpressions : public Transform { +class CloneExpressions : public Transform { public: - ClonePathExpressions() { + CloneExpressions() { visitDagOnce = false; - setName("ClonePathExpressions"); + setName("CloneExpressions"); } const IR::Node *postorder(IR::PathExpression *path) override { path->path = path->path->clone(); diff --git a/frontends/p4/localizeActions.cpp b/frontends/p4/localizeActions.cpp index f285c1e49c..a95bc8be4a 100644 --- a/frontends/p4/localizeActions.cpp +++ b/frontends/p4/localizeActions.cpp @@ -22,7 +22,7 @@ namespace P4 { namespace { -class ParamCloner : public ClonePathExpressions { +class ParamCloner : public CloneExpressions { public: ParamCloner() { setName("ParamCloner"); } const IR::Node *postorder(IR::Parameter *param) override { diff --git a/frontends/p4/sideEffects.cpp b/frontends/p4/sideEffects.cpp index e0854567a4..4ad039f3ab 100644 --- a/frontends/p4/sideEffects.cpp +++ b/frontends/p4/sideEffects.cpp @@ -477,7 +477,7 @@ const IR::Node *DoSimplifyExpressions::preorder(IR::MethodCallExpression *mce) { visit(mce->method); - ClonePathExpressions cloner; // a cheap version of deep copy + CloneExpressions cloner; // a cheap version of deep copy for (auto p : *mi->substitution.getParametersInArgumentOrder()) { auto arg = mi->substitution.lookup(p); if (p->direction == IR::Direction::None) { diff --git a/frontends/p4/simplifyDefUse.h b/frontends/p4/simplifyDefUse.h index 5722ed2ad6..3cebf9490d 100644 --- a/frontends/p4/simplifyDefUse.h +++ b/frontends/p4/simplifyDefUse.h @@ -61,7 +61,7 @@ class RemoveHidden : public Transform { }; class SimplifyDefUse : public PassManager { - class Cloner : public ClonePathExpressions { + class Cloner : public CloneExpressions { public: Cloner() { setName("Cloner"); } const IR::Node *postorder(IR::EmptyStatement *stat) override { @@ -83,7 +83,7 @@ class SimplifyDefUse : public PassManager { LOG2("Cloning " << getOriginal()->id << " into " << result->id); return result; } - // Ideally we'd like ClonePathExpressions::postorder(stat), + // Ideally we'd like CloneExpressions::postorder(stat), // but that doesn't work. return Transform::postorder(stat); } diff --git a/midend/predication.cpp b/midend/predication.cpp index d24325647d..4b9bdf8e0c 100644 --- a/midend/predication.cpp +++ b/midend/predication.cpp @@ -139,7 +139,7 @@ const IR::Expression *Predication::clone(const IR::Expression *expression) { // in the end different code will be generated for the different clones of // an expression. This is most obvious if one clone is on the LHS and one // on the RHS of an assigment. - ClonePathExpressions cloner; + CloneExpressions cloner; cloner.setCalledBy(this); return expression->apply(cloner); } @@ -148,7 +148,7 @@ const IR::Node *Predication::clone(const IR::AssignmentStatement *statement) { // Expressions often need to be cloned. This is necessary because // in the end different code will be generated for the different clones of // an expression. - ClonePathExpressions cloner; + CloneExpressions cloner; cloner.setCalledBy(this); return statement->apply(cloner); }