Fix for issue #323 (#327)

frontends/p4/fromv1.0/converters.h

@@ -1,5 +1,5 @@
 /*
-Copyright 2013-present Barefoot Networks, Inc. 
+Copyright 2013-present Barefoot Networks, Inc.
 
 Licensed under the Apache License, Version 2.0 (the "License");
 you may not use this file except in compliance with the License.

frontends/p4/uniqueNames.cpp

@@ -19,6 +19,31 @@ limitations under the License.
 
 namespace P4 {
 
+namespace {
+
+class FindActionCalls : public Inspector {
+    ReferenceMap* refMap;
+    TypeMap* typeMap;
+    RenameMap* renameMap;
+ public:
+    explicit FindActionCalls(ReferenceMap* refMap, TypeMap* typeMap, RenameMap* renameMap) :
+            refMap(refMap), typeMap(typeMap), renameMap(renameMap)
+    { CHECK_NULL(refMap); CHECK_NULL(typeMap); CHECK_NULL(renameMap); }
+
+    void postorder(const IR::MethodCallExpression* expression) {
+        auto mi = MethodInstance::resolve(expression, refMap, typeMap);
+        if (!mi->is<P4::ActionCall>())
+            return;
+        auto ac = mi->to<P4::ActionCall>();
+
+        auto table = findContext<IR::P4Table>();
+        if (table != nullptr)
+            renameMap->foundInTable(ac->action);
+    }
+};
+
+}  // namespace
+
 // Add a @name annotation ONLY if it does not already exist.
 // Otherwise do nothing.
 static const IR::Annotations*
@@ -42,7 +67,8 @@ UniqueParameters::UniqueParameters(ReferenceMap* refMap, TypeMap* typeMap) :
         renameMap(new RenameMap) {
     setName("UniqueParameters");
     CHECK_NULL(refMap); CHECK_NULL(typeMap);
-    passes.emplace_back(new ResolveReferences(refMap));
+    passes.emplace_back(new TypeChecking(refMap, typeMap));
+    passes.emplace_back(new FindActionCalls(refMap, typeMap, renameMap));
     passes.emplace_back(new FindParameters(refMap, renameMap));
     passes.emplace_back(new RenameSymbols(refMap, renameMap));
     passes.emplace_back(new ClearTypeMap(typeMap));

frontends/p4/uniqueNames.h

@@ -26,6 +26,8 @@ namespace P4 {
 class RenameMap {
     // Internal declaration name
     std::map<const IR::IDeclaration*, cstring> newName;
+    std::set<const IR::P4Action*> inTable;  // all actions that appear in tables
+
  public:
     void setNewName(const IR::IDeclaration* decl, cstring name) {
         CHECK_NULL(decl);
@@ -45,6 +47,10 @@ class RenameMap {
         CHECK_NULL(decl);
         return newName.find(decl) != newName.end();
     }
+    void foundInTable(const IR::P4Action* action)
+    { inTable.emplace(action); }
+    bool isInTable(const IR::P4Action* action)
+    { return inTable.find(action) != inTable.end(); }
 };
 
 // Give unique names to various declarations to make it easier to
@@ -110,9 +116,11 @@ class FindParameters : public Inspector {
     ReferenceMap* refMap;  // used to generate new names
     RenameMap*    renameMap;
 
-    void doParameters(const IR::ParameterList* pl) {
+    // If all is true then rename all parameters, else rename only
+    // directional parameters
+    void doParameters(const IR::ParameterList* pl, bool all) {
         for (auto p : *pl->parameters) {
-            if (p->direction == IR::Direction::None)
+            if (!all && p->direction == IR::Direction::None)
                 continue;
             cstring newName = refMap->newName(p->name);
             renameMap->setNewName(p, newName);
@@ -123,9 +131,11 @@ class FindParameters : public Inspector {
             refMap(refMap), renameMap(renameMap)
     { CHECK_NULL(refMap); CHECK_NULL(renameMap); setName("FindParameters"); }
     void postorder(const IR::P4Table* table) override
-    { doParameters(table->parameters); }
-    void postorder(const IR::P4Action* action) override
-    { doParameters(action->parameters); }
+    { doParameters(table->parameters, false); }
+    void postorder(const IR::P4Action* action) override {
+        bool inTable = renameMap->isInTable(action);
+        doParameters(action->parameters, !inTable);
+    }
 };
 
 class UniqueParameters : public PassManager {

testdata/p4_16_samples/issue323.p4

@@ -0,0 +1,57 @@
+/*
+Copyright 2013-present Barefoot Networks, 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 <core.p4>
+#include <v1model.p4>
+
+header hdr {
+    bit<32> f;
+}
+
+struct Headers {
+    hdr h;
+}
+
+struct Meta { }
+
+parser p(packet_in b, out Headers h,
+         inout Meta m, inout standard_metadata_t sm) {
+    state start {
+        b.extract(h.h);
+        transition accept;
+    }
+}
+
+control vrfy(in Headers h, inout Meta m) { apply {} }
+control update(inout Headers h, inout Meta m) { apply {} }
+
+control egress(inout Headers h, inout Meta m, inout standard_metadata_t sm) {
+    apply {}
+}
+
+control deparser(packet_out b, in Headers h) {
+    apply { b.emit(h); }
+}
+
+control ingress(inout Headers h, inout Meta m, inout standard_metadata_t sm) {
+    action my_a(bit<32> v) { h.h.f = v; }
+    apply {
+        my_a(0);
+        my_a(1);
+    }
+}
+
+V1Switch(p(), vrfy(), ingress(), egress(), update(), deparser()) main;

testdata/p4_16_samples_outputs/action_call_ebpf-midend.p4

@@ -12,9 +12,9 @@ parser prs(packet_in p, out Headers_t headers) {
 
 control pipe(inout Headers_t headers, out bool pass) {
     bool x;
-    bool rej;
+    bool rej_0;
     @name("Reject") action Reject_0() {
-        rej = x;
+        rej_0 = x;
         pass = x;
     }
     action act() {

testdata/p4_16_samples_outputs/direct-action1-midend.p4

@@ -1,8 +1,8 @@
 control c(inout bit<16> y) {
     bit<32> x;
-    bit<32> arg;
+    bit<32> arg_0;
     @name("a") action a_0() {
-        arg = x;
+        arg_0 = x;
         y = (bit<16>)x;
     }
     action act() {

testdata/p4_16_samples_outputs/issue323-frontend.p4

@@ -0,0 +1,53 @@
+#include <core.p4>
+#include <v1model.p4>
+
+header hdr {
+    bit<32> f;
+}
+
+struct Headers {
+    hdr h;
+}
+
+struct Meta {
+}
+
+parser p(packet_in b, out Headers h, inout Meta m, inout standard_metadata_t sm) {
+    state start {
+        b.extract<hdr>(h.h);
+        transition accept;
+    }
+}
+
+control vrfy(in Headers h, inout Meta m) {
+    apply {
+    }
+}
+
+control update(inout Headers h, inout Meta m) {
+    apply {
+    }
+}
+
+control egress(inout Headers h, inout Meta m, inout standard_metadata_t sm) {
+    apply {
+    }
+}
+
+control deparser(packet_out b, in Headers h) {
+    apply {
+        b.emit<Headers>(h);
+    }
+}
+
+control ingress(inout Headers h, inout Meta m, inout standard_metadata_t sm) {
+    @name("my_a") action my_a_0(bit<32> v) {
+        h.h.f = v;
+    }
+    apply {
+        my_a_0(32w0);
+        my_a_0(32w1);
+    }
+}
+
+V1Switch<Headers, Meta>(p(), vrfy(), ingress(), egress(), update(), deparser()) main;

testdata/p4_16_samples_outputs/issue323-midend.p4

@@ -0,0 +1,72 @@
+#include <core.p4>
+#include <v1model.p4>
+
+header hdr {
+    bit<32> f;
+}
+
+struct Headers {
+    hdr h;
+}
+
+struct Meta {
+}
+
+parser p(packet_in b, out Headers h, inout Meta m, inout standard_metadata_t sm) {
+    state start {
+        b.extract<hdr>(h.h);
+        transition accept;
+    }
+}
+
+control vrfy(in Headers h, inout Meta m) {
+    apply {
+    }
+}
+
+control update(inout Headers h, inout Meta m) {
+    apply {
+    }
+}
+
+control egress(inout Headers h, inout Meta m, inout standard_metadata_t sm) {
+    apply {
+    }
+}
+
+control deparser(packet_out b, in Headers h) {
+    apply {
+        b.emit<Headers>(h);
+    }
+}
+
+control ingress(inout Headers h, inout Meta m, inout standard_metadata_t sm) {
+    bit<32> v_0;
+    bit<32> v_1;
+    @name("my_a") action my_a_0() {
+        v_0 = 32w0;
+        h.h.f = 32w0;
+    }
+    @name("my_a") action my_a_2() {
+        v_1 = 32w1;
+        h.h.f = 32w1;
+    }
+    table tbl_my_a() {
+        actions = {
+            my_a_0();
+        }
+        const default_action = my_a_0();
+    }
+    table tbl_my_a_0() {
+        actions = {
+            my_a_2();
+        }
+        const default_action = my_a_2();
+    }
+    apply {
+        tbl_my_a.apply();
+        tbl_my_a_0.apply();
+    }
+}
+
+V1Switch<Headers, Meta>(p(), vrfy(), ingress(), egress(), update(), deparser()) main;

testdata/p4_16_samples_outputs/issue323.p4

@@ -0,0 +1,53 @@
+#include <core.p4>
+#include <v1model.p4>
+
+header hdr {
+    bit<32> f;
+}
+
+struct Headers {
+    hdr h;
+}
+
+struct Meta {
+}
+
+parser p(packet_in b, out Headers h, inout Meta m, inout standard_metadata_t sm) {
+    state start {
+        b.extract(h.h);
+        transition accept;
+    }
+}
+
+control vrfy(in Headers h, inout Meta m) {
+    apply {
+    }
+}
+
+control update(inout Headers h, inout Meta m) {
+    apply {
+    }
+}
+
+control egress(inout Headers h, inout Meta m, inout standard_metadata_t sm) {
+    apply {
+    }
+}
+
+control deparser(packet_out b, in Headers h) {
+    apply {
+        b.emit(h);
+    }
+}
+
+control ingress(inout Headers h, inout Meta m, inout standard_metadata_t sm) {
+    action my_a(bit<32> v) {
+        h.h.f = v;
+    }
+    apply {
+        my_a(0);
+        my_a(1);
+    }
+}
+
+V1Switch(p(), vrfy(), ingress(), egress(), update(), deparser()) main;