[tint] Arm enable transform handle for direct variable access
Some arm devices do not seem to correctly handle texture access as
a function parameter that are not accessed with a sampler.
This fix has been tested on device with chrome.
Bug:387000529,406795494
Change-Id: Id2585fe8733461150f457b5b51eb7ac8e49f16f8
Reviewed-on: https://dawn-review.googlesource.com/c/dawn/+/243396
Reviewed-by: James Price <jrprice@google.com>
Commit-Queue: Peter McNeeley <petermcneeley@google.com>
Reviewed-by: dan sinclair <dsinclair@chromium.org>
diff --git a/src/dawn/native/Toggles.cpp b/src/dawn/native/Toggles.cpp
index 218dc1f..7c8ce6d 100644
--- a/src/dawn/native/Toggles.cpp
+++ b/src/dawn/native/Toggles.cpp
@@ -642,6 +642,10 @@
{Toggle::VulkanScalarizeClampBuiltin,
{"vulkan_scalarize_clamp_builtin", "Scalarize calls to the clamp builtin.",
"https://crbug.com/407109052", ToggleStage::Device}},
+ {Toggle::VulkanDirectVariableAccessTransformHandle,
+ {"vulkan_direct_variable_access_transform_handle",
+ "Transform handles using direct variable access.", "https://crbug.com/387000529",
+ ToggleStage::Device}},
{Toggle::VulkanAddWorkToEmptyResolvePass,
{"vulkan_add_work_to_empty_resolve_pass",
"Adds a small amount of work to empty render passes which perform a resolve. This toggle is "
diff --git a/src/dawn/native/Toggles.h b/src/dawn/native/Toggles.h
index de32833..ee2a54e 100644
--- a/src/dawn/native/Toggles.h
+++ b/src/dawn/native/Toggles.h
@@ -152,6 +152,7 @@
D3D12RelaxBufferTextureCopyPitchAndOffsetAlignment,
UseVulkanMemoryModel,
VulkanScalarizeClampBuiltin,
+ VulkanDirectVariableAccessTransformHandle,
VulkanAddWorkToEmptyResolvePass,
EnableIntegerRangeAnalysisInRobustness,
diff --git a/src/dawn/native/vulkan/PhysicalDeviceVk.cpp b/src/dawn/native/vulkan/PhysicalDeviceVk.cpp
index 10be069..604374b 100644
--- a/src/dawn/native/vulkan/PhysicalDeviceVk.cpp
+++ b/src/dawn/native/vulkan/PhysicalDeviceVk.cpp
@@ -831,6 +831,12 @@
deviceToggles->Default(Toggle::PolyfillPackUnpack4x8Norm, true);
}
+ if (gpu_info::IsARM(GetVendorId())) {
+ // chromium:387000529: Arm devices have issues passing texture handles as parameters to
+ // functions for accesses without a sampler (TextureLoad).
+ deviceToggles->Default(Toggle::VulkanDirectVariableAccessTransformHandle, true);
+ }
+
if (IsAndroidSamsung() || IsAndroidQualcomm() || IsAndroidHuawei()) {
deviceToggles->Default(Toggle::IgnoreImportedAHardwareBufferVulkanImageSize, true);
}
diff --git a/src/dawn/native/vulkan/ShaderModuleVk.cpp b/src/dawn/native/vulkan/ShaderModuleVk.cpp
index 51a1424..7de811a 100644
--- a/src/dawn/native/vulkan/ShaderModuleVk.cpp
+++ b/src/dawn/native/vulkan/ShaderModuleVk.cpp
@@ -286,7 +286,8 @@
GetDevice()->IsToggleEnabled(Toggle::UseVulkanMemoryModel);
req.tintOptions.scalarize_clamp_builtin =
GetDevice()->IsToggleEnabled(Toggle::VulkanScalarizeClampBuiltin);
-
+ req.tintOptions.dva_transform_handle =
+ GetDevice()->IsToggleEnabled(Toggle::VulkanDirectVariableAccessTransformHandle);
// Pass matrices to user functions by pointer on Qualcomm devices to workaround a known bug.
// See crbug.com/tint/2045.
if (ToBackend(GetDevice()->GetPhysicalDevice())->IsAndroidQualcomm()) {
diff --git a/src/tint/lang/core/ir/transform/direct_variable_access.h b/src/tint/lang/core/ir/transform/direct_variable_access.h
index 66d93c8..77902eb 100644
--- a/src/tint/lang/core/ir/transform/direct_variable_access.h
+++ b/src/tint/lang/core/ir/transform/direct_variable_access.h
@@ -62,7 +62,7 @@
/// DirectVariableAccess is a transform that transforms parameters in the 'storage',
/// 'uniform' and 'workgroup' address space so that they're accessed directly by the function,
-/// instead of being passed by pointer. It will potentiall also transform `private`, `handle` or
+/// instead of being passed by pointer. It will potentially also transform `private`, `handle` or
/// `function` parameters depending on provided options.
///
/// DirectVariableAccess works by creating specializations of functions that have matching
diff --git a/src/tint/lang/core/ir/transform/direct_variable_access_test.cc b/src/tint/lang/core/ir/transform/direct_variable_access_test.cc
index 6c7cf01..f11af2b 100644
--- a/src/tint/lang/core/ir/transform/direct_variable_access_test.cc
+++ b/src/tint/lang/core/ir/transform/direct_variable_access_test.cc
@@ -5189,6 +5189,80 @@
EXPECT_EQ(expect, str());
}
+TEST_F(IR_DirectVariableAccessTest_HandleAS, Enabled_LocalTextureParamTextureLoad) {
+ auto* tex =
+ b.Var("tex", handle, ty.sampled_texture(core::type::TextureDimension::k2d, ty.f32()),
+ core::Access::kReadWrite);
+ tex->SetBindingPoint(0, 0);
+ b.ir.root_block->Append(tex);
+
+ auto* t = b.FunctionParam("texparam",
+ ty.sampled_texture(core::type::TextureDimension::k2d, ty.f32()));
+
+ auto* fn = b.Function("f", ty.void_());
+ fn->SetParams({t});
+ b.Append(fn->Block(), [&] {
+ b.Let("p", b.Call(ty.vec4<f32>(), core::BuiltinFn::kTextureLoad, t,
+ b.Splat(ty.vec2<u32>(), 0_u), 0_u));
+ b.Return(fn);
+ });
+
+ auto* fn2 = b.Function("g", ty.void_());
+ b.Append(fn2->Block(), [&] {
+ auto* t2 = b.Load(tex);
+ b.Call(ty.void_(), fn, t2);
+ b.Return(fn2);
+ });
+
+ auto* src = R"(
+$B1: { # root
+ %tex:ptr<handle, texture_2d<f32>, read_write> = var undef @binding_point(0, 0)
+}
+
+%f = func(%texparam:texture_2d<f32>):void {
+ $B2: {
+ %4:vec4<f32> = textureLoad %texparam, vec2<u32>(0u), 0u
+ %p:vec4<f32> = let %4
+ ret
+ }
+}
+%g = func():void {
+ $B3: {
+ %7:texture_2d<f32> = load %tex
+ %8:void = call %f, %7
+ ret
+ }
+}
+)";
+
+ EXPECT_EQ(src, str());
+
+ auto* expect = R"(
+$B1: { # root
+ %tex:ptr<handle, texture_2d<f32>, read_write> = var undef @binding_point(0, 0)
+}
+
+%f = func():void {
+ $B2: {
+ %3:texture_2d<f32> = load %tex
+ %4:vec4<f32> = textureLoad %3, vec2<u32>(0u), 0u
+ %p:vec4<f32> = let %4
+ ret
+ }
+}
+%g = func():void {
+ $B3: {
+ %7:void = call %f
+ ret
+ }
+}
+)";
+
+ Run(DirectVariableAccess, kTransformHandle);
+
+ EXPECT_EQ(expect, str());
+}
+
TEST_F(IR_DirectVariableAccessTest_HandleAS, Enabled_ParamTextureLocalSampler) {
auto* tex_ty = ty.sampled_texture(core::type::TextureDimension::k2d, ty.f32());
auto* tex = b.Var("tex", handle, tex_ty, core::Access::kReadWrite);
diff --git a/src/tint/lang/spirv/writer/common/options.h b/src/tint/lang/spirv/writer/common/options.h
index efd1ead..d1372b4 100644
--- a/src/tint/lang/spirv/writer/common/options.h
+++ b/src/tint/lang/spirv/writer/common/options.h
@@ -204,6 +204,9 @@
/// Set to `true` if the clamp builtin should be scalarized for vector operations
bool scalarize_clamp_builtin = false;
+ /// Set to `true` if handles should be transformed by direct variable access.
+ bool dva_transform_handle = false;
+
/// Offsets of the minDepth and maxDepth push constants.
std::optional<RangeOffsets> depth_range_offsets = std::nullopt;
@@ -228,6 +231,7 @@
disable_polyfill_integer_div_mod,
use_vulkan_memory_model,
scalarize_clamp_builtin,
+ dva_transform_handle,
depth_range_offsets);
};
diff --git a/src/tint/lang/spirv/writer/raise/raise.cc b/src/tint/lang/spirv/writer/raise/raise.cc
index d6cc4ad..9f7d389 100644
--- a/src/tint/lang/spirv/writer/raise/raise.cc
+++ b/src/tint/lang/spirv/writer/raise/raise.cc
@@ -144,6 +144,7 @@
core::ir::transform::DirectVariableAccessOptions dva_options;
dva_options.transform_function = true;
dva_options.transform_private = true;
+ dva_options.transform_handle = options.dva_transform_handle;
RUN_TRANSFORM(core::ir::transform::DirectVariableAccess, module, dva_options);
if (options.pass_matrix_by_pointer) {
diff --git a/src/tint/lang/spirv/writer/var_test.cc b/src/tint/lang/spirv/writer/var_test.cc
index f98f4de..b69ab77 100644
--- a/src/tint/lang/spirv/writer/var_test.cc
+++ b/src/tint/lang/spirv/writer/var_test.cc
@@ -549,6 +549,71 @@
EXPECT_INST("%load = OpLoad %3 %v");
}
+TEST_F(SpirvWriterTest, TextureVar_TextureParamTextureLoad_NoDva) {
+ auto* tex =
+ b.Var("tex", handle, ty.sampled_texture(core::type::TextureDimension::k2d, ty.f32()),
+ core::Access::kReadWrite);
+ tex->SetBindingPoint(0, 0);
+ mod.root_block->Append(tex);
+
+ auto* t = b.FunctionParam("texparam",
+ ty.sampled_texture(core::type::TextureDimension::k2d, ty.f32()));
+
+ auto* fn = b.Function("f", ty.void_());
+ fn->SetParams({t});
+ b.Append(fn->Block(), [&] {
+ b.Let("p", b.Call(ty.vec4<f32>(), core::BuiltinFn::kTextureLoad, t,
+ b.Splat(ty.vec2<u32>(), 0_u), 0_u));
+ b.Return(fn);
+ });
+
+ auto* fn2 = b.Function("g", ty.void_());
+ b.Append(fn2->Block(), [&] {
+ auto* t2 = b.Load(tex);
+ b.Call(ty.void_(), fn, t2);
+ b.Return(fn2);
+ });
+
+ Options opts{};
+ opts.dva_transform_handle = false;
+
+ ASSERT_TRUE(Generate(opts)) << Error() << output_;
+ EXPECT_INST("OpFunctionParameter");
+}
+
+TEST_F(SpirvWriterTest, TextureVar_TextureParamTextureLoad_Dva) {
+ auto* tex =
+ b.Var("tex", handle, ty.sampled_texture(core::type::TextureDimension::k2d, ty.f32()),
+ core::Access::kReadWrite);
+ tex->SetBindingPoint(0, 0);
+ mod.root_block->Append(tex);
+
+ auto* t = b.FunctionParam("texparam",
+ ty.sampled_texture(core::type::TextureDimension::k2d, ty.f32()));
+
+ auto* fn = b.Function("f", ty.void_());
+ fn->SetParams({t});
+ b.Append(fn->Block(), [&] {
+ b.Let("p", b.Call(ty.vec4<f32>(), core::BuiltinFn::kTextureLoad, t,
+ b.Splat(ty.vec2<u32>(), 0_u), 0_u));
+ b.Return(fn);
+ });
+
+ auto* fn2 = b.Function("g", ty.void_());
+ b.Append(fn2->Block(), [&] {
+ auto* t2 = b.Load(tex);
+ b.Call(ty.void_(), fn, t2);
+ b.Return(fn2);
+ });
+
+ Options opts{};
+ opts.dva_transform_handle = true;
+
+ ASSERT_TRUE(Generate(opts)) << Error() << output_;
+ // Consider a EXPECT_NOT_INST macro.
+ ASSERT_TRUE(output_.find("OpFunctionParameter") == std::string::npos) << output_;
+}
+
TEST_F(SpirvWriterTest, ReadOnlyStorageTextureVar) {
auto format = core::TexelFormat::kRgba8Unorm;
auto* v = b.Var("v", ty.ptr(core::AddressSpace::kHandle,
