[tint][msl] Use tint::Result for Generate()
This is more consistent with the rest of the codebase.
Also renames `writer::Result` to `writer::Output` to avoid clashing
with tint::Result.
Change-Id: I563c03f46a9976c03e3132adfce63909262403f4
Reviewed-on: https://dawn-review.googlesource.com/c/dawn/+/144261
Commit-Queue: James Price <jrprice@google.com>
Reviewed-by: Dan Sinclair <dsinclair@chromium.org>
Auto-Submit: James Price <jrprice@google.com>
Kokoro: Kokoro <noreply+kokoro@google.com>
diff --git a/src/dawn/native/metal/ShaderModuleMTL.mm b/src/dawn/native/metal/ShaderModuleMTL.mm
index 0091d23..c74aa88 100644
--- a/src/dawn/native/metal/ShaderModuleMTL.mm
+++ b/src/dawn/native/metal/ShaderModuleMTL.mm
@@ -281,26 +281,27 @@
TRACE_EVENT0(r.platform.UnsafeGetValue(), General, "tint::msl::writer::Generate");
auto result = tint::msl::writer::Generate(&program, options);
- DAWN_INVALID_IF(!result.success, "An error occured while generating MSL: %s.",
- result.error);
+ DAWN_INVALID_IF(!result, "An error occured while generating MSL: %s.",
+ result.Failure());
// Metal uses Clang to compile the shader as C++14. Disable everything in the -Wall
// category. -Wunused-variable in particular comes up a lot in generated code, and some
// (old?) Metal drivers accidentally treat it as a MTLLibraryErrorCompileError instead
// of a warning.
- result.msl = R"(
+ auto msl = std::move(result->msl);
+ msl = R"(
#ifdef __clang__
#pragma clang diagnostic ignored "-Wall"
#endif
- )" + result.msl;
+ )" + msl;
auto workgroupAllocations =
- std::move(result.workgroup_allocations[remappedEntryPointName]);
+ std::move(result->workgroup_allocations.at(remappedEntryPointName));
return MslCompilation{{
- std::move(result.msl),
+ std::move(msl),
std::move(remappedEntryPointName),
- result.needs_storage_buffer_sizes,
- result.has_invariant_attribute,
+ result->needs_storage_buffer_sizes,
+ result->has_invariant_attribute,
std::move(workgroupAllocations),
localSize,
}};
diff --git a/src/tint/BUILD.gn b/src/tint/BUILD.gn
index 5abb8d1..e2d661dd 100644
--- a/src/tint/BUILD.gn
+++ b/src/tint/BUILD.gn
@@ -1149,8 +1149,8 @@
"lang/msl/writer/common/options.h",
"lang/msl/writer/common/printer_support.cc",
"lang/msl/writer/common/printer_support.h",
- "lang/msl/writer/result.cc",
- "lang/msl/writer/result.h",
+ "lang/msl/writer/output.cc",
+ "lang/msl/writer/output.h",
"lang/msl/writer/writer.cc",
"lang/msl/writer/writer.h",
]
diff --git a/src/tint/CMakeLists.txt b/src/tint/CMakeLists.txt
index 92b5621..31b3599 100644
--- a/src/tint/CMakeLists.txt
+++ b/src/tint/CMakeLists.txt
@@ -720,8 +720,8 @@
lang/msl/writer/common/options.h
lang/msl/writer/common/printer_support.cc
lang/msl/writer/common/printer_support.h
- lang/msl/writer/result.cc
- lang/msl/writer/result.h
+ lang/msl/writer/output.cc
+ lang/msl/writer/output.h
lang/msl/writer/writer.cc
lang/msl/writer/writer.h
lang/msl/writer/ast_printer/ast_printer.cc
diff --git a/src/tint/cmd/loopy.cc b/src/tint/cmd/loopy.cc
index 68267ed..c69926c 100644
--- a/src/tint/cmd/loopy.cc
+++ b/src/tint/cmd/loopy.cc
@@ -224,9 +224,9 @@
gen_options.array_length_from_uniform.bindpoint_to_size_index.emplace(tint::BindingPoint{0, 1},
1);
auto result = tint::msl::writer::Generate(input_program, gen_options);
- if (!result.success) {
+ if (!result) {
tint::cmd::PrintWGSL(std::cerr, *program);
- std::cerr << "Failed to generate: " << result.error << std::endl;
+ std::cerr << "Failed to generate: " << result.Failure() << std::endl;
return false;
}
diff --git a/src/tint/cmd/main.cc b/src/tint/cmd/main.cc
index 622e249..92873df 100644
--- a/src/tint/cmd/main.cc
+++ b/src/tint/cmd/main.cc
@@ -629,17 +629,17 @@
gen_options.array_length_from_uniform.bindpoint_to_size_index.emplace(tint::BindingPoint{0, 1},
1);
auto result = tint::msl::writer::Generate(input_program, gen_options);
- if (!result.success) {
+ if (!result) {
tint::cmd::PrintWGSL(std::cerr, *program);
- std::cerr << "Failed to generate: " << result.error << std::endl;
+ std::cerr << "Failed to generate: " << result.Failure() << std::endl;
return false;
}
- if (!WriteFile(options.output_file, "w", result.msl)) {
+ if (!WriteFile(options.output_file, "w", result->msl)) {
return false;
}
- const auto hash = tint::CRC32(result.msl.c_str());
+ const auto hash = tint::CRC32(result->msl.c_str());
if (options.print_hash) {
PrintHash(hash);
}
@@ -647,7 +647,7 @@
if (options.validate && options.skip_hash.count(hash) == 0) {
tint::msl::validate::Result res;
#ifdef TINT_ENABLE_MSL_VALIDATION_USING_METAL_API
- res = tint::msl::validate::UsingMetalAPI(result.msl);
+ res = tint::msl::validate::UsingMetalAPI(result->msl);
#else
#ifdef _WIN32
const char* default_xcrun_exe = "metal.exe";
@@ -657,7 +657,7 @@
auto xcrun = tint::Command::LookPath(
options.xcrun_path.empty() ? default_xcrun_exe : std::string(options.xcrun_path));
if (xcrun.Found()) {
- res = tint::msl::validate::Msl(xcrun.Path(), result.msl);
+ res = tint::msl::validate::Msl(xcrun.Path(), result->msl);
} else {
res.output = "xcrun executable not found. Cannot validate.";
res.failed = true;
diff --git a/src/tint/fuzzers/tint_common_fuzzer.cc b/src/tint/fuzzers/tint_common_fuzzer.cc
index 9979285..2e1da16 100644
--- a/src/tint/fuzzers/tint_common_fuzzer.cc
+++ b/src/tint/fuzzers/tint_common_fuzzer.cc
@@ -327,7 +327,7 @@
input_program = &*flattened;
}
- msl::writer::Generate(input_program, options_msl_);
+ (void)msl::writer::Generate(input_program, options_msl_);
#endif // TINT_BUILD_MSL_WRITER
break;
}
diff --git a/src/tint/fuzzers/tint_concurrency_fuzzer.cc b/src/tint/fuzzers/tint_concurrency_fuzzer.cc
index 85312c92..9b2d6f2 100644
--- a/src/tint/fuzzers/tint_concurrency_fuzzer.cc
+++ b/src/tint/fuzzers/tint_concurrency_fuzzer.cc
@@ -111,7 +111,7 @@
case Writer::kMSL: {
// Remap resource numbers to a flat namespace.
if (auto flattened = tint::writer::FlattenBindings(&program)) {
- tint::msl::writer::Generate(&flattened.value(), {});
+ (void)tint::msl::writer::Generate(&flattened.value(), {});
}
break;
}
diff --git a/src/tint/lang/msl/writer/ast_printer/ast_printer.cc b/src/tint/lang/msl/writer/ast_printer/ast_printer.cc
index 80155eb..1408c94 100644
--- a/src/tint/lang/msl/writer/ast_printer/ast_printer.cc
+++ b/src/tint/lang/msl/writer/ast_printer/ast_printer.cc
@@ -1875,6 +1875,7 @@
auto* func_sem = builder_.Sem().Get(func);
auto func_name = func->name->symbol.Name();
+ workgroup_allocations_.insert({func_name, {}});
// Returns the binding index of a variable, requiring that the group
// attribute have a value of zero.
diff --git a/src/tint/lang/msl/writer/ast_printer/ast_printer_test.cc b/src/tint/lang/msl/writer/ast_printer/ast_printer_test.cc
index 38e1087..c554816 100644
--- a/src/tint/lang/msl/writer/ast_printer/ast_printer_test.cc
+++ b/src/tint/lang/msl/writer/ast_printer/ast_printer_test.cc
@@ -29,7 +29,8 @@
auto program = std::make_unique<Program>(resolver::Resolve(*this));
ASSERT_FALSE(program->IsValid());
auto result = Generate(program.get(), Options{});
- EXPECT_EQ(result.error, "input program is not valid");
+ EXPECT_FALSE(result);
+ EXPECT_EQ(result.Failure(), "input program is not valid");
}
TEST_F(MslASTPrinterTest, UnsupportedExtension) {
@@ -166,8 +167,8 @@
auto allocations = gen.DynamicWorkgroupAllocations();
ASSERT_TRUE(allocations.count("comp_main"));
- ASSERT_EQ(allocations["comp_main"].size(), 1u);
- EXPECT_EQ(allocations["comp_main"][0], 2u * 2u * sizeof(float));
+ ASSERT_EQ(allocations.at("comp_main").size(), 1u);
+ EXPECT_EQ(allocations.at("comp_main")[0], 2u * 2u * sizeof(float));
}
TEST_F(MslASTPrinterTest, WorkgroupMatrixInArray) {
@@ -220,8 +221,8 @@
auto allocations = gen.DynamicWorkgroupAllocations();
ASSERT_TRUE(allocations.count("comp_main"));
- ASSERT_EQ(allocations["comp_main"].size(), 1u);
- EXPECT_EQ(allocations["comp_main"][0], 4u * 2u * 2u * sizeof(float));
+ ASSERT_EQ(allocations.at("comp_main").size(), 1u);
+ EXPECT_EQ(allocations.at("comp_main")[0], 4u * 2u * 2u * sizeof(float));
}
TEST_F(MslASTPrinterTest, WorkgroupMatrixInStruct) {
@@ -277,8 +278,8 @@
auto allocations = gen.DynamicWorkgroupAllocations();
ASSERT_TRUE(allocations.count("comp_main"));
- ASSERT_EQ(allocations["comp_main"].size(), 1u);
- EXPECT_EQ(allocations["comp_main"][0], (2 * 2 * sizeof(float)) + (4u * 4u * sizeof(float)));
+ ASSERT_EQ(allocations.at("comp_main").size(), 1u);
+ EXPECT_EQ(allocations.at("comp_main")[0], (2 * 2 * sizeof(float)) + (4u * 4u * sizeof(float)));
}
TEST_F(MslASTPrinterTest, WorkgroupMatrix_Multiples) {
@@ -421,13 +422,13 @@
ASSERT_TRUE(allocations.count("main1"));
ASSERT_TRUE(allocations.count("main2"));
ASSERT_TRUE(allocations.count("main3"));
- EXPECT_EQ(allocations.count("main4_no_usages"), 0u);
- ASSERT_EQ(allocations["main1"].size(), 1u);
- EXPECT_EQ(allocations["main1"][0], 20u * sizeof(float));
- ASSERT_EQ(allocations["main2"].size(), 1u);
- EXPECT_EQ(allocations["main2"][0], 32u * sizeof(float));
- ASSERT_EQ(allocations["main3"].size(), 1u);
- EXPECT_EQ(allocations["main3"][0], 40u * sizeof(float));
+ ASSERT_EQ(allocations.at("main1").size(), 1u);
+ EXPECT_EQ(allocations.at("main1")[0], 20u * sizeof(float));
+ ASSERT_EQ(allocations.at("main2").size(), 1u);
+ EXPECT_EQ(allocations.at("main2")[0], 32u * sizeof(float));
+ ASSERT_EQ(allocations.at("main3").size(), 1u);
+ EXPECT_EQ(allocations.at("main3")[0], 40u * sizeof(float));
+ EXPECT_EQ(allocations.at("main4_no_usages").size(), 0u);
}
} // namespace
diff --git a/src/tint/lang/msl/writer/result.cc b/src/tint/lang/msl/writer/output.cc
similarity index 82%
rename from src/tint/lang/msl/writer/result.cc
rename to src/tint/lang/msl/writer/output.cc
index 0b08133..debbc7b 100644
--- a/src/tint/lang/msl/writer/result.cc
+++ b/src/tint/lang/msl/writer/output.cc
@@ -12,14 +12,14 @@
// See the License for the specific language governing permissions and
// limitations under the License.
-#include "src/tint/lang/msl/writer/result.h"
+#include "src/tint/lang/msl/writer/output.h"
namespace tint::msl::writer {
-Result::Result() = default;
+Output::Output() = default;
-Result::~Result() = default;
+Output::~Output() = default;
-Result::Result(const Result&) = default;
+Output::Output(const Output&) = default;
} // namespace tint::msl::writer
diff --git a/src/tint/lang/msl/writer/result.h b/src/tint/lang/msl/writer/output.h
similarity index 79%
rename from src/tint/lang/msl/writer/result.h
rename to src/tint/lang/msl/writer/output.h
index 5a2968d..1c29dfc 100644
--- a/src/tint/lang/msl/writer/result.h
+++ b/src/tint/lang/msl/writer/output.h
@@ -12,8 +12,8 @@
// See the License for the specific language governing permissions and
// limitations under the License.
-#ifndef SRC_TINT_LANG_MSL_WRITER_RESULT_H_
-#define SRC_TINT_LANG_MSL_WRITER_RESULT_H_
+#ifndef SRC_TINT_LANG_MSL_WRITER_OUTPUT_H_
+#define SRC_TINT_LANG_MSL_WRITER_OUTPUT_H_
#include <string>
#include <unordered_map>
@@ -22,22 +22,16 @@
namespace tint::msl::writer {
-/// The result produced when generating MSL.
-struct Result {
+/// The output produced when generating MSL.
+struct Output {
/// Constructor
- Result();
+ Output();
/// Destructor
- ~Result();
+ ~Output();
/// Copy constructor
- Result(const Result&);
-
- /// True if generation was successful.
- bool success = false;
-
- /// The errors generated during code generation, if any.
- std::string error;
+ Output(const Output&);
/// The generated MSL.
std::string msl = "";
@@ -60,4 +54,4 @@
} // namespace tint::msl::writer
-#endif // SRC_TINT_LANG_MSL_WRITER_RESULT_H_
+#endif // SRC_TINT_LANG_MSL_WRITER_OUTPUT_H_
diff --git a/src/tint/lang/msl/writer/writer.cc b/src/tint/lang/msl/writer/writer.cc
index e4535c2..788849ff 100644
--- a/src/tint/lang/msl/writer/writer.cc
+++ b/src/tint/lang/msl/writer/writer.cc
@@ -26,52 +26,50 @@
namespace tint::msl::writer {
-Result Generate(const Program* program, const Options& options) {
- Result result;
+Result<Output, std::string> Generate(const Program* program, const Options& options) {
if (!program->IsValid()) {
- result.error = "input program is not valid";
- return result;
+ return std::string("input program is not valid");
}
+ Output output;
#if TINT_BUILD_IR
if (options.use_tint_ir) {
// Convert the AST program to an IR module.
auto converted = wgsl::reader::ProgramToIR(program);
if (!converted) {
- result.error = "IR converter: " + converted.Failure();
- return result;
+ return std::string("IR converter: " + converted.Failure());
}
// Generate the MSL code.
auto ir = converted.Move();
auto impl = std::make_unique<Printer>(&ir);
- result.success = impl->Generate();
- result.error = impl->Diagnostics().str();
- result.msl = impl->Result();
+ if (!impl->Generate()) {
+ return impl->Diagnostics().str();
+ }
+ output.msl = impl->Result();
} else // NOLINT(readability/braces)
#endif
{
// Sanitize the program.
auto sanitized_result = Sanitize(program, options);
if (!sanitized_result.program.IsValid()) {
- result.success = false;
- result.error = sanitized_result.program.Diagnostics().str();
- return result;
+ return sanitized_result.program.Diagnostics().str();
}
- result.needs_storage_buffer_sizes = sanitized_result.needs_storage_buffer_sizes;
- result.used_array_length_from_uniform_indices =
+ output.needs_storage_buffer_sizes = sanitized_result.needs_storage_buffer_sizes;
+ output.used_array_length_from_uniform_indices =
std::move(sanitized_result.used_array_length_from_uniform_indices);
// Generate the MSL code.
auto impl = std::make_unique<ASTPrinter>(&sanitized_result.program);
- result.success = impl->Generate();
- result.error = impl->Diagnostics().str();
- result.msl = impl->Result();
- result.has_invariant_attribute = impl->HasInvariant();
- result.workgroup_allocations = impl->DynamicWorkgroupAllocations();
+ if (!impl->Generate()) {
+ return impl->Diagnostics().str();
+ }
+ output.msl = impl->Result();
+ output.has_invariant_attribute = impl->HasInvariant();
+ output.workgroup_allocations = impl->DynamicWorkgroupAllocations();
}
- return result;
+ return output;
}
} // namespace tint::msl::writer
diff --git a/src/tint/lang/msl/writer/writer.h b/src/tint/lang/msl/writer/writer.h
index e11de12..1bfbc8f 100644
--- a/src/tint/lang/msl/writer/writer.h
+++ b/src/tint/lang/msl/writer/writer.h
@@ -15,8 +15,11 @@
#ifndef SRC_TINT_LANG_MSL_WRITER_WRITER_H_
#define SRC_TINT_LANG_MSL_WRITER_WRITER_H_
+#include <string>
+
#include "src/tint/lang/msl/writer/common/options.h"
-#include "src/tint/lang/msl/writer/result.h"
+#include "src/tint/lang/msl/writer/output.h"
+#include "src/tint/utils/result/result.h"
// Forward declarations
namespace tint {
@@ -25,13 +28,12 @@
namespace tint::msl::writer {
-/// Generate MSL for a program, according to a set of configuration options. The
-/// result will contain the MSL, as well as success status and diagnostic
-/// information.
+/// Generate MSL for a program, according to a set of configuration options.
+/// The result will contain the MSL and supplementary information, or an error string.
/// @param program the program to translate to MSL
/// @param options the configuration options to use when generating MSL
-/// @returns the resulting MSL and supplementary information
-Result Generate(const Program* program, const Options& options);
+/// @returns the resulting MSL and supplementary information, or an error string
+Result<Output, std::string> Generate(const Program* program, const Options& options);
} // namespace tint::msl::writer
diff --git a/src/tint/lang/msl/writer/writer_bench.cc b/src/tint/lang/msl/writer/writer_bench.cc
index 5f8e9c5..9e8e008 100644
--- a/src/tint/lang/msl/writer/writer_bench.cc
+++ b/src/tint/lang/msl/writer/writer_bench.cc
@@ -61,8 +61,8 @@
}
for (auto _ : state) {
auto res = Generate(&program, gen_options);
- if (!res.error.empty()) {
- state.SkipWithError(res.error.c_str());
+ if (!res) {
+ state.SkipWithError(res.Failure().c_str());
}
}
}
