Add abseil string helpers (#4971)

* Add abseil formatters for: - cstring - IR::Node - IR::ID This improves the code itself as we no longer need to do explicit .toString() / .string_view() calls. Also, this prints directly to sink, providing some performance improvements. Signed-off-by: Anton Korobeynikov <anton@korobeynikov.info> * Ensure that conversion from cstring to char* is explicit Signed-off-by: Anton Korobeynikov <anton@korobeynikov.info> * Update control-plane as now conversion cstring => string_view is not ambiguous Signed-off-by: Anton Korobeynikov <anton@korobeynikov.info> * Remove more instances of `.string_view()` calls Signed-off-by: Anton Korobeynikov <anton@korobeynikov.info> * Fix some obvious Tofino cstring misuse Signed-off-by: Anton Korobeynikov <anton@korobeynikov.info> * Clarify abseil formatter helpers Signed-off-by: Anton Korobeynikov <anton@korobeynikov.info> * Review suggestions Signed-off-by: Anton Korobeynikov <anton@korobeynikov.info> --------- Signed-off-by: Anton Korobeynikov <anton@korobeynikov.info>
p4lang · Nov 6, 2024 · 446911e · 446911e
1 parent aa9bb95
commit 446911e
Show file tree

Hide file tree

Showing 88 changed files with 670 additions and 691 deletions.
diff --git a/backends/bmv2/common/JsonObjects.cpp b/backends/bmv2/common/JsonObjects.cpp
@@ -109,7 +109,7 @@ void JsonObjects::add_meta_info() {
 /// @param fields a JsonArray for the fields in the header
 unsigned JsonObjects::add_header_type(const cstring &name, Util::JsonArray *&fields,
                                       unsigned max_length) {
-    std::string sname(name, name.size());
+    std::string sname = name.string();
     auto header_type_id_it = header_type_id.find(sname);
     if (header_type_id_it != header_type_id.end()) {
         return header_type_id_it->second;
@@ -131,7 +131,7 @@ unsigned JsonObjects::add_header_type(const cstring &name, Util::JsonArray *&fie
 }
 
 unsigned JsonObjects::add_union_type(const cstring &name, Util::JsonArray *&fields) {
-    std::string sname(name, name.size());
+    std::string sname = name.string();
     auto it = union_type_id.find(sname);
     if (it != union_type_id.end()) return it->second;
     auto union_type = new Util::JsonObject();
@@ -150,7 +150,7 @@ unsigned JsonObjects::add_union_type(const cstring &name, Util::JsonArray *&fiel
 }
 
 unsigned JsonObjects::add_header_type(const cstring &name) {
-    std::string sname(name, name.size());
+    std::string sname = name.string();
     auto header_type_id_it = header_type_id.find(sname);
     if (header_type_id_it != header_type_id.end()) {
         return header_type_id_it->second;

diff --git a/backends/bmv2/common/annotations.h b/backends/bmv2/common/annotations.h
@@ -28,7 +28,7 @@ using namespace P4::literals;
 class ParseAnnotations : public P4::ParseAnnotations {
  public:
     ParseAnnotations()
-        : P4::ParseAnnotations("BMV2"_cs, false,
+        : P4::ParseAnnotations("BMV2", false,
                                {
                                    PARSE_EMPTY("metadata"_cs),
                                    PARSE_EXPRESSION_LIST("field_list"_cs),

diff --git a/backends/bmv2/common/header.cpp b/backends/bmv2/common/header.cpp
@@ -290,7 +290,7 @@ void HeaderConverter::addHeaderType(const IR::Type_StructLike *st) {
         if (auto aliasAnnotation = f->getAnnotation("alias"_cs)) {
             auto container = new Util::JsonArray();
             auto alias = new Util::JsonArray();
-            auto target_name = "";
+            cstring target_name;
             if (BMV2Context::get().options().loadIRFromJson == false) {
                 target_name = aliasAnnotation->expr.front()->to<IR::StringLiteral>()->value;
             } else {

diff --git a/backends/bmv2/common/midend.h b/backends/bmv2/common/midend.h
@@ -33,7 +33,7 @@ class EnumOn32Bits : public P4::ChooseEnumRepresentation {
     bool convert(const IR::Type_Enum *type) const override {
         if (type->srcInfo.isValid()) {
             auto sourceFile = type->srcInfo.getSourceFile();
-            if (sourceFile.endsWith(filename.string_view()))
+            if (sourceFile.endsWith(filename))
                 // Don't convert any of the standard enums
                 return false;
         }

diff --git a/backends/bmv2/pna_nic/midend.cpp b/backends/bmv2/pna_nic/midend.cpp
@@ -78,7 +78,7 @@ class PnaEnumOn32Bits : public P4::ChooseEnumRepresentation {
         if (type->name == "PNA_MeterColor_t") return true;
         if (type->srcInfo.isValid()) {
             auto sourceFile = type->srcInfo.getSourceFile();
-            if (sourceFile.endsWith(filename.string_view()))
+            if (sourceFile.endsWith(filename))
                 // Don't convert any of the standard enums
                 return false;
         }

diff --git a/backends/bmv2/pna_nic/pnaProgramStructure.h b/backends/bmv2/pna_nic/pnaProgramStructure.h
@@ -39,14 +39,13 @@ class PnaProgramStructure : public P4::PortableProgramStructure {
 
     /// Checks if a string is of type PNA_CounterType_t returns true
     /// if it is, false otherwise.
-    static bool isCounterMetadata(cstring ptName) { return !strcmp(ptName, "PNA_CounterType_t"); }
+    static bool isCounterMetadata(cstring ptName) { return ptName == "PNA_CounterType_t"; }
 
     /// Checks if a string is a pna metadata returns true
     /// if it is, false otherwise.
     static bool isStandardMetadata(cstring ptName) {
-        return (!strcmp(ptName, "pna_main_parser_input_metadata_t") ||
-                !strcmp(ptName, "pna_main_input_metadata_t") ||
-                !strcmp(ptName, "pna_main_output_metadata_t"));
+        return ptName == "pna_main_parser_input_metadata_t" ||
+               ptName == "pna_main_input_metadata_t" || ptName == "pna_main_output_metadata_t";
     }
 };
 

diff --git a/backends/bmv2/psa_switch/midend.cpp b/backends/bmv2/psa_switch/midend.cpp
@@ -78,7 +78,7 @@ class PsaEnumOn32Bits : public P4::ChooseEnumRepresentation {
         if (type->name == "PSA_MeterColor_t") return true;
         if (type->srcInfo.isValid()) {
             auto sourceFile = type->srcInfo.getSourceFile();
-            if (sourceFile.endsWith(filename.string_view()))
+            if (sourceFile.endsWith(filename))
                 // Don't convert any of the standard enums
                 return false;
         }

diff --git a/backends/common/psaProgramStructure.h b/backends/common/psaProgramStructure.h
@@ -44,18 +44,18 @@ class PsaProgramStructure : public PortableProgramStructure {
 
     /// Checks if a string is of type PSA_CounterType_t returns true
     /// if it is, false otherwise.
-    static bool isCounterMetadata(cstring ptName) { return !strcmp(ptName, "PSA_CounterType_t"); }
+    static bool isCounterMetadata(cstring ptName) { return ptName == "PSA_CounterType_t"; }
 
     /// Checks if a string is a psa metadata returns true
     /// if it is, false otherwise.
     static bool isStandardMetadata(cstring ptName) {
-        return (!strcmp(ptName, "psa_ingress_parser_input_metadata_t") ||
-                !strcmp(ptName, "psa_egress_parser_input_metadata_t") ||
-                !strcmp(ptName, "psa_ingress_input_metadata_t") ||
-                !strcmp(ptName, "psa_ingress_output_metadata_t") ||
-                !strcmp(ptName, "psa_egress_input_metadata_t") ||
-                !strcmp(ptName, "psa_egress_deparser_input_metadata_t") ||
-                !strcmp(ptName, "psa_egress_output_metadata_t"));
+        return (ptName == "psa_ingress_parser_input_metadata_t" ||
+                ptName == "psa_egress_parser_input_metadata_t" ||
+                ptName == "psa_ingress_input_metadata_t" ||
+                ptName == "psa_ingress_output_metadata_t" ||
+                ptName == "psa_egress_input_metadata_t" ||
+                ptName == "psa_egress_deparser_input_metadata_t" ||
+                ptName == "psa_egress_output_metadata_t");
     }
 };
 

diff --git a/backends/dpdk/dpdkAsmOpt.cpp b/backends/dpdk/dpdkAsmOpt.cpp
@@ -425,7 +425,7 @@ void EmitDpdkTableConfig::addAction(const IR::Expression *actionRef, P4::Referen
         actionName = newNameMap[actionDecl->name.name];
     else
         actionName = actionDecl->name.name;
-    print(actionName.string_view(), " ");
+    print(actionName, " ");
     if (actionDecl->parameters->parameters.size() == 1) {
         std::vector<cstring> paramNames;
         std::vector<big_int> argVals;
@@ -458,7 +458,7 @@ void EmitDpdkTableConfig::addAction(const IR::Expression *actionRef, P4::Referen
         }
 
         for (size_t i = 0; i < argVals.size(); i++) {
-            print(paramNames[i].string_view(), " ");
+            print(paramNames[i], " ");
             print(argVals[i], " ");
         }
     }

diff --git a/backends/dpdk/dpdkHelpers.cpp b/backends/dpdk/dpdkHelpers.cpp
@@ -1530,9 +1530,9 @@ void ConvertStatementToDpdk::add128bitwiseInstr(const IR::Expression *src1Op,
     const IR::Type_Header *Type_Header = nullptr;
     const IR::Type_Header *Type_Tmp = nullptr;
     for (auto header : structure->header_types) {
-        if (strcmp(header.first, "_p4c_sandbox_header_t") == 0) {
+        if (header.first == "_p4c_sandbox_header_t") {
             Type_Header = header.second;
-        } else if (strcmp(header.first, "_p4c_tmp128_t") == 0) {
+        } else if (header.first == "_p4c_tmp128_t") {
             Type_Tmp = header.second;
         }
     }
@@ -1637,7 +1637,7 @@ void ConvertStatementToDpdk::add128ComparisonInstr(cstring true_label, const IR:
     }
     const IR::Type_Header *Type_Header = nullptr;
     for (auto header : structure->header_types) {
-        if (strcmp(header.first, "_p4c_sandbox_header_t") == 0) {
+        if (header.first == "_p4c_sandbox_header_t") {
             Type_Header = header.second;
         }
     }
@@ -1700,9 +1700,9 @@ void ConvertStatementToDpdk::add128bitComplInstr(const IR::Expression *dst,
     const IR::Type_Header *Type_Header = nullptr;
     const IR::Type_Header *Type_Tmp = nullptr;
     for (auto header : structure->header_types) {
-        if (strcmp(header.first, "_p4c_sandbox_header_t") == 0) {
+        if (header.first == "_p4c_sandbox_header_t") {
             Type_Header = header.second;
-        } else if (strcmp(header.first, "_p4c_tmp128_t") == 0) {
+        } else if (header.first == "_p4c_tmp128_t") {
             Type_Tmp = header.second;
         }
     }

diff --git a/backends/dpdk/dpdkProgramStructure.cpp b/backends/dpdk/dpdkProgramStructure.cpp
@@ -242,14 +242,12 @@ void InspectDpdkProgram::addTypesAndInstances(const IR::Type_StructLike *type, b
 }
 
 bool InspectDpdkProgram::isStandardMetadata(cstring ptName) {
-    // FIXME: do we really need strcmp here?
-    return (!strcmp(ptName, "psa_ingress_parser_input_metadata_t") ||
-            !strcmp(ptName, "psa_egress_parser_input_metadata_t") ||
-            !strcmp(ptName, "psa_ingress_input_metadata_t") ||
-            !strcmp(ptName, "psa_ingress_output_metadata_t") ||
-            !strcmp(ptName, "psa_egress_input_metadata_t") ||
-            !strcmp(ptName, "psa_egress_deparser_input_metadata_t") ||
-            !strcmp(ptName, "psa_egress_output_metadata_t"));
+    return (ptName == "psa_ingress_parser_input_metadata_t" ||
+            ptName == "psa_egress_parser_input_metadata_t") ||
+           ptName == "psa_ingress_input_metadata_t" || ptName == "psa_ingress_output_metadata_t" ||
+           ptName == "psa_egress_input_metadata_t" ||
+           ptName == "psa_egress_deparser_input_metadata_t" ||
+           ptName == "psa_egress_output_metadata_t";
 }
 
 bool InspectDpdkProgram::preorder(const IR::Declaration_Variable *dv) {

diff --git a/backends/ebpf/codeGen.cpp b/backends/ebpf/codeGen.cpp
@@ -39,14 +39,14 @@ bool CodeGenInspector::preorder(const IR::Constant *expression) {
     cstring str = EBPFInitializerUtils::genHexStr(expression->value, width, expression);
     builder->append("{ ");
     for (size_t i = 0; i < str.size() / 2; ++i)
-        builder->appendFormat("0x%s, ", str.substr(2 * i, 2));
+        builder->appendFormat("0x%v, ", str.substr(2 * i, 2));
     builder->append("}");
 
     return false;
 }
 
 bool CodeGenInspector::preorder(const IR::StringLiteral *expression) {
-    builder->appendFormat("\"%s\"", expression->toString());
+    builder->appendFormat("\"%v\"", expression->toString());
     return true;
 }
 
@@ -491,7 +491,7 @@ void CodeGenInspector::emitAndConvertByteOrder(const IR::Expression *expr, cstri
         loadSize = 64;
     }
     unsigned shift = loadSize - widthToEmit;
-    builder->appendFormat("%s(", emit);
+    builder->appendFormat("%v(", emit);
     visit(expr);
     if (shift != 0 && byte_order == "HOST") builder->appendFormat(" << %d", shift);
     builder->append(")");

diff --git a/backends/ebpf/ebpfControl.cpp b/backends/ebpf/ebpfControl.cpp
@@ -142,8 +142,7 @@ bool ControlBodyTranslator::preorder(const IR::MethodCallExpression *expression)
         // Action arguments have been eliminated by the mid-end.
         BUG_CHECK(expression->arguments->size() == 0, "%1%: unexpected arguments for action call",
                   expression);
-        cstring msg =
-            absl::StrFormat("Control: explicit calling action %s()", ac->action->name.name);
+        cstring msg = absl::StrFormat("Control: explicit calling action %v()", ac->action->name);
         builder->target->emitTraceMessage(builder, msg.c_str());
         visit(ac->action->body);
         return false;
@@ -165,9 +164,9 @@ void ControlBodyTranslator::compileEmitField(const IR::Expression *expr, cstring
     if (!swap.isNullOrEmpty()) {
         builder->emitIndent();
         visit(expr);
-        builder->appendFormat(".%s = %s(", field.c_str(), swap);
+        builder->appendFormat(".%v = %v(", field, swap);
         visit(expr);
-        builder->appendFormat(".%s)", field.c_str());
+        builder->appendFormat(".%v)", field);
         builder->endOfStatement(true);
     }
 
@@ -304,7 +303,7 @@ void ControlBodyTranslator::processApply(const P4::ApplyMethod *method) {
     auto table = control->getTable(method->object->getName().name);
     BUG_CHECK(table != nullptr, "No table for %1%", method->expr);
 
-    msgStr = absl::StrFormat("Control: applying %s", method->object->getName().name);
+    msgStr = absl::StrFormat("Control: applying %v", method->object->getName());
     builder->target->emitTraceMessage(builder, msgStr.c_str());
 
     builder->emitIndent();
@@ -393,7 +392,7 @@ void ControlBodyTranslator::processApply(const P4::ApplyMethod *method) {
     builder->blockEnd(true);
     builder->blockEnd(true);
 
-    msgStr = absl::StrFormat("Control: %s applied", method->object->getName().name);
+    msgStr = absl::StrFormat("Control: %v applied", method->object->getName());
     builder->target->emitTraceMessage(builder, msgStr.c_str());
 }
 
@@ -595,8 +594,7 @@ void EBPFControl::emitDeclaration(CodeBuilder *builder, const IR::Declaration *d
                     // Therefore, this piece of code zero-initialize structures
                     // that might be used as keys.
                     builder->emitIndent();
-                    builder->appendFormat("__builtin_memset((void *) &%s, 0, sizeof(",
-                                          vd->name.name);
+                    builder->appendFormat("__builtin_memset((void *) &%v, 0, sizeof(", vd->name);
                     etype->declare(builder, cstring::empty, false);
                     builder->append("))");
                     builder->endOfStatement(true);

diff --git a/backends/ebpf/ebpfDeparser.cpp b/backends/ebpf/ebpfDeparser.cpp
@@ -201,13 +201,13 @@ void DeparserHdrEmitTranslator::emitField(CodeBuilder *builder, cstring field,
             visit(hdrExpr);
             builder->appendFormat(".%s", field.c_str());
             builder->endOfStatement(true);
-            msgStr = absl::StrFormat("Deparser: emitting field %s=0x%%llx (%u bits)", field,
+            msgStr = absl::StrFormat("Deparser: emitting field %v=0x%%llx (%u bits)", field,
                                      widthToEmit);
             builder->target->emitTraceMessage(builder, msgStr.c_str(), 1, "tmp");
             builder->blockEnd(true);
         }
     } else {
-        msgStr = absl::StrFormat("Deparser: emitting field %s (%u bits)", field, widthToEmit);
+        msgStr = absl::StrFormat("Deparser: emitting field %v (%u bits)", field, widthToEmit);
         builder->target->emitTraceMessage(builder, msgStr.c_str());
     }
 
@@ -229,9 +229,9 @@ void DeparserHdrEmitTranslator::emitField(CodeBuilder *builder, cstring field,
     if (!swap.isNullOrEmpty()) {
         builder->emitIndent();
         visit(hdrExpr);
-        builder->appendFormat(".%s = %s(", field, swap);
+        builder->appendFormat(".%v = %v(", field, swap);
         visit(hdrExpr);
-        builder->appendFormat(".%s", field);
+        builder->appendFormat(".%v", field);
         if (shift != 0) builder->appendFormat(" << %d", shift);
         builder->append(")");
         builder->endOfStatement(true);
@@ -244,7 +244,7 @@ void DeparserHdrEmitTranslator::emitField(CodeBuilder *builder, cstring field,
         builder->emitIndent();
         builder->appendFormat("%s = ((char*)(&", program->byteVar.c_str());
         visit(hdrExpr);
-        builder->appendFormat(".%s))[%d]", field, i);
+        builder->appendFormat(".%v))[%d]", field, i);
         builder->endOfStatement(true);
         unsigned freeBits = alignment != 0 ? (8 - alignment) : 8;
         bitsInCurrentByte = left >= 8 ? 8 : left;
@@ -371,14 +371,14 @@ void EBPFDeparser::emit(CodeBuilder *builder) {
     emitBufferAdjusts(builder);
 
     builder->emitIndent();
-    builder->appendFormat("%s = %s;", program->packetStartVar,
+    builder->appendFormat("%v = %v;", program->packetStartVar,
                           builder->target->dataOffset(program->model.CPacketName.toString()));
     builder->newline();
     builder->emitIndent();
-    builder->appendFormat("%s = %s;", program->headerStartVar, program->packetStartVar);
+    builder->appendFormat("%v = %v;", program->headerStartVar, program->packetStartVar);
     builder->newline();
     builder->emitIndent();
-    builder->appendFormat("%s = %s;", program->packetEndVar,
+    builder->appendFormat("%v = %v;", program->packetEndVar,
                           builder->target->dataEnd(program->model.CPacketName.toString()));
     builder->newline();