Handle correctly general switch statements when simplifying (#3624)

* Handle correctly general switch statements when simplifying * Properly handle side-effects when removing switch statements Signed-off-by: Mihai Budiu <mbudiu@vmware.com>
p4lang · Nov 2, 2022 · 31e2d5f · 31e2d5f
1 parent 97cffff
commit 31e2d5f
Show file tree

Hide file tree

Showing 12 changed files with 95 additions and 10 deletions.
diff --git a/frontends/p4/simplify.cpp b/frontends/p4/simplify.cpp
@@ -16,6 +16,7 @@ limitations under the License.
 
 #include "simplify.h"
 #include "sideEffects.h"
+#include "tableApply.h"
 
 namespace P4 {
 
@@ -92,16 +93,19 @@ const IR::Node* DoSimplifyControlFlow::postorder(IR::EmptyStatement* statement)
 const IR::Node* DoSimplifyControlFlow::postorder(IR::SwitchStatement* statement)  {
     LOG3("Visiting " << dbp(getOriginal()));
     if (statement->cases.empty()) {
-        // The P4_16 spec prohibits expressions other than table application as
-        // switch conditions.  The parser should have rejected programs for
-        // which this is not the case.
-        BUG_CHECK(statement->expression->is<IR::Member>(),
-                  "%1%: expected a Member", statement->expression);
-        auto expr = statement->expression->to<IR::Member>();
-        BUG_CHECK(expr->expr->is<IR::MethodCallExpression>(),
-                  "%1%: expected a table invocation", expr->expr);
-        auto mce = expr->expr->to<IR::MethodCallExpression>();
-        return new IR::MethodCallStatement(mce->srcInfo, mce);
+        // If this is a table application remove the switch altogether but keep
+        // the table application.  Otherwise remove the switch altogether.
+        if (TableApplySolver::isActionRun(statement->expression, refMap, typeMap)) {
+            auto mce = statement->expression->checkedTo<IR::Member>()->
+                    expr->checkedTo<IR::MethodCallExpression>();
+            LOG2("Removing switch statement " << statement << " keeping " << mce);
+            return new IR::MethodCallStatement(statement->srcInfo, mce);
+        }
+        if (SideEffects::check(statement->expression, this, refMap, typeMap))
+            // This can happen if this pass is run before SideEffectOrdering.
+            return statement;
+        LOG2("Removing switch statement " << statement);
+        return nullptr;
     }
     auto last = statement->cases.back();
     if (last->statement == nullptr) {

diff --git a/testdata/p4_16_samples/issue3619-1.p4 b/testdata/p4_16_samples/issue3619-1.p4
@@ -0,0 +1,15 @@
+bit<4> f(in bit<4> d) {
+    return d;
+}
+
+control c(in bit<4> v) {
+    apply {
+        switch(f(v)) {
+        }
+    }
+}
+
+control C(in bit<4> b);
+package top(C c);
+
+top(c()) main;
diff --git a/testdata/p4_16_samples/issue3619.p4 b/testdata/p4_16_samples/issue3619.p4
@@ -0,0 +1,7 @@
+control c(in bit<4> v) {
+    apply {
+        switch(v) {
+            default:
+        }
+    }
+}
diff --git a/testdata/p4_16_samples_outputs/issue3619-1-first.p4 b/testdata/p4_16_samples_outputs/issue3619-1-first.p4
@@ -0,0 +1,13 @@
+bit<4> f(in bit<4> d) {
+    return d;
+}
+control c(in bit<4> v) {
+    apply {
+        switch (f(v)) {
+        }
+    }
+}
+
+control C(in bit<4> b);
+package top(C c);
+top(c()) main;
diff --git a/testdata/p4_16_samples_outputs/issue3619-1-frontend.p4 b/testdata/p4_16_samples_outputs/issue3619-1-frontend.p4
@@ -0,0 +1,8 @@
+control c(in bit<4> v) {
+    apply {
+    }
+}
+
+control C(in bit<4> b);
+package top(C c);
+top(c()) main;
diff --git a/testdata/p4_16_samples_outputs/issue3619-1-midend.p4 b/testdata/p4_16_samples_outputs/issue3619-1-midend.p4
@@ -0,0 +1,8 @@
+control c(in bit<4> v) {
+    apply {
+    }
+}
+
+control C(in bit<4> b);
+package top(C c);
+top(c()) main;
diff --git a/testdata/p4_16_samples_outputs/issue3619-1.p4 b/testdata/p4_16_samples_outputs/issue3619-1.p4
@@ -0,0 +1,13 @@
+bit<4> f(in bit<4> d) {
+    return d;
+}
+control c(in bit<4> v) {
+    apply {
+        switch (f(v)) {
+        }
+    }
+}
+
+control C(in bit<4> b);
+package top(C c);
+top(c()) main;
diff --git a/testdata/p4_16_samples_outputs/issue3619-1.p4-stderr b/testdata/p4_16_samples_outputs/issue3619-1.p4-stderr
diff --git a/testdata/p4_16_samples_outputs/issue3619-first.p4 b/testdata/p4_16_samples_outputs/issue3619-first.p4
@@ -0,0 +1,5 @@
+control c(in bit<4> v) {
+    apply {
+    }
+}
+
diff --git a/testdata/p4_16_samples_outputs/issue3619-frontend.p4 b/testdata/p4_16_samples_outputs/issue3619-frontend.p4
diff --git a/testdata/p4_16_samples_outputs/issue3619.p4 b/testdata/p4_16_samples_outputs/issue3619.p4
@@ -0,0 +1,8 @@
+control c(in bit<4> v) {
+    apply {
+        switch (v) {
+            default: 
+        }
+    }
+}
+
diff --git a/testdata/p4_16_samples_outputs/issue3619.p4-stderr b/testdata/p4_16_samples_outputs/issue3619.p4-stderr
@@ -0,0 +1,4 @@
+issue3619.p4(4): [--Wwarn=missing] warning: SwitchCase: fallthrough with no statement
+            default:
+            ^^^^^^^
+[--Wwarn=missing] warning: Program does not contain a `main' module