Make the tree cloner also clone Member(TypeNameExpression) to ensure …

…the result is a DAG (#4539)

* Make the tree cloner also clone Member(TypeNameExpression) to ensure the result is a DAG

Signed-off-by: Mihai Budiu <[email protected]>

* clang-format

Signed-off-by: Mihai Budiu <[email protected]>

* Rename ClonePathExpressions -> CloneExpressions

---------

Signed-off-by: Mihai Budiu <[email protected]>
Co-authored-by: kfcripps <[email protected]>

backends/bmv2/psa_switch/psaSwitch.cpp

@@ -284,7 +284,7 @@ void PsaSwitchBackend::convert(const IR::ToplevelBlock *tlb) {
         new P4::RemoveAllUnusedDeclarations(refMap, P4::RemoveUnusedPolicy()),
         // 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(); },

backends/bmv2/simple_switch/simpleSwitch.cpp

@@ -1189,7 +1189,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,

frontends/p4/cloner.h

@@ -21,19 +21,27 @@ 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();
         return path;
     }
 
+    // Clone expressions of the form Member(TypeNameExpression)
+    const IR::Node *postorder(IR::Member *member) override {
+        if (member->expr->is<IR::TypeNameExpression>()) {
+            return new IR::Member(member->expr->clone(), member->member);
+        }
+        return member;
+    }
+
     template <typename T>
     const T *clone(const IR::Node *node) {
         return node->apply(*this)->to<T>();

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 {

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) {

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);
         }

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);
 }

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;

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;

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;

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;

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;