Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

[P4Testgen] Unify compiler options and tool options. Ensure options context is always initialized correctly. #4787

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
+266 −334
Open

[P4Testgen] Unify compiler options and tool options. Ensure options context is always initialized correctly. #4787

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions backends/p4tools/BUILD.bazel
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -117,11 +117,11 @@ filegroup(
srcs =
glob(
["modules/testgen/targets/%s/**/*.h" % target for target in TESTGEN_TARGETS],
exclude = ["modules/testgen/targets/%s/test/**" % target for target in TESTGEN_TARGETS],
exclude = ["modules/testgen/targets/%s/test/**/*.h" % target for target in TESTGEN_TARGETS],
) +
glob(
["modules/testgen/targets/%s/**/*.cpp" % target for target in TESTGEN_TARGETS],
exclude = ["modules/testgen/targets/%s/test/**" % target for target in TESTGEN_TARGETS],
exclude = ["modules/testgen/targets/%s/test/**/*.cpp" % target for target in TESTGEN_TARGETS],
),
)

Expand Down
20 changes: 0 additions & 20 deletions backends/p4tools/common/compiler/compiler_target.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,9 +1,7 @@
#include "backends/p4tools/common/compiler/compiler_target.h"

#include <string>
#include <vector>

#include "backends/p4tools/common/compiler/context.h"
#include "backends/p4tools/common/compiler/midend.h"
#include "backends/p4tools/common/core/target.h"
#include "frontends/common/applyOptionsPragmas.h"
Expand All @@ -16,15 +14,6 @@

namespace P4::P4Tools {

ICompileContext *CompilerTarget::makeContext(std::string_view toolName) {
return get(toolName).makeContextImpl();
}

std::vector<const char *> *CompilerTarget::initCompiler(std::string_view toolName, int argc,
char **argv) {
return get(toolName).initCompilerImpl(argc, argv);
}

CompilerResultOrError CompilerTarget::runCompiler(const CompilerOptions &options,
std::string_view toolName) {
const auto *program = P4Tools::CompilerTarget::runParser(options);
Expand Down Expand Up @@ -67,15 +56,6 @@ CompilerResultOrError CompilerTarget::runCompilerImpl(const CompilerOptions &opt
return *new CompilerResult(*program);
}

ICompileContext *CompilerTarget::makeContextImpl() const {
return new CompileContext<CompilerOptions>();
}

std::vector<const char *> *CompilerTarget::initCompilerImpl(int argc, char **argv) const {
auto *result = CompileContext<CompilerOptions>::get().options().process(argc, argv);
return ::P4::errorCount() > 0 ? nullptr : result;
}

const IR::P4Program *CompilerTarget::runParser(const ParserOptions &options) {
const auto *program = P4::parseP4File(options);
if (::P4::errorCount() > 0) {
Expand Down
18 changes: 0 additions & 18 deletions backends/p4tools/common/compiler/compiler_target.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,6 @@
#define BACKENDS_P4TOOLS_COMMON_COMPILER_COMPILER_TARGET_H_

#include <string>
#include <vector>

#include "backends/p4tools/common/compiler/compiler_result.h"
#include "backends/p4tools/common/compiler/context.h"
Expand All @@ -19,15 +18,6 @@ namespace P4::P4Tools {
/// Encapsulates the details of invoking the P4 compiler for a target device and architecture.
class CompilerTarget : public Target {
public:
/// @returns a new compilation context for the compiler.
static ICompileContext *makeContext(std::string_view toolName);

/// Initializes the P4 compiler with the given compiler-specific command-line arguments.
///
/// @returns any unprocessed arguments, or nullptr if there was an error.
static std::vector<const char *> *initCompiler(std::string_view toolName, int argc,
char **argv);

/// Runs the P4 compiler to produce an IR and various other kinds of information on the input
/// program.
///
Expand All @@ -49,18 +39,10 @@ class CompilerTarget : public Target {
std::string_view toolName, const IR::P4Program *);

protected:
/// @see @makeContext.
[[nodiscard]] virtual ICompileContext *makeContextImpl() const;

/// @see runCompiler.
virtual CompilerResultOrError runCompilerImpl(const CompilerOptions &options,
const IR::P4Program *) const;

/// This implementation just forwards the given arguments to the compiler.
///
/// @see @initCompiler.
virtual std::vector<const char *> *initCompilerImpl(int argc, char **argv) const;

/// Parses the P4 program specified on the command line.
///
/// @returns nullptr if an error occurs during parsing.
Expand Down
8 changes: 4 additions & 4 deletions backends/p4tools/common/compiler/context.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@
#define BACKENDS_P4TOOLS_COMMON_COMPILER_CONTEXT_H_

#include "backends/p4tools/common/compiler/configuration.h"
#include "frontends/common/options.h"
#include "frontends/common/parser_options.h"

namespace P4::P4Tools {

Expand All @@ -18,17 +18,17 @@ class CompileContext : public virtual P4CContext {

template <typename OptionsDerivedType>
explicit CompileContext(CompileContext<OptionsDerivedType> &context)
: optionsInstance(context.options()) {}
: _optionsInstance(context.options()) {}

/// @return the compiler options for this compilation context.
OptionsType &options() override { return optionsInstance; }
OptionsType &options() override { return _optionsInstance; }

protected:
const CompilerConfiguration &getConfigImpl() override { return CompilerConfiguration::get(); }

private:
/// The compiler options for this compilation context.
OptionsType optionsInstance;
OptionsType _optionsInstance;
};

} // namespace P4::P4Tools
Expand Down
97 changes: 84 additions & 13 deletions backends/p4tools/common/core/target.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,15 +6,14 @@
#include <map>
#include <optional>
#include <string>
#include <utility>

#include "backends/p4tools/common/lib/variables.h"
#include "ir/irutils.h"

namespace P4::P4Tools {

Target::Spec::Spec(std::string deviceName, std::string archName)
: deviceName(std::move(deviceName)), archName(std::move(archName)) {
Target::Spec::Spec(std::string_view deviceName, std::string_view archName)
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not sure about string_view here, can revert if necessary.

: deviceName(deviceName), archName(archName) {
// Convert names to lower case.
std::transform(this->archName.begin(), this->archName.end(), this->archName.begin(), ::tolower);
std::transform(this->deviceName.begin(), this->deviceName.end(), this->deviceName.begin(),
Expand All @@ -37,8 +36,8 @@ std::map<Target::Spec, std::map<std::string, const Target *, std::less<>>> Targe
std::map<std::string, std::string, std::less<>> Target::defaultArchByDevice = {};
std::map<std::string, std::string, std::less<>> Target::defaultDeviceByArch = {};

bool Target::init(std::string deviceName, std::string archName) {
Spec spec(std::move(deviceName), std::move(archName));
bool Target::init(std::string_view deviceName, std::string_view archName) {
Spec spec(deviceName, archName);

if (registry.count(spec) != 0U) {
curTarget = spec;
Expand All @@ -48,23 +47,95 @@ bool Target::init(std::string deviceName, std::string archName) {
return false;
}

bool Target::setDevice(std::string deviceName) {
std::transform(deviceName.begin(), deviceName.end(), deviceName.begin(), ::tolower);
if (defaultArchByDevice.count(deviceName) == 0U) {
std::optional<ICompileContext *> Target::initializeTarget(std::string_view toolName,
std::string_view target,
std::string_view arch) {
// Establish a dummy compilation context so that we can use ::error to report errors while
// processing target and arch.
class DummyCompileContext : public BaseCompileContext {
} dummyContext;
AutoCompileContext autoDummyContext(&dummyContext);
if (!P4Tools::Target::setDevice(target)) {
::P4::error("Unsupported device: %s", target);
return std::nullopt;
}
if (!P4Tools::Target::setArch(arch)) {
::P4::error("Unsupported architecture: %s", arch);
return std::nullopt;
}
const auto &instances = registry.at(*curTarget);
auto instance = instances.find(toolName);
BUG_CHECK(instance != instances.end(), "Architecture %1% on device %2% not supported for %3%",
curTarget->archName, curTarget->deviceName, toolName);

return instance->second->makeContext();
}

std::optional<ICompileContext *> Target::initializeTarget(std::string_view toolName,
const std::vector<const char *> &args) {
// Establish a dummy compilation context so that we can use ::error to report errors while
// processing target and arch.
class DummyCompileContext : public BaseCompileContext {
} dummyContext;
AutoCompileContext autoDummyContext(&dummyContext);
if (args.size() < 3) {
::P4::error("Missing --target and --arch arguments");
return std::nullopt;
}
std::optional<std::string> target;
std::optional<std::string> arch;
// Loop through arguments (skip the program name)
for (size_t i = 1; i < args.size(); ++i) {
const std::string &arg = args[i];
if (arg == "--arch") {
if (i + 1 < args.size()) {
arch = args[i + 1];
} else {
::P4::error("Missing architecture name after --arch");
return std::nullopt;
}
}
if (arg == "--target") {
if (i + 1 < args.size()) {
target = args[i + 1];
} else {
::P4::error("Missing device name after --target");
return std::nullopt;
}
}
}
if (!target) {
::P4::error("Missing --target argument");
return std::nullopt;
}
if (!arch) {
::P4::error("Missing --arch argument");
return std::nullopt;
}
return initializeTarget(toolName, target.value(), arch.value());
}

bool Target::setDevice(std::string_view deviceName) {
std::string lowerCaseDeviceName(deviceName);
std::transform(lowerCaseDeviceName.begin(), lowerCaseDeviceName.end(),
lowerCaseDeviceName.begin(), ::tolower);
if (defaultArchByDevice.count(lowerCaseDeviceName) == 0U) {
return false;
}

auto archName = curTarget ? curTarget->archName : defaultArchByDevice.at(deviceName);
auto archName = curTarget ? curTarget->archName : defaultArchByDevice.at(deviceName.data());
return init(deviceName, archName);
}

bool Target::setArch(std::string archName) {
std::transform(archName.begin(), archName.end(), archName.begin(), ::tolower);
if (defaultDeviceByArch.count(archName) == 0U) {
bool Target::setArch(std::string_view archName) {
std::string lowerCaseArchName(archName);
std::transform(lowerCaseArchName.begin(), lowerCaseArchName.end(), lowerCaseArchName.begin(),
::tolower);
if (defaultDeviceByArch.count(lowerCaseArchName) == 0U) {
return false;
}

auto deviceName = curTarget ? curTarget->deviceName : defaultDeviceByArch.at(archName);
auto deviceName = curTarget ? curTarget->deviceName : defaultDeviceByArch.at(archName.data());
return init(deviceName, archName);
}

Expand Down
21 changes: 17 additions & 4 deletions backends/p4tools/common/core/target.h
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,11 +1,13 @@
#ifndef BACKENDS_P4TOOLS_COMMON_CORE_TARGET_H_
#define BACKENDS_P4TOOLS_COMMON_CORE_TARGET_H_

#include <cstdlib>
#include <map>
#include <optional>
#include <string>

#include "ir/ir.h"
#include "lib/compile_context.h"
#include "lib/exceptions.h"

namespace P4::P4Tools {
Expand All @@ -22,7 +24,7 @@ class Target {
std::string archName;

/// Names provided to this constructor are converted to lower case.
Spec(std::string deviceName, std::string archName);
Spec(std::string_view deviceName, std::string_view archName);

/// Lexicographic ordering on (deviceName, archName).
bool operator<(const Spec &) const;
Expand All @@ -32,23 +34,23 @@ class Target {
///
/// @returns true on success. If initialization fails, false is returned, and nothing is
/// changed.
static bool init(std::string deviceName, std::string archName);
static bool init(std::string_view deviceName, std::string_view archName);

/// Initializes the global target device to @deviceName without changing the architecture. If
/// no architecture was previously selected, then the first architecture registered for the
/// device is chosen.
///
/// @returns true on success. If initialization fails, false is returned, and nothing is
/// changed.
static bool setDevice(std::string deviceName);
static bool setDevice(std::string_view deviceName);

/// Initializes the global target architecture to @archName without changing the device. If no
/// device was previously selected, then the first device registered for the architecture is
/// chosen.
///
/// @returns true on success. If initialization fails, false is returned, and nothing is
/// changed.
static bool setArch(std::string archName);
static bool setArch(std::string_view archName);

/// The name of the tool supported by this instance.
std::string toolName;
Expand All @@ -67,11 +69,22 @@ class Target {
virtual const IR::Expression *createTargetUninitialized(const IR::Type *type,
bool forceTaint) const;

/// Initializes the global target device and architecture to @deviceName and @archName.
/// Returns 0 on success. If initialization fails, returns -1.
static std::optional<ICompileContext *> initializeTarget(std::string_view toolName,
const std::vector<const char *> &args);
static std::optional<ICompileContext *> initializeTarget(std::string_view toolName,
std::string_view target,
std::string_view arch);

protected:
/// Creates and registers a new Target instance for the given @toolName, @deviceName, and
/// @archName.
Target(std::string_view toolName, const std::string &deviceName, const std::string &archName);

/// @returns a new compilation context for the compiler.
[[nodiscard]] virtual ICompileContext *makeContext() const = 0;

/// @returns the target instance for the given tool and active target, as selected by @init.
//
// Implemented here because of limitations of templates.
Expand Down
Loading
Loading