[ir] Convert VarForDynamicIndex to a free function
Add a `ValidateAndDumpIfNeeded` helper that validates the incoming
module in debug mode, and dumps the output module if a build-time flag
is set.
Bug: tint:1718
Change-Id: I33e1090ed90026743f88cea3402a862c52ee3730
Reviewed-on: https://dawn-review.googlesource.com/c/dawn/+/143086
Auto-Submit: James Price <jrprice@google.com>
Kokoro: Kokoro <noreply+kokoro@google.com>
Commit-Queue: James Price <jrprice@google.com>
Reviewed-by: Ben Clayton <bclayton@google.com>
diff --git a/src/tint/lang/core/ir/transform/helper_test.h b/src/tint/lang/core/ir/transform/helper_test.h
index 91a03df..e055afb 100644
--- a/src/tint/lang/core/ir/transform/helper_test.h
+++ b/src/tint/lang/core/ir/transform/helper_test.h
@@ -52,6 +52,21 @@
EXPECT_TRUE(res) << res.Failure().str();
}
+ /// Transforms the module, using @p transform.
+ /// @param transform_func the transform to run
+ void Run(std::function<Result<SuccessType, std::string>(Module*)> transform_func) {
+ // Run the transform.
+ auto result = transform_func(&mod);
+ EXPECT_TRUE(result) << result.Failure();
+ if (!result) {
+ return;
+ }
+
+ // Validate the output IR.
+ auto valid = ir::Validate(mod);
+ EXPECT_TRUE(valid) << valid.Failure().str();
+ }
+
/// @returns the transformed module as a disassembled string
std::string str() {
ir::Disassembler dis(mod);
diff --git a/src/tint/lang/core/ir/transform/var_for_dynamic_index.cc b/src/tint/lang/core/ir/transform/var_for_dynamic_index.cc
index f8390dd..773d02a 100644
--- a/src/tint/lang/core/ir/transform/var_for_dynamic_index.cc
+++ b/src/tint/lang/core/ir/transform/var_for_dynamic_index.cc
@@ -18,14 +18,13 @@
#include "src/tint/lang/core/ir/builder.h"
#include "src/tint/lang/core/ir/module.h"
+#include "src/tint/lang/core/ir/validator.h"
#include "src/tint/lang/core/type/array.h"
#include "src/tint/lang/core/type/matrix.h"
#include "src/tint/lang/core/type/pointer.h"
#include "src/tint/lang/core/type/vector.h"
#include "src/tint/utils/containers/hashmap.h"
-TINT_INSTANTIATE_TYPEINFO(tint::ir::transform::VarForDynamicIndex);
-
using namespace tint::number_suffixes; // NOLINT
namespace tint::ir::transform {
@@ -110,13 +109,7 @@
return result;
}
-} // namespace
-
-VarForDynamicIndex::VarForDynamicIndex() = default;
-
-VarForDynamicIndex::~VarForDynamicIndex() = default;
-
-void VarForDynamicIndex::Run(ir::Module* ir) const {
+void Run(ir::Module* ir) {
ir::Builder builder(*ir);
// Find the access instructions that need replacing.
@@ -193,4 +186,17 @@
}
}
+} // namespace
+
+Result<SuccessType, std::string> VarForDynamicIndex(Module* ir) {
+ auto result = ValidateAndDumpIfNeeded(*ir, "VarForDynamicIndex transform");
+ if (!result) {
+ return result;
+ }
+
+ Run(ir);
+
+ return Success;
+}
+
} // namespace tint::ir::transform
diff --git a/src/tint/lang/core/ir/transform/var_for_dynamic_index.h b/src/tint/lang/core/ir/transform/var_for_dynamic_index.h
index 03837e3..dcd0556 100644
--- a/src/tint/lang/core/ir/transform/var_for_dynamic_index.h
+++ b/src/tint/lang/core/ir/transform/var_for_dynamic_index.h
@@ -15,7 +15,14 @@
#ifndef SRC_TINT_LANG_CORE_IR_TRANSFORM_VAR_FOR_DYNAMIC_INDEX_H_
#define SRC_TINT_LANG_CORE_IR_TRANSFORM_VAR_FOR_DYNAMIC_INDEX_H_
-#include "src/tint/lang/core/ir/transform/transform.h"
+#include <string>
+
+#include "src/tint/utils/result/result.h"
+
+// Forward declarations.
+namespace tint::ir {
+class Module;
+}
namespace tint::ir::transform {
@@ -23,16 +30,9 @@
/// indexed to a temporary local `var` before performing the index. This transform is used by the
/// SPIR-V writer as there is no SPIR-V instruction that can dynamically index a non-pointer
/// composite.
-class VarForDynamicIndex final : public Castable<VarForDynamicIndex, Transform> {
- public:
- /// Constructor
- VarForDynamicIndex();
- /// Destructor
- ~VarForDynamicIndex() override;
-
- /// @copydoc Transform::Run
- void Run(ir::Module* module) const override;
-};
+/// @param module the module to transform
+/// @returns an error string on failure
+Result<SuccessType, std::string> VarForDynamicIndex(Module* module);
} // namespace tint::ir::transform
diff --git a/src/tint/lang/core/ir/transform/var_for_dynamic_index_test.cc b/src/tint/lang/core/ir/transform/var_for_dynamic_index_test.cc
index 6e1ee7a..3acce56 100644
--- a/src/tint/lang/core/ir/transform/var_for_dynamic_index_test.cc
+++ b/src/tint/lang/core/ir/transform/var_for_dynamic_index_test.cc
@@ -47,7 +47,7 @@
}
)";
- Run<VarForDynamicIndex>();
+ Run(VarForDynamicIndex);
EXPECT_EQ(expect, str());
}
@@ -70,7 +70,7 @@
}
)";
- Run<VarForDynamicIndex>();
+ Run(VarForDynamicIndex);
EXPECT_EQ(expect, str());
}
@@ -96,7 +96,7 @@
}
)";
- Run<VarForDynamicIndex>();
+ Run(VarForDynamicIndex);
EXPECT_EQ(expect, str());
}
@@ -122,7 +122,7 @@
}
)";
- Run<VarForDynamicIndex>();
+ Run(VarForDynamicIndex);
EXPECT_EQ(expect, str());
}
@@ -146,7 +146,7 @@
}
)";
- Run<VarForDynamicIndex>();
+ Run(VarForDynamicIndex);
EXPECT_EQ(expect, str());
}
@@ -172,7 +172,7 @@
}
)";
- Run<VarForDynamicIndex>();
+ Run(VarForDynamicIndex);
EXPECT_EQ(expect, str());
}
@@ -198,7 +198,7 @@
}
)";
- Run<VarForDynamicIndex>();
+ Run(VarForDynamicIndex);
EXPECT_EQ(expect, str());
}
@@ -224,7 +224,7 @@
}
)";
- Run<VarForDynamicIndex>();
+ Run(VarForDynamicIndex);
EXPECT_EQ(expect, str());
}
@@ -250,7 +250,7 @@
}
)";
- Run<VarForDynamicIndex>();
+ Run(VarForDynamicIndex);
EXPECT_EQ(expect, str());
}
@@ -277,7 +277,7 @@
}
)";
- Run<VarForDynamicIndex>();
+ Run(VarForDynamicIndex);
EXPECT_EQ(expect, str());
}
@@ -304,7 +304,7 @@
}
)";
- Run<VarForDynamicIndex>();
+ Run(VarForDynamicIndex);
EXPECT_EQ(expect, str());
}
@@ -343,7 +343,7 @@
}
)";
- Run<VarForDynamicIndex>();
+ Run(VarForDynamicIndex);
EXPECT_EQ(expect, str());
}
@@ -377,7 +377,7 @@
}
)";
- Run<VarForDynamicIndex>();
+ Run(VarForDynamicIndex);
EXPECT_EQ(expect, str());
}
@@ -412,7 +412,7 @@
}
)";
- Run<VarForDynamicIndex>();
+ Run(VarForDynamicIndex);
EXPECT_EQ(expect, str());
}
diff --git a/src/tint/lang/core/ir/validator.cc b/src/tint/lang/core/ir/validator.cc
index af87a08..ca5eb5a 100644
--- a/src/tint/lang/core/ir/validator.cc
+++ b/src/tint/lang/core/ir/validator.cc
@@ -58,6 +58,12 @@
#include "src/tint/utils/macros/scoped_assignment.h"
#include "src/tint/utils/rtti/switch.h"
+/// If set to 1 then the Tint will dump the IR when validating.
+#define TINT_DUMP_IR_WHEN_VALIDATING 0
+#if TINT_DUMP_IR_WHEN_VALIDATING
+#include <iostream>
+#endif
+
namespace tint::ir {
Validator::Validator(Module& mod) : mod_(mod) {}
@@ -626,4 +632,27 @@
return v.IsValid();
}
+Result<SuccessType, std::string> ValidateAndDumpIfNeeded([[maybe_unused]] Module& ir,
+ [[maybe_unused]] const char* msg) {
+#ifndef NDEBUG
+ auto result = Validate(ir);
+ if (!result) {
+ diag::List errors;
+ StringStream ss;
+ ss << "validating input to " << msg << " failed" << std::endl << result.Failure().str();
+ return ss.str();
+ }
+#endif
+
+#if TINT_DUMP_IR_WHEN_VALIDATING
+ Disassembler disasm(ir);
+ std::cout << "=========================================================" << std::endl;
+ std::cout << "== IR dump before " << msg << ":" << std::endl;
+ std::cout << "=========================================================" << std::endl;
+ std::cout << disasm.Disassemble();
+#endif
+
+ return Success;
+}
+
} // namespace tint::ir
diff --git a/src/tint/lang/core/ir/validator.h b/src/tint/lang/core/ir/validator.h
index 80af5e9..c409985 100644
--- a/src/tint/lang/core/ir/validator.h
+++ b/src/tint/lang/core/ir/validator.h
@@ -42,6 +42,12 @@
/// @returns true on success, an error result otherwise
Result<SuccessType, diag::List> Validate(Module& mod);
+/// Validates the module @p ir and dumps its contents if required by the build configuration.
+/// @param ir the module to transform
+/// @param msg the msg to accompany the output
+/// @returns an error string if the module is not valid
+Result<SuccessType, std::string> ValidateAndDumpIfNeeded(Module& ir, const char* msg);
+
/// The core IR validator.
class Validator {
public:
diff --git a/src/tint/lang/spirv/writer/printer/printer.cc b/src/tint/lang/spirv/writer/printer/printer.cc
index ea54789..d027689 100644
--- a/src/tint/lang/spirv/writer/printer/printer.cc
+++ b/src/tint/lang/spirv/writer/printer/printer.cc
@@ -89,7 +89,15 @@
constexpr uint32_t kWriterVersion = 1;
-void Sanitize(ir::Module* module) {
+Result<SuccessType, std::string> Sanitize(ir::Module* module) {
+#define RUN_TRANSFORM(name) \
+ do { \
+ auto result = ir::transform::name(module); \
+ if (!result) { \
+ return result; \
+ } \
+ } while (false)
+
ir::transform::AddEmptyEntryPoint{}.Run(module);
ir::transform::BlockDecoratedStructs{}.Run(module);
ir::transform::BuiltinPolyfillSpirv{}.Run(module);
@@ -99,7 +107,10 @@
ir::transform::MergeReturn{}.Run(module);
ir::transform::ShaderIOSpirv{}.Run(module);
ir::transform::Std140{}.Run(module);
- ir::transform::VarForDynamicIndex{}.Run(module);
+
+ RUN_TRANSFORM(VarForDynamicIndex);
+
+ return Success;
}
SpvStorageClass StorageClass(builtin::AddressSpace addrspace) {
@@ -168,13 +179,16 @@
: ir_(module), zero_init_workgroup_memory_(zero_init_workgroup_mem) {}
Result<std::vector<uint32_t>, std::string> Printer::Generate() {
- auto valid = ir::Validate(*ir_);
- if (!valid) {
- return valid.Failure().str();
+ // Run the IR transformations to prepare for SPIR-V emission.
+ auto sanitize = Sanitize(ir_);
+ if (!sanitize) {
+ return std::move(sanitize.Failure());
}
- // Run the IR transformations to prepare for SPIR-V emission.
- Sanitize(ir_);
+ auto valid = ir::ValidateAndDumpIfNeeded(*ir_, "SPIR-V writer");
+ if (!valid) {
+ return std::move(valid.Failure());
+ }
// TODO(crbug.com/tint/1906): Check supported extensions.
