Use type-safe discriminated union for Annotation (#5018)

* Reduce size of Annotation by ~30% via switching to std::variant for different fields Signed-off-by: Anton Korobeynikov <[email protected]> * Ensure vector of annotations is printed neatly Signed-off-by: Anton Korobeynikov <[email protected]> * Simplify Signed-off-by: Anton Korobeynikov <[email protected]> * Add missed headers Signed-off-by: Anton Korobeynikov <[email protected]> * Unbreak Tofino Signed-off-by: Anton Korobeynikov <[email protected]> * Address review comments Signed-off-by: Anton Korobeynikov <[email protected]> * Add getExpr() helper for Annotation Signed-off-by: Anton Korobeynikov <[email protected]> * Address review comments Signed-off-by: Anton Korobeynikov <[email protected]> --------- Signed-off-by: Anton Korobeynikov <[email protected]>
p4lang · Nov 19, 2024 · e66525a · e66525a
1 parent 9bef7ee
commit e66525a
Show file tree

Hide file tree

Showing 96 changed files with 873 additions and 578 deletions.
diff --git a/backends/bmv2/common/control.h b/backends/bmv2/common/control.h
@@ -120,8 +120,8 @@ class ControlConverter : public Inspector {
                     keyelement->emplace("match_type", match_type);
                 }
                 if (auto na = ke->getAnnotation(IR::Annotation::nameAnnotation)) {
-                    BUG_CHECK(na->expr.size() == 1, "%1%: expected 1 name", na);
-                    auto name = na->expr[0]->to<IR::StringLiteral>();
+                    BUG_CHECK(na->getExpr().size() == 1, "%1%: expected 1 name", na);
+                    auto name = na->getExpr(0)->to<IR::StringLiteral>();
                     BUG_CHECK(name != nullptr, "%1%: expected a string", na);
                     // This is a BMv2 JSON extension: specify a
                     // control-plane name for this key
@@ -578,16 +578,14 @@ class ControlConverter : public Inspector {
             }
             entry->emplace("action_entry", action);
 
-            auto priorityAnnotation = e->getAnnotation("priority"_cs);
-            if (priorityAnnotation != nullptr) {
-                if (priorityAnnotation->expr.size() > 1)
-                    ::P4::error(ErrorType::ERR_INVALID, "Invalid priority value %1%",
-                                priorityAnnotation->expr);
-                auto priValue = priorityAnnotation->expr.front();
+            if (auto priorityAnnotation = e->getAnnotation("priority"_cs)) {
+                const auto &expr = priorityAnnotation->getExpr();
+                if (expr.size() > 1)
+                    ::P4::error(ErrorType::ERR_INVALID, "Invalid priority value %1%", expr);
+                auto priValue = expr.front();
                 if (!priValue->is<IR::Constant>())
                     ::P4::error(ErrorType::ERR_INVALID,
-                                "Invalid priority value %1%; must be constant.",
-                                priorityAnnotation->expr);
+                                "Invalid priority value %1%; must be constant.", expr);
                 entry->emplace("priority", priValue->to<IR::Constant>()->value);
             } else {
                 entry->emplace("priority", entryPriority);

diff --git a/backends/bmv2/common/header.cpp b/backends/bmv2/common/header.cpp
@@ -292,10 +292,10 @@ void HeaderConverter::addHeaderType(const IR::Type_StructLike *st) {
             auto alias = new Util::JsonArray();
             cstring target_name;
             if (BMV2Context::get().options().loadIRFromJson == false) {
-                target_name = aliasAnnotation->expr.front()->to<IR::StringLiteral>()->value;
+                target_name = aliasAnnotation->getExpr().front()->to<IR::StringLiteral>()->value;
             } else {
-                if (aliasAnnotation->body.size() != 0) {
-                    target_name = aliasAnnotation->body.at(0)->text;
+                if (aliasAnnotation->getUnparsed().size() != 0) {
+                    target_name = aliasAnnotation->getUnparsed().at(0)->text;
                 } else {
                     // aliasAnnotation->body is empty or not saved correctly
                     ::P4::error(ErrorType::ERR_INVALID,

diff --git a/backends/bmv2/common/parser.cpp b/backends/bmv2/common/parser.cpp
@@ -515,7 +515,7 @@ void ParserConverter::addValueSets(const IR::P4Parser *parser) {
     auto isExactMatch = [this](const IR::StructField *sf) {
         auto matchAnnotation = sf->getAnnotation(IR::Annotation::matchAnnotation);
         if (!matchAnnotation) return true;  // default (missing annotation) is exact
-        auto matchPathExpr = matchAnnotation->expr[0]->to<IR::PathExpression>();
+        auto matchPathExpr = matchAnnotation->getExpr(0)->to<IR::PathExpression>();
         CHECK_NULL(matchPathExpr);
         auto matchTypeDecl =
             ctxt->refMap->getDeclaration(matchPathExpr->path, true)->to<IR::Declaration_ID>();

diff --git a/backends/bmv2/simple_switch/simpleSwitch.cpp b/backends/bmv2/simple_switch/simpleSwitch.cpp
@@ -1041,7 +1041,7 @@ void SimpleSwitchBackend::createRecirculateFieldsList(ConversionContext *ctxt,
         auto anno = f->getAnnotation("field_list"_cs);
         if (anno == nullptr) continue;
 
-        for (auto e : anno->expr) {
+        for (auto e : anno->getExpr()) {
             auto cst = e->to<IR::Constant>();
             if (cst == nullptr) {
                 ::P4::error(ErrorType::ERR_UNSUPPORTED_ON_TARGET,

diff --git a/backends/ebpf/target.h b/backends/ebpf/target.h
@@ -203,15 +203,16 @@ class KernelSamplesTarget : public Target {
 class P4TCTarget : public KernelSamplesTarget {
  public:
     explicit P4TCTarget(bool emitTrace) : KernelSamplesTarget(emitTrace, "P4TC"_cs) {}
-    cstring getByteOrderFromAnnotation(const IR::Vector<IR::Annotation> &annotations) const {
-        for (const auto *anno : annotations) {
-            if (anno->name != "tc_type") continue;
-            for (const auto *annoVal : anno->body) {
-                if (annoVal->text == "macaddr" || annoVal->text == "ipv4" ||
-                    annoVal->text == "ipv6" || annoVal->text == "be16" || annoVal->text == "be32" ||
-                    annoVal->text == "be64") {
-                    return "NETWORK"_cs;
-                }
+    // FIXME: The code is terrible broken: the function is called on both
+    // *parsed* and *unparsed* annotations (!!!!)
+    static cstring getByteOrderFromAnnotation(const IR::IAnnotated *node) {
+        if (const auto *anno = node->getAnnotation("tc_type"_cs)) {
+            cstring value = anno->needsParsing()
+                                ? anno->getUnparsed().at(0)->text
+                                : anno->getExpr().at(0)->checkedTo<IR::StringLiteral>()->value;
+            if (value == "macaddr" || value == "ipv4" || value == "ipv6" || value == "be16" ||
+                value == "be32" || value == "be64") {
+                return "NETWORK"_cs;
             }
         }
         return "HOST"_cs;
@@ -223,14 +224,14 @@ class P4TCTarget : public KernelSamplesTarget {
             auto type = typeMap->getType(mem->expr, true);
             if (type->is<IR::Type_StructLike>()) {
                 auto field = type->to<IR::Type_StructLike>()->getField(mem->member);
-                return getByteOrderFromAnnotation(field->getAnnotations());
+                return getByteOrderFromAnnotation(field);
             }
         } else if (action) {
             auto paramList = action->getParameters();
             if (paramList != nullptr && !paramList->empty()) {
                 for (auto param : paramList->parameters) {
                     if (param->name.originalName == exp->toString()) {
-                        return getByteOrderFromAnnotation(param->getAnnotations());
+                        return getByteOrderFromAnnotation(param);
                     }
                 }
             }

diff --git a/backends/p4fmt/p4formatter.cpp b/backends/p4fmt/p4formatter.cpp
@@ -1100,40 +1100,48 @@ bool P4Formatter::preorder(const IR::Annotation *a) {
     builder.append(a->name);
     const char *open = a->structured ? "[" : "(";
     const char *close = a->structured ? "]" : ")";
-    if (!a->expr.empty()) {
-        builder.append(open);
-        {
-            WithSeparator comma(*this, ", ");
-            preorder(&a->expr);
-        }
-        builder.append(close);
-    }
-    if (!a->kv.empty()) {
-        builder.append(open);
-        visitCollection(a->kv, ", ", [&](const auto &kvp) {
-            builder.append(kvp->name);
-            builder.append("=");
-            visit(kvp->expression);
-        });
-        builder.append(close);
-    }
-    if (a->expr.empty() && a->kv.empty() && a->structured) {
-        builder.append("[]");
-    }
-    if (!a->body.empty() && a->expr.empty() && a->kv.empty()) {
-        // Have an unparsed annotation.
-        // We could be prettier here with smarter logic, but let's do the easy
-        // thing by separating every token with a space.
-        builder.append(open);
-        visitCollection(a->body, " ", [&](const auto &tok) {
-            bool haveStringLiteral =
-                tok->token_type == P4::P4Parser::token_type::TOK_STRING_LITERAL;
-            if (haveStringLiteral) builder.append("\"");
-            builder.append(tok->text);
-            if (haveStringLiteral) builder.append("\"");
-        });
-        builder.append(close);
-    }
+
+    std::visit(
+        [&](const auto &body) {
+            using T = std::decay_t<decltype(body)>;
+            if constexpr (std::is_same_v<T, IR::Vector<IR::AnnotationToken>>) {
+                // Have an unparsed annotation.
+                // We could be prettier here with smarter logic, but let's do the easy
+                // thing by separating every token with a space.
+                builder.append(open);
+                visitCollection(body, " ", [&](const auto &tok) {
+                    bool haveStringLiteral =
+                        tok->token_type == P4::P4Parser::token_type::TOK_STRING_LITERAL;
+                    if (haveStringLiteral) builder.append("\"");
+                    builder.append(tok->text);
+                    if (haveStringLiteral) builder.append("\"");
+                });
+                builder.append(close);
+            } else if constexpr (std::is_same_v<T, IR::Vector<IR::Expression>>) {
+                // Do not print () for empty unstructured annotations
+                if (body.empty() && !a->structured) return;
+                builder.append(open);
+                {
+                    WithSeparator comma(*this, ", ");
+                    preorder(&body);
+                }
+                builder.append(close);
+            } else if constexpr (std::is_same_v<T, IR::IndexedVector<IR::NamedExpression>>) {
+                // Do not print () for empty unstructured annotations
+                if (body.empty() && !a->structured) return;
+                builder.append(open);
+                visitCollection(body, ", ", [&](const auto &kvp) {
+                    builder.append(kvp->name);
+                    builder.append("=");
+                    visit(kvp->expression);
+                });
+                builder.append(close);
+            } else {
+                BUG("Unexpected variant field");
+            }
+        },
+        a->body);
+
     return false;
 }
 

diff --git a/backends/p4tools/common/compiler/reachability.cpp b/backends/p4tools/common/compiler/reachability.cpp
@@ -26,12 +26,11 @@ P4ProgramDCGCreator::P4ProgramDCGCreator(NodesCallGraph *dcg) : dcg(dcg), p4prog
 }
 
 bool P4ProgramDCGCreator::preorder(const IR::Annotation *annotation) {
-    if (annotation->name != IR::Annotation::nameAnnotation) {
-        return true;
-    }
+    if (annotation->name != IR::Annotation::nameAnnotation) return true;
+
     IR::ID name;
-    if (!annotation->expr.empty()) {
-        if (const auto *strLit = annotation->expr[0]->to<IR::StringLiteral>()) {
+    if (!annotation->getExpr().empty()) {
+        if (const auto *strLit = annotation->getExpr(0)->to<IR::StringLiteral>()) {
             if (strLit->value == ".NoAction") {
                 // Ignore NoAction annotations, because they aren't unique.
                 return true;

diff --git a/backends/p4tools/modules/smith/common/table.cpp b/backends/p4tools/modules/smith/common/table.cpp
@@ -49,7 +49,7 @@ IR::Key *TableGenerator::genKeyElementList(size_t len) {
         // @name
         // Tao: actually, this may never happen
         const auto *keyAnno = key->getAnnotations().at(0);
-        const auto *annotExpr = keyAnno->expr.at(0);
+        const auto *annotExpr = keyAnno->getExpr(0);
         cstring keyAnnotatName;
         if (annotExpr->is<IR::StringLiteral>()) {
             const auto *strExpr = annotExpr->to<IR::StringLiteral>();

diff --git a/backends/p4tools/modules/testgen/targets/bmv2/expr_stepper.cpp b/backends/p4tools/modules/testgen/targets/bmv2/expr_stepper.cpp
@@ -47,10 +47,9 @@ std::string Bmv2V1ModelExprStepper::getClassName() { return "Bmv2V1ModelExprStep
 bool Bmv2V1ModelExprStepper::isPartOfFieldList(const IR::StructField *field,
                                                uint64_t recirculateIndex) {
     // Check whether the field has a "field_list" annotation associated with it.
-    const auto *annotation = field->getAnnotation("field_list"_cs);
-    if (annotation != nullptr) {
+    if (const auto *annotation = field->getAnnotation("field_list"_cs)) {
         // Grab the index of the annotation.
-        auto annoExprs = annotation->expr;
+        auto annoExprs = annotation->getExpr();
         auto annoExprSize = annoExprs.size();
         BUG_CHECK(annoExprSize == 1,
                   "The field list annotation should only have one member. Has %1%.", annoExprSize);

diff --git a/backends/p4tools/modules/testgen/targets/bmv2/p4_asserts_parser.cpp b/backends/p4tools/modules/testgen/targets/bmv2/p4_asserts_parser.cpp
@@ -524,7 +524,7 @@ const IR::Node *AssertsParser::postorder(IR::P4Action *actionContext) {
         typeMap[arg->controlPlaneName()] = arg->type;
     }
 
-    for (const auto *restrStr : annotation->body) {
+    for (const auto *restrStr : annotation->getUnparsed()) {
         auto restrictions =
             genIRStructs(actionContext->controlPlaneName(), restrStr->text, typeMap);
         // Using Z3Solver, we check the feasibility of restrictions, if they are not
@@ -551,7 +551,7 @@ const IR::Node *AssertsParser::postorder(IR::P4Table *tableContext) {
     }
 
     Z3Solver solver;
-    for (const auto *restrStr : annotation->body) {
+    for (const auto *restrStr : annotation->getUnparsed()) {
         auto restrictions = genIRStructs(tableContext->controlPlaneName(), restrStr->text, typeMap);
         // Using Z3Solver, we check the feasibility of restrictions, if they are not
         // feasible, we delete keys and entries from the table to execute

diff --git a/backends/p4tools/modules/testgen/targets/bmv2/p4_refers_to_parser.cpp b/backends/p4tools/modules/testgen/targets/bmv2/p4_refers_to_parser.cpp
@@ -91,7 +91,7 @@ const IR::SymbolicVariable *RefersToParser::lookUpKeyInTable(const IR::P4Table &
 
 const IR::SymbolicVariable *RefersToParser::getReferencedKey(const IR::P4Control &ctrlContext,
                                                              const IR::Annotation &refersAnno) {
-    const auto &annotationList = refersAnno.body;
+    const auto &annotationList = refersAnno.getUnparsed();
     BUG_CHECK(annotationList.size() > 2,
               "'@refers_to' annotation %1% does not have the correct format.", refersAnno);
 

diff --git a/backends/p4tools/modules/testgen/targets/bmv2/p4runtime_translation.cpp b/backends/p4tools/modules/testgen/targets/bmv2/p4runtime_translation.cpp
@@ -21,15 +21,15 @@ P4Tools::P4Testgen::Bmv2::PropagateP4RuntimeTranslation::lookupP4RuntimeAnnotati
     }
     const auto *p4runtimeAnnotation = annotatedType->getAnnotation("p4runtime_translation"_cs);
     if (p4runtimeAnnotation != nullptr) {
-        BUG_CHECK(!p4runtimeAnnotation->needsParsing,
+        BUG_CHECK(!p4runtimeAnnotation->needsParsing(),
                   "The @p4runtime_translation annotation should have been parsed already.");
         p4RuntimeAnnotations.push_back(p4runtimeAnnotation);
     }
     const auto *p4runtimeTranslationMappings =
         annotatedType->getAnnotation("p4runtime_translation_mappings"_cs);
     if (p4runtimeTranslationMappings != nullptr) {
         BUG_CHECK(
-            !p4runtimeTranslationMappings->needsParsing,
+            !p4runtimeTranslationMappings->needsParsing(),
             "The @p4runtime_translation_mappings annotation should have been parsed already.");
         p4RuntimeAnnotations.push_back(p4runtimeTranslationMappings);
     }

diff --git a/backends/p4tools/modules/testgen/targets/bmv2/test_backend/protobuf_ir.cpp b/backends/p4tools/modules/testgen/targets/bmv2/test_backend/protobuf_ir.cpp
@@ -30,7 +30,7 @@ std::optional<std::string> ProtobufIr::checkForP4RuntimeTranslationAnnotation(
     if (p4RuntimeTranslationAnnotation == nullptr) {
         return std::nullopt;
     }
-    auto annotationVector = p4RuntimeTranslationAnnotation->expr;
+    auto annotationVector = p4RuntimeTranslationAnnotation->getExpr();
     BUG_CHECK(annotationVector.size() == 2, "Expected size of %1% to be 2. ", annotationVector);
     const auto *targetValue = annotationVector.at(1);
     if (targetValue->is<IR::StringLiteral>()) {
@@ -50,9 +50,9 @@ std::map<cstring, cstring> ProtobufIr::getP4RuntimeTranslationMappings(const IR:
     if (p4RuntimeTranslationMappingAnnotation == nullptr) {
         return p4RuntimeTranslationMappings;
     }
-    BUG_CHECK(!p4RuntimeTranslationMappingAnnotation->needsParsing,
+    BUG_CHECK(!p4RuntimeTranslationMappingAnnotation->needsParsing(),
               "The @p4runtime_translation_mappings annotation should have been parsed already.");
-    auto annotationExpr = p4RuntimeTranslationMappingAnnotation->expr;
+    auto annotationExpr = p4RuntimeTranslationMappingAnnotation->getExpr();
     BUG_CHECK(annotationExpr.size() == 1, "Expected size of %1% to be 1. ", annotationExpr);
     const auto *exprList = annotationExpr.at(0)->checkedTo<IR::ListExpression>();
     for (const auto *expr : exprList->components) {
@@ -76,8 +76,9 @@ std::string ProtobufIr::getFormatOfNode(const IR::IAnnotated *node) {
     if (formatAnnotation == nullptr) {
         return "hex_str";
     }
-    BUG_CHECK(formatAnnotation->body.size() == 1, "@format annotation can only have one member.");
-    auto annotationFormatString = formatAnnotation->body.at(0)->text;
+    BUG_CHECK(formatAnnotation->getUnparsed().size() == 1,
+              "@format annotation can only have one member.");
+    auto annotationFormatString = formatAnnotation->getUnparsed().at(0)->text;
     if (annotationFormatString == "IPV4_ADDRESS") {
         return "ipv4";
     }

diff --git a/backends/tc/backend.cpp b/backends/tc/backend.cpp
@@ -247,7 +247,7 @@ void ConvertToBackendIR::postorder(const IR::P4Action *action) {
                     tcActionParam->setBitSize(width);
                 }
                 if (const auto *anno = param->getAnnotation(ParseTCAnnotations::tcType)) {
-                    auto expr = anno->expr[0];
+                    auto expr = anno->getExpr(0);
                     if (auto typeLiteral = expr->to<IR::StringLiteral>()) {
                         auto val = getTcType(typeLiteral);
                         if (val != TC::BIT_TYPE) {
@@ -302,8 +302,8 @@ void ConvertToBackendIR::updateConstEntries(const IR::P4Table *t, IR::TCTable *t
         for (size_t itr = 0; itr < keyset->components.size(); itr++) {
             auto keyElement = keys->keyElements.at(itr);
             auto keyString = keyElement->expression->toString();
-            if (auto anno = keyElement->getAnnotation("name"_cs)) {
-                keyString = anno->expr.at(0)->to<IR::StringLiteral>()->value;
+            if (auto anno = keyElement->getAnnotation(IR::Annotation::nameAnnotation)) {
+                keyString = anno->getExpr(0)->to<IR::StringLiteral>()->value;
             }
             auto keySetElement = keyset->components.at(itr);
             auto key = keySetElement->toString();
@@ -714,7 +714,7 @@ std::pair<cstring, cstring> *ConvertToBackendIR::GetAnnotatedAccessPath(
     const IR::Annotation *anno) {
     cstring control_path, data_path;
     if (anno) {
-        auto expr = anno->expr[0];
+        auto expr = anno->getExpr(0);
         if (auto typeLiteral = expr->to<IR::StringLiteral>()) {
             auto permisson_str = typeLiteral->value;
             auto char_pos = permisson_str.find(":");
@@ -765,7 +765,7 @@ void ConvertToBackendIR::postorder(const IR::P4Table *t) {
             if (anno->name == ParseTCAnnotations::tc_acl) {
                 tablePermissions.emplace(t->name.originalName, GetAnnotatedAccessPath(anno));
             } else if (anno->name == ParseTCAnnotations::numMask) {
-                auto expr = anno->expr[0];
+                auto expr = anno->getExpr(0);
                 if (auto val = expr->to<IR::Constant>()) {
                     tableDefinition->setNumMask(val->asUint64());
                 } else {
@@ -1007,7 +1007,7 @@ safe_vector<const IR::TCKey *> ConvertToBackendIR::HandleTypeNameStructField(
                 for (auto f : param_struct->fields) {
                     cstring ptype = absl::StrCat("bit", f->type->width_bits());
                     if (auto anno = f->getAnnotation(ParseTCAnnotations::tcType)) {
-                        auto expr = anno->expr[0];
+                        auto expr = anno->getExpr(0);
                         if (auto typeLiteral = expr->to<IR::StringLiteral>()) {
                             ptype = typeLiteral->value;
                         }
@@ -1019,7 +1019,7 @@ safe_vector<const IR::TCKey *> ConvertToBackendIR::HandleTypeNameStructField(
             } else {
                 cstring ptype = absl::StrCat("bit", param_val->width_bits());
                 if (auto anno = field->getAnnotation(ParseTCAnnotations::tcType)) {
-                    auto expr = anno->expr[0];
+                    auto expr = anno->getExpr(0);
                     if (auto typeLiteral = expr->to<IR::StringLiteral>()) {
                         ptype = typeLiteral->value;
                     }