[tint] Allow platform discard on metal >= 2.3
For Metal 2.3+ the discard semantics have changed to demote to
helper. This means we no longer need to run our own demote to helper
internal transformation.
This feature is tested by both CTS and the dawn_end2end
`DiscardBasicTest`
Platform Discard document:
https://docs.google.com/document/d/1CEnS-99jspI-ghs_wOpZ8vMGhOW80wQxxTDly87bFN4/edit?pli=1&resourcekey=0-o02HubN1NPVz1ssuEyqIPQ&tab=t.0
Metal 2.3 version:
https://developer.apple.com/documentation/metal/mtllanguageversion/mtllanguageversion2_3
Bug: 42250787, 372714384
Change-Id: Icbb56f8a08b35178283104b3ae7c2ad53c479e16
Reviewed-on: https://dawn-review.googlesource.com/c/dawn/+/216497
Commit-Queue: Peter McNeeley <petermcneeley@google.com>
Reviewed-by: James Price <jrprice@google.com>
diff --git a/src/dawn/native/Toggles.cpp b/src/dawn/native/Toggles.cpp
index f56bb66..cbd85c7 100644
--- a/src/dawn/native/Toggles.cpp
+++ b/src/dawn/native/Toggles.cpp
@@ -207,6 +207,10 @@
{"disable_workgroup_init",
"Disables the workgroup memory zero-initialization for compute shaders.",
"https://crbug.com/tint/1003", ToggleStage::Device}},
+ {Toggle::DisableDemoteToHelper,
+ {"disable_demote_to_helper",
+ "Disables the conversion of discard to demote to helper thread in the IR transform",
+ "https://crbug.com/42250787", ToggleStage::Device}},
{Toggle::VulkanUseDemoteToHelperInvocationExtension,
{"vulkan_use_demote_to_helper_invocation_extension",
"Sets the use of the vulkan demote to helper extension", "https://crbug.com/42250787",
diff --git a/src/dawn/native/Toggles.h b/src/dawn/native/Toggles.h
index 38d8a3e..8f6c7ad 100644
--- a/src/dawn/native/Toggles.h
+++ b/src/dawn/native/Toggles.h
@@ -72,6 +72,7 @@
DisallowSpirv,
DumpShaders,
DisableWorkgroupInit,
+ DisableDemoteToHelper,
VulkanUseDemoteToHelperInvocationExtension,
DisableSymbolRenaming,
UseUserDefinedLabelsInBackend,
diff --git a/src/dawn/native/metal/PhysicalDeviceMTL.mm b/src/dawn/native/metal/PhysicalDeviceMTL.mm
index b33c4d0..d03ee34 100644
--- a/src/dawn/native/metal/PhysicalDeviceMTL.mm
+++ b/src/dawn/native/metal/PhysicalDeviceMTL.mm
@@ -444,6 +444,14 @@
deviceToggles->Default(Toggle::MetalEnableVertexPulling, true);
}
+ // Shader `discard_fragment` changed semantics to be uniform in Metal 2.3+. See section 6.10.1.3
+ // of the Metal Spec (v3.2).
+ if (@available(macOS 11.0, iOS 14.0, *)) {
+ deviceToggles->Default(Toggle::DisableDemoteToHelper, true);
+ } else {
+ deviceToggles->ForceSet(Toggle::DisableDemoteToHelper, false);
+ }
+
// TODO(crbug.com/dawn/846): tighten this workaround when the driver bug is fixed.
deviceToggles->Default(Toggle::AlwaysResolveIntoZeroLevelAndLayer, true);
diff --git a/src/dawn/native/metal/ShaderModuleMTL.mm b/src/dawn/native/metal/ShaderModuleMTL.mm
index d3ef5cd..46517ef 100644
--- a/src/dawn/native/metal/ShaderModuleMTL.mm
+++ b/src/dawn/native/metal/ShaderModuleMTL.mm
@@ -278,6 +278,8 @@
req.tintOptions.buffer_size_ubo_index = kBufferLengthBufferSlot;
req.tintOptions.fixed_sample_mask = sampleMask;
req.tintOptions.disable_workgroup_init = false;
+ req.tintOptions.disable_demote_to_helper =
+ device->IsToggleEnabled(Toggle::DisableDemoteToHelper);
req.tintOptions.emit_vertex_point_size =
stage == SingleShaderStage::Vertex &&
renderPipeline->GetPrimitiveTopology() == wgpu::PrimitiveTopology::PointList;
diff --git a/src/tint/cmd/tint/main.cc b/src/tint/cmd/tint/main.cc
index 94c3d5a..66eff44 100644
--- a/src/tint/cmd/tint/main.cc
+++ b/src/tint/cmd/tint/main.cc
@@ -940,6 +940,7 @@
gen_options.pixel_local_attachments = options.pixel_local_attachments;
gen_options.bindings = tint::msl::writer::GenerateBindings(*input_program);
gen_options.array_length_from_uniform.ubo_binding = 30;
+ gen_options.disable_demote_to_helper = options.disable_demote_to_helper;
// Add array_length_from_uniform entries for all storage buffers with runtime sized arrays.
std::unordered_set<tint::BindingPoint> storage_bindings;
@@ -992,6 +993,11 @@
}
}
+ // The correct behavior for platform demote to helper in metal 2.3+.
+ if (options.disable_demote_to_helper) {
+ msl_version = std::max(msl_version, tint::msl::validate::MslVersion::kMsl_2_3);
+ }
+
if (options.validate && options.skip_hash.count(hash) == 0) {
tint::msl::validate::Result res;
#if TINT_BUILD_IS_MAC
diff --git a/src/tint/lang/msl/writer/common/options.h b/src/tint/lang/msl/writer/common/options.h
index f14b8fa..5e74426 100644
--- a/src/tint/lang/msl/writer/common/options.h
+++ b/src/tint/lang/msl/writer/common/options.h
@@ -141,6 +141,9 @@
/// Set to `true` to disable workgroup memory zero initialization
bool disable_workgroup_init = false;
+ /// Set to `true` to disable demote to helper transform
+ bool disable_demote_to_helper = false;
+
/// Set to `true` to generate a [[point_size]] attribute which is set to 1.0
/// for all vertex shaders in the module.
bool emit_vertex_point_size = false;
@@ -170,6 +173,7 @@
TINT_REFLECT(Options,
disable_robustness,
disable_workgroup_init,
+ disable_demote_to_helper,
emit_vertex_point_size,
disable_polyfill_integer_div_mod,
buffer_size_ubo_index,
diff --git a/src/tint/lang/msl/writer/discard_test.cc b/src/tint/lang/msl/writer/discard_test.cc
index ba040dd..4dc7d6b 100644
--- a/src/tint/lang/msl/writer/discard_test.cc
+++ b/src/tint/lang/msl/writer/discard_test.cc
@@ -32,7 +32,7 @@
namespace tint::msl::writer {
namespace {
-TEST_F(MslWriterTest, Discard) {
+TEST_F(MslWriterTest, DiscardWithDemoteToHelper) {
auto* func = b.Function("foo", ty.void_());
b.Append(func->Block(), [&] {
auto* if_ = b.If(true);
@@ -49,7 +49,10 @@
b.Return(ep);
});
- ASSERT_TRUE(Generate()) << err_ << output_.msl;
+ Options options;
+ options.disable_demote_to_helper = false;
+
+ ASSERT_TRUE(Generate(options)) << err_ << output_.msl;
EXPECT_EQ(output_.msl, MetalHeader() + R"(
struct tint_module_vars_struct {
thread bool* continue_execution;
@@ -72,5 +75,39 @@
)");
}
+TEST_F(MslWriterTest, DiscardWithoutDemoteToHelper) {
+ auto* func = b.Function("foo", ty.void_());
+ b.Append(func->Block(), [&] {
+ auto* if_ = b.If(true);
+ b.Append(if_->True(), [&] {
+ b.Discard();
+ b.ExitIf(if_);
+ });
+ b.Return(func);
+ });
+
+ auto* ep = b.Function("frag_main", ty.void_(), core::ir::Function::PipelineStage::kFragment);
+ b.Append(ep->Block(), [&] {
+ b.Call(func);
+ b.Return(ep);
+ });
+
+ Options options;
+ options.disable_demote_to_helper = true;
+
+ ASSERT_TRUE(Generate(options)) << err_ << output_.msl;
+ EXPECT_EQ(output_.msl, MetalHeader() + R"(
+void foo() {
+ if (true) {
+ discard_fragment();
+ }
+}
+
+fragment void frag_main() {
+ foo();
+}
+)");
+}
+
} // namespace
} // namespace tint::msl::writer
diff --git a/src/tint/lang/msl/writer/raise/raise.cc b/src/tint/lang/msl/writer/raise/raise.cc
index a386b79..2d878ba 100644
--- a/src/tint/lang/msl/writer/raise/raise.cc
+++ b/src/tint/lang/msl/writer/raise/raise.cc
@@ -129,7 +129,9 @@
RUN_TRANSFORM(core::ir::transform::RemoveContinueInSwitch, module);
// DemoteToHelper must come before any transform that introduces non-core instructions.
- RUN_TRANSFORM(core::ir::transform::DemoteToHelper, module);
+ if (!options.disable_demote_to_helper) {
+ RUN_TRANSFORM(core::ir::transform::DemoteToHelper, module);
+ }
RUN_TRANSFORM(raise::ShaderIO, module,
raise::ShaderIOConfig{options.emit_vertex_point_size, options.fixed_sample_mask});
