Make don't care args action-local when used in actions (p4lang#4817)

* Make don't care args action-local when used in actions

Signed-off-by: Kyle Cripps <[email protected]>

* Add gtest for RemoveDontcareArgs pass

Signed-off-by: kfcripps <[email protected]>

* Remove unused refMap and typeMap from strength reduction gtest

Signed-off-by: kfcripps <[email protected]>

* Appease cpplint

Signed-off-by: Kyle Cripps <[email protected]>

---------

Signed-off-by: Kyle Cripps <[email protected]>
Signed-off-by: kfcripps <[email protected]>

frontends/p4/dontcareArgs.h

@@ -44,6 +44,14 @@ class DontcareArgs : public Transform, public ResolutionContext {
         toAdd.clear();
         return function;
     }
+    const IR::Node *postorder(IR::P4Action *action) override {
+        IR::IndexedVector<IR::StatOrDecl> body;
+        for (auto d : toAdd) body.push_back(d);
+        body.append(action->body->components);
+        action->body = new IR::BlockStatement(action->body->srcInfo, body);
+        toAdd.clear();
+        return action;
+    }
     const IR::Node *postorder(IR::P4Parser *parser) override {
         toAdd.append(parser->parserLocals);
         parser->parserLocals = toAdd;

test/CMakeLists.txt

@@ -48,6 +48,7 @@ set (GTEST_UNITTEST_SOURCES
   gtest/ordered_set.cpp
   gtest/parser_unroll.cpp
   gtest/p4runtime.cpp
+  gtest/remove_dontcare_args_test.cpp
   gtest/source_file_test.cpp
   gtest/strength_reduction.cpp
   gtest/string_map.cpp

test/gtest/remove_dontcare_args_test.cpp

@@ -0,0 +1,115 @@
+#include <gtest/gtest.h>
+
+#include "frontends/common/parseInput.h"
+#include "frontends/p4/dontcareArgs.h"
+#include "frontends/p4/frontend.h"
+#include "helpers.h"
+#include "ir/ir.h"
+
+using namespace P4;
+
+namespace Test {
+
+struct RemoveDontcareArgsTest : P4CTest {
+    const IR::Node *parseAndProcess(std::string program) {
+        const auto *pgm = P4::parseP4String(program, CompilerOptions::FrontendVersion::P4_16);
+        EXPECT_TRUE(pgm);
+        EXPECT_EQ(::errorCount(), 0);
+        if (!pgm) {
+            return nullptr;
+        }
+
+        P4::TypeMap typeMap;
+        RemoveDontcareArgs rdca(&typeMap);
+
+        return pgm->apply(rdca);
+    }
+};
+
+class CollectActionAndControlLocals : public Inspector {
+ public:
+    unsigned actionDecls = 0;
+    unsigned controlDecls = 0;
+
+    bool preorder(const IR::P4Action *action) override {
+        for (const auto *c : action->body->components) {
+            if (c->is<IR::Declaration_Variable>()) ++actionDecls;
+        }
+        return true;
+    }
+
+    bool preorder(const IR::P4Control *control) override {
+        for (const auto *c : control->controlLocals) {
+            if (c->is<IR::Declaration_Variable>()) ++controlDecls;
+        }
+        return true;
+    }
+};
+
+TEST_F(RemoveDontcareArgsTest, Default) {
+    std::string program_source = P4_SOURCE(R"(
+struct S {
+    bit<64> f;
+}
+
+control C(inout S s) {
+    @name("d") action d_0(@name("b") out bit<64> b_0) {
+        b_0 = 64w4;
+    }
+    @name("foo") action foo_0() {
+        d_0(_);
+    }
+    @name("t") table t_0 {
+        actions = {
+            foo_0();
+        }
+        default_action = foo_0();
+    }
+    apply {
+        t_0.apply();
+    }
+}
+
+control proto(inout S s);
+package top(proto p);
+top(C()) main;
+    )");
+
+    const auto *program = parseAndProcess(program_source);
+    ASSERT_TRUE(program);
+    ASSERT_EQ(::errorCount(), 0);
+
+    CollectActionAndControlLocals collect;
+    program->apply(collect);
+
+    // Just make sure that the control block has no decls and the action has a single decl.
+    // The expected output should look something like this:
+    // struct S {
+    //     bit<64> f;
+    // }
+    // control C(inout S s) {
+    //     @name("d") action d_0(@name("b") out bit<64> b_0) {
+    //         b_0 = 64w4;
+    //     }
+    //     @name("foo") action foo_0() {
+    //         bit<64> arg;
+    //         d_0(arg);
+    //     }
+    //     @name("t") table t_0 {
+    //         actions = {
+    //             foo_0();
+    //         }
+    //         default_action = foo_0();
+    //     }
+    //     apply {
+    //         t_0.apply();
+    //     }
+    // }
+    // control proto(inout S s);
+    // package top(proto p);
+    // top(C()) main;
+    ASSERT_EQ(collect.actionDecls, 1);
+    ASSERT_EQ(collect.controlDecls, 0);
+}
+
+}  // namespace Test

test/gtest/strength_reduction.cpp

@@ -80,9 +80,6 @@ struct StrengthReductionPolicy : public P4::FrontEndPolicy {
 TEST_F(StrengthReductionTest, Default) {
     auto test = createStrengthReductionTestCase(P4_SOURCE(R"(headers.h.f1 = headers.h.f1 - 1;)"));
 
-    ReferenceMap refMap;
-    TypeMap typeMap;
-
     Util::SourceCodeBuilder builder;
     ToP4 top4(builder, false);
     test->program->apply(top4);
@@ -99,9 +96,6 @@ TEST_F(StrengthReductionTest, DisableSubConstToAddConst) {
     auto test =
         createStrengthReductionTestCase(P4_SOURCE(R"(headers.h.f1 = headers.h.f1 - 1;)"), &policy);
 
-    ReferenceMap refMap;
-    TypeMap typeMap;
-
     Util::SourceCodeBuilder builder;
     ToP4 top4(builder, false);
     test->program->apply(top4);

testdata/p4_16_samples/localize_action_dont_care_args.p4

@@ -0,0 +1,27 @@
+struct S {
+    bit<64> f;
+}
+
+control C(inout S s) {
+    action d(out bit<64> b) {
+        b = 4;
+    }
+
+    action foo() {
+        d(_);
+    }
+
+    table t {
+        actions = { foo; }
+        default_action = foo;
+    }
+
+    apply {
+        t.apply();
+    }
+}
+
+control proto(inout S s);
+package top(proto p);
+
+top(C()) main;

testdata/p4_16_samples_outputs/localize_action_dont_care_args-first.p4

@@ -0,0 +1,25 @@
+struct S {
+    bit<64> f;
+}
+
+control C(inout S s) {
+    action d(out bit<64> b) {
+        b = 64w4;
+    }
+    action foo() {
+        d(_);
+    }
+    table t {
+        actions = {
+            foo();
+        }
+        default_action = foo();
+    }
+    apply {
+        t.apply();
+    }
+}
+
+control proto(inout S s);
+package top(proto p);
+top(C()) main;

testdata/p4_16_samples_outputs/localize_action_dont_care_args-frontend.p4

@@ -0,0 +1,21 @@
+struct S {
+    bit<64> f;
+}
+
+control C(inout S s) {
+    @name("C.foo") action foo() {
+    }
+    @name("C.t") table t_0 {
+        actions = {
+            foo();
+        }
+        default_action = foo();
+    }
+    apply {
+        t_0.apply();
+    }
+}
+
+control proto(inout S s);
+package top(proto p);
+top(C()) main;

testdata/p4_16_samples_outputs/localize_action_dont_care_args-midend.p4

@@ -0,0 +1,21 @@
+struct S {
+    bit<64> f;
+}
+
+control C(inout S s) {
+    @name("C.foo") action foo() {
+    }
+    @name("C.t") table t_0 {
+        actions = {
+            foo();
+        }
+        default_action = foo();
+    }
+    apply {
+        t_0.apply();
+    }
+}
+
+control proto(inout S s);
+package top(proto p);
+top(C()) main;

testdata/p4_16_samples_outputs/localize_action_dont_care_args.p4

@@ -0,0 +1,25 @@
+struct S {
+    bit<64> f;
+}
+
+control C(inout S s) {
+    action d(out bit<64> b) {
+        b = 4;
+    }
+    action foo() {
+        d(_);
+    }
+    table t {
+        actions = {
+            foo;
+        }
+        default_action = foo;
+    }
+    apply {
+        t.apply();
+    }
+}
+
+control proto(inout S s);
+package top(proto p);
+top(C()) main;