WIP: partial fix for issue p4lang#386 (p4lang#395)

* WIP: partial fix for issue p4lang#386 * Remember the name of global actions * Made testing script robust to pass changes; save names for global actions * Fix incorrect comment * Rebased from upstream * missed one test case
hanw · Apr 4, 2017 · ea45b5d · ea45b5d
1 parent f66ba22
commit ea45b5d
Show file tree

Hide file tree

Showing 647 changed files with 8,054 additions and 8,131 deletions.
diff --git a/backends/bmv2/jsonconverter.cpp b/backends/bmv2/jsonconverter.cpp
@@ -103,6 +103,12 @@ class ErrorCodesVisitor : public Inspector {
     }
 };
 
+static cstring extVisibleName(const IR::IDeclaration* decl) {
+    cstring name = decl->externalName();
+    if (name.startsWith("."))
+        return name.substr(1);
+    return name;
+}
 
 // This pass makes sure that when several match tables share a selector, they use the same input for
 // the selection algorithm. This is because bmv2 considers that the selection key is part of the
@@ -440,7 +446,7 @@ class ExpressionConverter : public Inspector {
             auto field = st->getField(expression->member);
             if (field != nullptr)
                 // field could be a method call, i.e., isValid.
-                fieldName = field->externalName();
+                fieldName = extVisibleName(field);
         }
 
         // handle the 'error' type
@@ -907,7 +913,7 @@ JsonConverter::convertActionBody(const IR::Vector<IR::StatOrDecl>* body,
                         auto parameters = mkParameters(primitive);
                         auto ctr = new Util::JsonObject();
                         ctr->emplace("type", "counter_array");
-                        ctr->emplace("value", em->object->externalName());
+                        ctr->emplace("value", extVisibleName(em->object));
                         parameters->append(ctr);
                         auto index = conv->convert(mc->arguments->at(0));
                         parameters->append(index);
@@ -920,7 +926,7 @@ JsonConverter::convertActionBody(const IR::Vector<IR::StatOrDecl>* body,
                         auto parameters = mkParameters(primitive);
                         auto mtr = new Util::JsonObject();
                         mtr->emplace("type", "meter_array");
-                        mtr->emplace("value", em->object->externalName());
+                        mtr->emplace("value", extVisibleName(em->object));
                         parameters->append(mtr);
                         auto index = conv->convert(mc->arguments->at(0));
                         parameters->append(index);
@@ -932,7 +938,7 @@ JsonConverter::convertActionBody(const IR::Vector<IR::StatOrDecl>* body,
                     BUG_CHECK(mc->arguments->size() == 2, "Expected 2 arguments for %1%", mc);
                     auto reg = new Util::JsonObject();
                     reg->emplace("type", "register_array");
-                    cstring name = em->object->externalName();
+                    cstring name = extVisibleName(em->object);
                     reg->emplace("value", name);
                     if (em->method->name == v1model.registers.read.name) {
                         auto primitive = mkPrimitive("register_read", result);
@@ -1034,7 +1040,7 @@ JsonConverter::convertActionBody(const IR::Vector<IR::StatOrDecl>* body,
                             BUG_CHECK(origType->is<IR::Type_Struct>(),
                                       "%1%: expected a struct type", origType);
                             auto st = origType->to<IR::Type_Struct>();
-                            listName = st->externalName();
+                            listName = extVisibleName(st);
                         }
                     }
                     int id = createFieldList(mc->arguments->at(1), "learn_lists",
@@ -1062,7 +1068,7 @@ JsonConverter::convertActionBody(const IR::Vector<IR::StatOrDecl>* body,
                             BUG_CHECK(origType->is<IR::Type_Struct>(),
                                       "%1%: expected a struct type", origType);
                             auto st = origType->to<IR::Type_Struct>();
-                            listName = st->externalName();
+                            listName = extVisibleName(st);
                         }
                     }
                     int id = createFieldList(mc->arguments->at(0), "field_lists",
@@ -1162,7 +1168,7 @@ Util::JsonArray* JsonConverter::createActions(Util::JsonArray* fieldLists,
                 v1model.ingress.metadataParam.index);
         }
 
-        cstring name = action->externalName();
+        cstring name = extVisibleName(action);
         auto jact = new Util::JsonObject();
         jact->emplace("name", name);
         unsigned id = nextId("actions");
@@ -1303,7 +1309,7 @@ bool JsonConverter::handleTableImplementation(const IR::Property* implementation
             ::error("%1%: expected a reference to an instance", pathe);
             return false;
         }
-        apname = decl->externalName();
+        apname = extVisibleName(decl);
         auto dcltype = typeMap->getType(pathe, true);
         if (!dcltype->is<IR::Type_Extern>()) {
             ::error("%1%: unexpected type for implementation", dcltype);
@@ -1354,7 +1360,7 @@ JsonConverter::convertTable(const CFG::TableNode* node,
     auto table = node->table;
     LOG3("Processing " << dbp(table));
     auto result = new Util::JsonObject();
-    cstring name = table->externalName();
+    cstring name = extVisibleName(table);
     result->emplace("name", name);
     result->emplace("id", nextId("tables"));
     cstring table_match_type = "exact";
@@ -1553,7 +1559,9 @@ JsonConverter::convertTable(const CFG::TableNode* node,
                 }
                 meterMap.setTable(decl, table);
                 meterMap.setSize(decl, size);
-                cstring name = decl->externalName();
+                BUG_CHECK(decl->is<IR::Declaration_Instance>(),
+                          "%1%: expected an instance", decl->getNode());
+                cstring name = extVisibleName(decl);
                 result->emplace("direct_meters", name);
             }
         } else {
@@ -1579,7 +1587,7 @@ JsonConverter::convertTable(const CFG::TableNode* node,
         auto action = decl->to<IR::P4Action>();
         unsigned id = get(structure.ids, action);
         action_ids->append(id);
-        auto name = action->externalName();
+        auto name = extVisibleName(action);
         actions->append(name);
         useActionName.emplace(action->name, name);
     }
@@ -1750,7 +1758,7 @@ Util::IJson* JsonConverter::convertControl(const IR::ControlBlock* block, cstrin
         if (c->is<IR::Declaration_Instance>()) {
             auto bl = block->getValue(c);
             CHECK_NULL(bl);
-            cstring name = c->externalName();
+            cstring name = extVisibleName(c);
             if (bl->is<IR::ExternBlock>()) {
                 auto eb = bl->to<IR::ExternBlock>();
                 if (eb->type->name == v1model.counter.name) {
@@ -1844,7 +1852,7 @@ Util::IJson* JsonConverter::convertControl(const IR::ControlBlock* block, cstrin
                         type = "both";
                     jmtr->emplace("type", type);
                     jmtr->emplace("size", info->tableSize);
-                    cstring tblname = info->table->externalName();
+                    cstring tblname = extVisibleName(info->table);
                     jmtr->emplace("binding", tblname);
                     auto result = conv->convert(info->destinationField);
                     jmtr->emplace("result_target", result->to<Util::JsonObject>()->get("value"));
@@ -1907,7 +1915,7 @@ void JsonConverter::addHeaderStacks(const IR::Type_Struct* headersStruct) {
 
         LOG3("Creating " << stack);
         auto json = new Util::JsonObject();
-        json->emplace("name", f->externalName());
+        json->emplace("name", extVisibleName(f));
         json->emplace("id", nextId("stack"));
         json->emplace("size", stack->getSize());
         auto type = typeMap->getTypeType(stack->elementType, true);
@@ -1922,7 +1930,7 @@ void JsonConverter::addHeaderStacks(const IR::Type_Struct* headersStruct) {
             unsigned id = nextId("headers");
             stackMembers->append(id);
             auto header = new Util::JsonObject();
-            cstring name = f->externalName() + "[" + Util::toString(i) + "]";
+            cstring name = extVisibleName(f) + "[" + Util::toString(i) + "]";
             header->emplace("name", name);
             header->emplace("id", id);
             header->emplace("header_type", header_type);
@@ -2277,9 +2285,9 @@ void JsonConverter::addTypesAndInstances(const IR::Type_StructLike* type, bool m
         auto ft = typeMap->getType(f, true);
         if (ft->is<IR::Type_StructLike>()) {
             auto json = new Util::JsonObject();
-            json->emplace("name", f->externalName());
+            json->emplace("name", extVisibleName(f));
             json->emplace("id", nextId("headers"));
-            json->emplace("header_type", ft->to<IR::Type_StructLike>()->externalName());
+            json->emplace("header_type", extVisibleName(ft->to<IR::Type_StructLike>()));
             json->emplace("metadata", meta);
             headerInstances->append(json);
         } else if (ft->is<IR::Type_Stack>()) {
@@ -2353,7 +2361,7 @@ void JsonConverter::pushFields(cstring prefix, const IR::Type_StructLike *st,
 cstring JsonConverter::createJsonType(const IR::Type_StructLike *st) {
     if (headerTypesCreated.count(st->name)) return headerTypesCreated[st->name];
     auto typeJson = new Util::JsonObject();
-    cstring name = st->externalName();
+    cstring name = extVisibleName(st);
     headerTypesCreated[st->name] = name;
     typeJson->emplace("name", name);
     typeJson->emplace("id", nextId("header_types"));
@@ -2635,7 +2643,7 @@ Util::IJson* JsonConverter::toJson(const IR::ParserState* state) {
         return nullptr;
 
     auto result = new Util::JsonObject();
-    result->emplace("name", state->externalName());
+    result->emplace("name", extVisibleName(state));
     result->emplace("id", nextId("parse_states"));
     auto operations = mkArrayField(result, "parser_ops");
     for (auto s : *state->components) {

diff --git a/backends/bmv2/midend.cpp b/backends/bmv2/midend.cpp
@@ -51,6 +51,7 @@ limitations under the License.
 #include "midend/predication.h"
 #include "midend/expandLookahead.h"
 #include "midend/tableHit.h"
+#include "midend/midEndLast.h"
 
 namespace BMV2 {
 
@@ -186,7 +187,8 @@ MidEnd::MidEnd(CompilerOptions& options) {
         new P4::SimplifyControlFlow(&refMap, &typeMap),
         new P4::RemoveAllUnusedDeclarations(&refMap),
         evaluator,
-        new VisitFunctor([this, evaluator]() { toplevel = evaluator->getToplevelBlock(); })
+        new VisitFunctor([this, evaluator]() { toplevel = evaluator->getToplevelBlock(); }),
+        new P4::MidEndLast()
     });
 }
 

diff --git a/backends/ebpf/midend.cpp b/backends/ebpf/midend.cpp
@@ -30,6 +30,7 @@ limitations under the License.
 #include "midend/eliminateTuples.h"
 #include "midend/noMatch.h"
 #include "midend/convertEnums.h"
+#include "midend/midEndLast.h"
 #include "frontends/p4/uniqueNames.h"
 #include "frontends/p4/moveDeclarations.h"
 #include "frontends/p4/typeMap.h"
@@ -115,6 +116,7 @@ const IR::ToplevelBlock* MidEnd::run(EbpfOptions& options, const IR::P4Program*
         new P4::SimplifyControlFlow(&refMap, &typeMap),
         new P4::ValidateTableProperties({"implementation"}),
         evaluator,
+        new P4::MidEndLast()
     };
     midEnd.setName("MidEnd");
     midEnd.addDebugHooks(hooks);

diff --git a/backends/p4test/midend.cpp b/backends/p4test/midend.cpp
@@ -33,7 +33,9 @@ limitations under the License.
 #include "midend/copyStructures.h"
 #include "midend/predication.h"
 #include "midend/noMatch.h"
+#include "midend/tableHit.h"
 #include "midend/expandLookahead.h"
+#include "midend/midEndLast.h"
 #include "frontends/p4/simplifyParsers.h"
 #include "frontends/p4/typeMap.h"
 #include "frontends/p4/evaluator/evaluator.h"
@@ -132,10 +134,12 @@ MidEnd::MidEnd(CompilerOptions& options) {
         new P4::MoveDeclarations(),  // more may have been introduced
         new P4::SimplifyControlFlow(&refMap, &typeMap),
         new P4::CompileTimeOperations(),
+        new P4::TableHit(&refMap, &typeMap),
         new P4::SynthesizeActions(&refMap, &typeMap, new SkipControls(v1controls)),
         new P4::MoveActionsToTables(&refMap, &typeMap),
         evaluator,
-        new VisitFunctor([this, evaluator]() { toplevel = evaluator->getToplevelBlock(); })
+        new VisitFunctor([this, evaluator]() { toplevel = evaluator->getToplevelBlock(); }),
+        new P4::MidEndLast()
     });
 }
 

diff --git a/backends/p4test/run-p4-sample.py b/backends/p4test/run-p4-sample.py
@@ -26,6 +26,7 @@
 import shutil
 import difflib
 import subprocess
+import glob
 
 SUCCESS = 0
 FAILURE = 1
@@ -169,8 +170,8 @@ def process_file(options, argv):
 
     # We rely on the fact that these keys are in alphabetical order.
     rename = { "FrontEnd_12_SimplifyControlFlow": "first",
-               "FrontEnd_27_FrontEndLast": "frontend",
-               "MidEnd_35_Evaluator": "midend" }
+               "FrontEndLast": "frontend",
+               "MidEndLast": "midend" }
 
     if options.verbose:
         print("Writing temporary files into ", tmpdir)
@@ -211,11 +212,15 @@ def process_file(options, argv):
     lastFile = None
 
     for k in sorted(rename.keys()):
-        file = file_name(tmpdir, base, k, ext)
-        if os.path.isfile(file):
-            newName = file_name(tmpdir, base, rename[k], ext)
-            os.rename(file, newName)
-            lastFile = newName
+        files = glob.glob(tmpdir + "/" + base + "*" + k + "*.p4");
+        if len(files) > 1:
+            print("Multiple files matching", k);
+        elif len(files) == 1:
+            file = files[0]
+            if os.path.isfile(file):
+                newName = file_name(tmpdir, base, rename[k], ext)
+                os.rename(file, newName)
+                lastFile = newName
 
     if (result == SUCCESS):
         result = check_generated_files(options, tmpdir, expected_dirname);

diff --git a/frontends/p4/fromv1.0/programStructure.cpp b/frontends/p4/fromv1.0/programStructure.cpp
@@ -1385,8 +1385,9 @@ ProgramStructure::convertAction(const IR::ActionFunction* action, cstring newNam
         body->push_back(stat);
     }
 
-    // Save the original action name in an annotation
-    auto annos = addNameAnnotation(action->name, action->annotations);
+    // Save the original action name in an annotation.
+    // The leading dot indicates a global action
+    auto annos = addNameAnnotation(cstring(".") + action->name, action->annotations);
     auto result = new IR::P4Action(
         action->srcInfo, newName, annos, new IR::ParameterList(params),
         new IR::BlockStatement(body->srcInfo, IR::Annotations::empty, body));

diff --git a/midend/Makefile.am b/midend/Makefile.am
@@ -65,6 +65,7 @@ noinst_HEADERS += \
 	midend/validateProperties.h \
 	midend/expandLookahead.h \
 	midend/noMatch.h \
-	midend/tableHit.h
+	midend/tableHit.h \
+	midend/midEndLast.h
 
 cpplint_FILES += $(midend_UNIFIED) $(midend_NONUNIFIED)
diff --git a/midend/inlining.cpp b/midend/inlining.cpp
@@ -184,6 +184,7 @@ class ComputeNewNames : public Inspector {
     cstring           prefix;
     P4::ReferenceMap* refMap;
     SymRenameMap*     renameMap;
+
  public:
     ComputeNewNames(cstring prefix, P4::ReferenceMap* refMap, SymRenameMap* renameMap) :
             prefix(prefix), refMap(refMap), renameMap(renameMap) {
@@ -194,7 +195,12 @@ class ComputeNewNames : public Inspector {
     void rename(const IR::Declaration* decl) {
         BUG_CHECK(decl->is<IR::IAnnotated>(), "%1%: no annotations", decl);
         cstring name = decl->externalName();
-        cstring extName = prefix + "." + name;
+        cstring extName;
+        if (name.startsWith("."))
+            // Do not change the external name of objects starting with a leading dot
+            extName = name;
+        else
+            extName = prefix + "." + name;
         cstring baseName = extName.replace('.', '_');
         cstring newName = refMap->newName(baseName);
         renameMap->setNewName(decl, newName, extName);

diff --git a/midend/localizeActions.cpp b/midend/localizeActions.cpp
@@ -36,6 +36,20 @@ class ParamCloner : public ClonePathExpressions {
 
 }  // namespace
 
+const IR::Node* TagGlobalActions::preorder(IR::P4Action* action) {
+    if (findContext<IR::P4Control>() == nullptr) {
+        auto annos = action->annotations;
+        if (annos == nullptr)
+            annos = IR::Annotations::empty;
+        cstring name = cstring(".") + action->name;
+        annos = annos->addAnnotationIfNew(IR::Annotation::nameAnnotation,
+                                          new IR::StringLiteral(Util::SourceInfo(), name));
+        action->annotations = annos;
+    }
+    prune();
+    return action;
+}
+
 bool FindGlobalActionUses::preorder(const IR::P4Action* action) {
     if (findContext<IR::P4Control>() == nullptr)
         globalActions.emplace(action);

diff --git a/midend/localizeActions.h b/midend/localizeActions.h
@@ -136,11 +136,23 @@ class DuplicateActions : public Transform {
     const IR::Node* postorder(IR::P4Control* control) override;
 };
 
+// Add a @name annotation on each global action that does not have one
+class TagGlobalActions : public Transform {
+ public:
+    TagGlobalActions() { setName("TagGlobalActions"); }
+    const IR::Node* preorder(IR::P4Action* action) override;
+    const IR::Node* preorder(IR::P4Parser* parser) override
+    { prune(); return parser; }
+    const IR::Node* preorder(IR::P4Control* control) override
+    { prune(); return control; }
+};
+
 class LocalizeAllActions : public PassManager {
     GlobalActionReplacements globalReplacements;
     ActionReplacement        localReplacements;
  public:
     explicit LocalizeAllActions(ReferenceMap* refMap) {
+        passes.emplace_back(new TagGlobalActions());
         passes.emplace_back(new PassRepeated {
             new ResolveReferences(refMap),
             new FindGlobalActionUses(refMap, &globalReplacements),

diff --git a/midend/midEndLast.h b/midend/midEndLast.h
@@ -0,0 +1,33 @@
+/*
+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.
+*/
+
+#ifndef _MIDEND_MIDENDLAST_H_
+#define _MIDEND_MIDENDLAST_H_
+
+#include "ir/ir.h"
+
+namespace P4 {
+
+class MidEndLast : public Inspector {
+ public:
+    MidEndLast() { setName("MidEndLast"); }
+    bool preorder(const IR::P4Program*) override
+    { return false; }
+};
+
+}  // namespace P4
+
+#endif /* _MIDEND_MIDENDLAST_H_ */