Skip to content

Commit

Permalink
Get rid of custom implementation of Utils::PathName (p4lang#4721)
Browse files Browse the repository at this point in the history
* Get rid of custom implementation of Utils::PathName.
  - Switch to std::filesystem::path
  - Simplify and cleanup code aroung
  - Get rid of (some) cstrings around paths

* Get rid of Util::PathName alias

* Resolve a fixme
  • Loading branch information
asl authored Jun 20, 2024
1 parent 34c93ce commit a03f846
Show file tree
Hide file tree
Showing 63 changed files with 227 additions and 558 deletions.
6 changes: 3 additions & 3 deletions backends/bmv2/common/options.h
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,7 @@ class BMV2Options : public CompilerOptions {
/// Generate externs.
bool emitExterns = false;
/// File to output to.
cstring outputFile = nullptr;
std::filesystem::path outputFile;
/// Read from json.
bool loadIRFromJson = false;

Expand All @@ -44,15 +44,15 @@ class BMV2Options : public CompilerOptions {
registerOption(
"-o", "outfile",
[this](const char *arg) {
outputFile = cstring(arg);
outputFile = arg;
return true;
},
"Write output to outfile");
registerOption(
"--fromJSON", "file",
[this](const char *arg) {
loadIRFromJson = true;
file = cstring(arg);
file = arg;
return true;
},
"Use IR representation from JsonFile dumped previously,"
Expand Down
4 changes: 2 additions & 2 deletions backends/bmv2/psa_switch/main.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -98,7 +98,7 @@ int main(int argc, char *const argv[]) {
try {
toplevel = midEnd.process(program);
if (::errorCount() > 1 || toplevel == nullptr || toplevel->getMain() == nullptr) return 1;
if (options.dumpJsonFile)
if (!options.dumpJsonFile.empty())
JSONGenerator(*openFile(options.dumpJsonFile, true), true) << program << std::endl;
} catch (const std::exception &bug) {
std::cerr << bug.what() << std::endl;
Expand All @@ -119,7 +119,7 @@ int main(int argc, char *const argv[]) {
}
if (::errorCount() > 0) return 1;

if (!options.outputFile.isNullOrEmpty()) {
if (!options.outputFile.empty()) {
std::ostream *out = openFile(options.outputFile, false);
if (out != nullptr) {
backend->serialize(*out);
Expand Down
2 changes: 1 addition & 1 deletion backends/bmv2/psa_switch/psaSwitch.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -314,7 +314,7 @@ void PsaSwitchBackend::convert(const IR::ToplevelBlock *tlb) {
}
}
program->apply(toJson);
json->add_program_info(options.file);
json->add_program_info(cstring(options.file));
json->add_meta_info();
}

Expand Down
6 changes: 3 additions & 3 deletions backends/bmv2/simple_switch/main.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@ limitations under the License.
*/

#include <cstdio>
#include <fstream>
#include <iostream>
#include <string>

Expand All @@ -27,7 +28,6 @@ limitations under the License.
#include "frontends/common/applyOptionsPragmas.h"
#include "frontends/common/parseInput.h"
#include "frontends/p4/frontend.h"
#include "fstream"
#include "ir/ir.h"
#include "ir/json_generator.h"
#include "ir/json_loader.h"
Expand Down Expand Up @@ -99,7 +99,7 @@ int main(int argc, char *const argv[]) {
try {
toplevel = midEnd.process(program);
if (::errorCount() > 1 || toplevel == nullptr || toplevel->getMain() == nullptr) return 1;
if (options.dumpJsonFile && !options.loadIRFromJson)
if (!options.dumpJsonFile.empty() && !options.loadIRFromJson)
JSONGenerator(*openFile(options.dumpJsonFile, true), true) << program << std::endl;
} catch (const std::exception &bug) {
std::cerr << bug.what() << std::endl;
Expand All @@ -120,7 +120,7 @@ int main(int argc, char *const argv[]) {
}
if (::errorCount() > 0) return 1;

if (!options.outputFile.isNullOrEmpty()) {
if (!options.outputFile.empty()) {
std::ostream *out = openFile(options.outputFile, false);
if (out != nullptr) {
backend->serialize(*out);
Expand Down
2 changes: 1 addition & 1 deletion backends/bmv2/simple_switch/simpleSwitch.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1207,7 +1207,7 @@ void SimpleSwitchBackend::convert(const IR::ToplevelBlock *tlb) {
// called (e.g. resubmit or generate_digest)
BMV2::nextId("field_lists"_cs);
BMV2::nextId("learn_lists"_cs);
json->add_program_info(options.file);
json->add_program_info(cstring(options.file));
json->add_meta_info();

// convert all enums to json
Expand Down
10 changes: 5 additions & 5 deletions backends/dpdk/backend.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -116,12 +116,12 @@ void DpdkBackend::convert(const IR::ToplevelBlock *tlb) {
new DpdkArchLast(),
new VisitFunctor([this, genContextJson] {
// Serialize context json object into user specified file
if (!options.ctxtFile.isNullOrEmpty()) {
std::ostream *out = openFile(options.ctxtFile, false);
if (out != nullptr) {
if (!options.ctxtFile.empty()) {
if (std::ostream *out = openFile(options.ctxtFile, false)) {
genContextJson->serializeContextJson(out);
}
out->flush();
out->flush();
} else
::error(ErrorType::ERR_IO, "Could not open file: %1%", options.ctxtFile);
}
}),
new ReplaceHdrMetaField(),
Expand Down
9 changes: 1 addition & 8 deletions backends/dpdk/control-plane/bfruntime_ext.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -221,14 +221,7 @@ const Util::JsonObject *BFRuntimeSchemaGenerator::genSchema() const {
auto *json = new Util::JsonObject();

if (isTDI) {
cstring progName = options.file;
auto fileName = progName.findlast('/');
// Handle the case when input file is in the current working directory.
// fileName would be null in that case, hence progName should remain unchanged.
if (fileName) progName = cstring(fileName);
auto fileext = cstring(progName.find("."));
progName = cstring(progName.replace(fileext, cstring::empty));
progName = progName.trim("/\t\n\r");
auto progName = options.file.stem();
json->emplace("program_name"_cs, progName);
json->emplace("build_date"_cs, cstring(options.getBuildDate()));
json->emplace("compile_command"_cs, cstring(options.getCompileCommand()));
Expand Down
5 changes: 2 additions & 3 deletions backends/dpdk/dpdkContext.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -23,10 +23,9 @@ limitations under the License.
namespace DPDK {

cstring DpdkContextGenerator::removePipePrefix(cstring tableName) {
if (!options.bfRtSchema.isNullOrEmpty() || !options.tdiFile.isNullOrEmpty()) {
if (!options.bfRtSchema.empty() || !options.tdiFile.empty()) {
cstring tablename = cstring(tableName.find('.'));
tablename = tablename.trim(".\t\n\r");
return tablename;
return tablename.trim(".\t\n\r");
}
return tableName;
}
Expand Down
11 changes: 2 additions & 9 deletions backends/dpdk/dpdkContext.h
Original file line number Diff line number Diff line change
Expand Up @@ -82,21 +82,14 @@ struct externAttributes {

/// Program level information for context json.
struct TopLevelCtxt {
cstring progName;
std::string progName;
cstring buildDate;
cstring compileCommand;
cstring compilerVersion;
void initTopLevelCtxt(DpdkOptions &options) {
buildDate = options.getBuildDate();
compileCommand = options.getCompileCommand();
progName = options.file;
auto fileName = progName.findlast('/');
// Handle the case when input file is in the current working directory.
// fileName would be null in that case, hence progName should remain unchanged.
if (fileName) progName = cstring(fileName);
auto fileext = progName.find(".");
progName = progName.replace(cstring(fileext), cstring::empty);
progName = progName.trim("/\t\n\r");
progName = options.file.stem();
compilerVersion = options.compilerVersion;
}
};
Expand Down
12 changes: 6 additions & 6 deletions backends/dpdk/main.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -52,7 +52,7 @@ void generateTDIBfrtJson(bool isTDI, const IR::P4Program *program, DPDK::DpdkOpt
"pna"_cs, new P4::ControlPlaneAPI::Standard::PNAArchHandlerBuilderForDPDK());
auto p4Runtime = P4::generateP4Runtime(program, options.arch);

cstring filename = isTDI ? options.tdiFile : options.bfRtSchema;
std::filesystem::path filename = isTDI ? options.tdiFile : options.bfRtSchema;
auto p4rt = new P4::BFRT::BFRuntimeSchemaGenerator(*p4Runtime.p4Info, isTDI, options);
std::ostream *out = openFile(filename, false);
if (!out) {
Expand Down Expand Up @@ -115,14 +115,14 @@ int main(int argc, char *const argv[]) {
P4::serializeP4RuntimeIfRequired(program, options);
if (::errorCount() > 0) return 1;

if (!options.tdiBuilderConf.isNullOrEmpty()) {
if (!options.tdiBuilderConf.empty()) {
DPDK::TdiBfrtConf::generate(program, options);
}

if (!options.bfRtSchema.isNullOrEmpty()) {
if (!options.bfRtSchema.empty()) {
generateTDIBfrtJson(false, program, options);
}
if (!options.tdiFile.isNullOrEmpty()) {
if (!options.tdiFile.empty()) {
generateTDIBfrtJson(true, program, options);
}

Expand All @@ -133,7 +133,7 @@ int main(int argc, char *const argv[]) {
try {
toplevel = midEnd.process(program);
if (::errorCount() > 1 || toplevel == nullptr || toplevel->getMain() == nullptr) return 1;
if (options.dumpJsonFile)
if (!options.dumpJsonFile.empty())
JSONGenerator(*openFile(options.dumpJsonFile, true), true) << program << std::endl;
} catch (const std::exception &bug) {
std::cerr << bug.what() << std::endl;
Expand All @@ -146,7 +146,7 @@ int main(int argc, char *const argv[]) {
backend->convert(toplevel);
if (::errorCount() > 0) return 1;

if (!options.outputFile.isNullOrEmpty()) {
if (!options.outputFile.empty()) {
std::ostream *out = openFile(options.outputFile, false);
if (out != nullptr) {
backend->codegen(*out);
Expand Down
22 changes: 11 additions & 11 deletions backends/dpdk/options.h
Original file line number Diff line number Diff line change
Expand Up @@ -23,15 +23,15 @@ namespace DPDK {

class DpdkOptions : public CompilerOptions {
public:
cstring bfRtSchema = cstring::empty;
std::filesystem::path bfRtSchema;
/// File to output to.
cstring outputFile = nullptr;
std::filesystem::path outputFile;
/// File to output TDI JSON to.
cstring tdiFile = cstring::empty;
std::filesystem::path tdiFile;
/// File to output context JSON to.
cstring ctxtFile = cstring::empty;
std::filesystem::path ctxtFile;
/// File to output the TDI builder configuration to.
cstring tdiBuilderConf = cstring::empty;
std::filesystem::path tdiBuilderConf;
/// Read from JSON.
bool loadIRFromJson = false;
/// Enable/disable Egress pipeline in PSA.
Expand All @@ -58,43 +58,43 @@ class DpdkOptions : public CompilerOptions {
registerOption(
"--bf-rt-schema", "file",
[this](const char *arg) {
bfRtSchema = cstring(arg);
bfRtSchema = arg;
return true;
},
"Generate and write BF-RT JSON schema to the specified file");
registerOption(
"-o", "outfile",
[this](const char *arg) {
outputFile = cstring(arg);
outputFile = arg;
return true;
},
"Write output to outfile");
registerOption(
"--tdi-builder-conf", "file",
[this](const char *arg) {
tdiBuilderConf = cstring(arg);
tdiBuilderConf = arg;
return true;
},
"Generate and write the TDI builder configuration to the specified file");
registerOption(
"--tdi", "file",
[this](const char *arg) {
tdiFile = cstring(arg);
tdiFile = arg;
return true;
},
"Generate and write TDI JSON to the specified file");
registerOption(
"--context", "file",
[this](const char *arg) {
ctxtFile = cstring(arg);
ctxtFile = arg;
return true;
},
"Generate and write context JSON to the specified file");
registerOption(
"--fromJSON", "file",
[this](const char *arg) {
loadIRFromJson = true;
file = cstring(arg);
file = arg;
return true;
},
"Use IR representation from JsonFile dumped previously,"
Expand Down
21 changes: 11 additions & 10 deletions backends/dpdk/tdiConf.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -68,33 +68,33 @@ std::optional<cstring> TdiBfrtConf::findPipeName(const IR::P4Program *prog,
}

void TdiBfrtConf::generate(const IR::P4Program *prog, DPDK::DpdkOptions &options) {
if (options.outputFile.isNullOrEmpty()) {
if (options.outputFile.empty()) {
::error(ErrorType::ERR_UNEXPECTED,
"No output file provided. Unable to generate correct TDI builder config file.");
return;
}
auto inputFile = std::filesystem::absolute(options.file.c_str());
auto outFile = std::filesystem::absolute(options.outputFile.c_str());
auto inputFile = std::filesystem::absolute(options.file);
auto outFile = std::filesystem::absolute(options.outputFile);
auto outDir = outFile.parent_path();
auto tdiFile = std::filesystem::absolute(options.tdiBuilderConf.c_str());
auto tdiFile = std::filesystem::absolute(options.tdiBuilderConf);
auto programName = inputFile.stem();

if (options.bfRtSchema.isNullOrEmpty()) {
options.bfRtSchema = cstring((outDir / programName).replace_filename("json").c_str());
if (options.bfRtSchema.empty()) {
options.bfRtSchema = (outDir / programName).replace_filename("json");
::warning(
"BF-Runtime Schema file name not provided, but is required for the TDI builder "
"configuration. Generating file %1%",
options.bfRtSchema);
}
if (options.ctxtFile.isNullOrEmpty()) {
options.ctxtFile = cstring((outDir / "context.json").c_str());
if (options.ctxtFile.empty()) {
options.ctxtFile = outDir / "context.json";
::warning(
"DPDK context file name not provided, but is required for the TDI builder "
"configuration. Generating file %1%",
options.ctxtFile);
}

auto contextFile = std::filesystem::absolute(options.ctxtFile.c_str());
auto contextFile = std::filesystem::absolute(options.ctxtFile);
auto bfRtSchema = std::filesystem::absolute(options.bfRtSchema.c_str());

auto pipeName = findPipeName(prog, options);
Expand All @@ -103,6 +103,7 @@ void TdiBfrtConf::generate(const IR::P4Program *prog, DPDK::DpdkOptions &options
}
// TODO: Ideally, this should be a template. We could use Inja, but this adds another
// dependency.
// FIXME: Just use absl::StrSubstitute
std::stringstream ss;
ss << R"""({
"chip_list": [
Expand Down Expand Up @@ -149,7 +150,7 @@ void TdiBfrtConf::generate(const IR::P4Program *prog, DPDK::DpdkOptions &options
if (out.is_open()) {
out << ss;
} else {
::error(ErrorType::ERR_IO, "Could not open file: %1%", tdiFile.c_str());
::error(ErrorType::ERR_IO, "Could not open file: %1%", tdiFile);
return;
}
out.close();
Expand Down
20 changes: 7 additions & 13 deletions backends/ebpf/ebpfBackend.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -37,19 +37,14 @@ void emitFilterModel(const EbpfOptions &options, Target *target, const IR::Tople
auto ebpfprog = new EBPFProgram(options, toplevel->getProgram(), refMap, typeMap, toplevel);
if (!ebpfprog->build()) return;

if (options.outputFile.isNullOrEmpty()) return;
if (options.outputFile.empty()) return;

cstring cfile = options.outputFile;
auto cstream = openFile(cfile, false);
auto *cstream = openFile(options.outputFile, false);
if (cstream == nullptr) return;

cstring hfile;
const char *dot = cfile.findlast('.');
if (dot == nullptr)
hfile = cfile + ".h";
else
hfile = cfile.before(dot) + ".h";
auto hstream = openFile(hfile, false);
std::filesystem::path hfile = options.outputFile;
hfile.replace_extension(".h");
auto *hstream = openFile(hfile, false);
if (hstream == nullptr) return;

ebpfprog->emitH(&h, hfile);
Expand Down Expand Up @@ -96,10 +91,9 @@ void run_ebpf_backend(const EbpfOptions &options, const IR::ToplevelBlock *tople
auto backend = new EBPF::PSASwitchBackend(options, target, refMap, typeMap);
backend->convert(toplevel);

if (options.outputFile.isNullOrEmpty()) return;
if (options.outputFile.empty()) return;

cstring cfile = options.outputFile;
auto cstream = openFile(cfile, false);
auto cstream = openFile(options.outputFile, false);
if (cstream == nullptr) return;

backend->codegen(*cstream);
Expand Down
4 changes: 2 additions & 2 deletions backends/ebpf/ebpfOptions.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@ EbpfOptions::EbpfOptions() {
registerOption(
"-o", "outfile",
[this](const char *arg) {
outputFile = cstring(arg);
outputFile = arg;
return true;
},
"Write output to outfile");
Expand All @@ -42,7 +42,7 @@ EbpfOptions::EbpfOptions() {
"--fromJSON", "file",
[this](const char *arg) {
loadIRFromJson = true;
file = cstring(arg);
file = arg;
return true;
},
"Use IR representation from JsonFile dumped previously,"
Expand Down
Loading

0 comments on commit a03f846

Please sign in to comment.