tint/transform: Fix ICE in HLSL backend
When referencing a pointer parameter with a 'let', but not using that
let.
Also fix a bunch of places where we used the old names for the pointer
transforms.
Bug: chromium:1433499
Change-Id: I8decefeacd6150bd6f7637f80e62b9cb62936235
Reviewed-on: https://dawn-review.googlesource.com/c/dawn/+/127540
Kokoro: Kokoro <noreply+kokoro@google.com>
Auto-Submit: Ben Clayton <bclayton@google.com>
Reviewed-by: Antonio Maiorano <amaiorano@google.com>
Commit-Queue: Ben Clayton <bclayton@google.com>
diff --git a/src/tint/transform/simplify_pointers.cc b/src/tint/transform/simplify_pointers.cc
index bec9019..b5af458 100644
--- a/src/tint/transform/simplify_pointers.cc
+++ b/src/tint/transform/simplify_pointers.cc
@@ -132,6 +132,13 @@
utils::Hashmap<const ast::Expression*, Symbol, 8> saved_vars;
bool needs_transform = false;
+ for (auto* ty : ctx.src->Types()) {
+ if (ty->Is<type::Pointer>()) {
+ // Program contains pointers which need removing.
+ needs_transform = true;
+ break;
+ }
+ }
// Find all the pointer-typed `let` declarations.
// Note that these must be function-scoped, as module-scoped `let`s are not
diff --git a/src/tint/transform/simplify_pointers_test.cc b/src/tint/transform/simplify_pointers_test.cc
index b05dfc4..4390c7b 100644
--- a/src/tint/transform/simplify_pointers_test.cc
+++ b/src/tint/transform/simplify_pointers_test.cc
@@ -28,16 +28,6 @@
EXPECT_FALSE(ShouldRun<SimplifyPointers>(src));
}
-TEST_F(SimplifyPointersTest, NoAddressOf) {
- auto* src = R"(
-fn f(p : ptr<function, i32>) {
- var v : i32;
-}
-)";
-
- EXPECT_FALSE(ShouldRun<SimplifyPointers>(src));
-}
-
TEST_F(SimplifyPointersTest, FoldPointer) {
auto* src = R"(
fn f() {
@@ -59,6 +49,36 @@
EXPECT_EQ(expect, str(got));
}
+TEST_F(SimplifyPointersTest, UnusedPointerParam) {
+ auto* src = R"(
+fn f(p : ptr<function, i32>) {
+}
+)";
+
+ auto* expect = src;
+
+ auto got = Run<Unshadow, SimplifyPointers>(src);
+
+ EXPECT_EQ(expect, str(got));
+}
+
+TEST_F(SimplifyPointersTest, ReferencedPointerParam) {
+ auto* src = R"(
+fn f(p : ptr<function, i32>) {
+ let x = p;
+}
+)";
+
+ auto* expect = R"(
+fn f(p : ptr<function, i32>) {
+}
+)";
+
+ auto got = Run<Unshadow, SimplifyPointers>(src);
+
+ EXPECT_EQ(expect, str(got));
+}
+
TEST_F(SimplifyPointersTest, AddressOfDeref) {
auto* src = R"(
fn f() {
diff --git a/src/tint/writer/glsl/generator_impl.cc b/src/tint/writer/glsl/generator_impl.cc
index 5ae7650..a666eaa 100644
--- a/src/tint/writer/glsl/generator_impl.cc
+++ b/src/tint/writer/glsl/generator_impl.cc
@@ -2982,9 +2982,8 @@
out << "x" << mat->rows();
}
} else if (TINT_UNLIKELY(type->Is<type::Pointer>())) {
- TINT_ICE(Writer, diagnostics_)
- << "Attempting to emit pointer type. These should have been removed "
- "with the InlinePointerLets transform";
+ TINT_ICE(Writer, diagnostics_) << "Attempting to emit pointer type. These should have been "
+ "removed with the SimplifyPointers transform";
return false;
} else if (type->Is<type::Sampler>()) {
return false;
diff --git a/src/tint/writer/glsl/generator_impl_sanitizer_test.cc b/src/tint/writer/glsl/generator_impl_sanitizer_test.cc
index 79a3ea3..097b519 100644
--- a/src/tint/writer/glsl/generator_impl_sanitizer_test.cc
+++ b/src/tint/writer/glsl/generator_impl_sanitizer_test.cc
@@ -228,7 +228,7 @@
EXPECT_EQ(expect, got);
}
-TEST_F(GlslSanitizerTest, InlinePtrLetsBasic) {
+TEST_F(GlslSanitizerTest, SimplifyPointersBasic) {
// var v : i32;
// let p : ptr<function, i32> = &v;
// let x : i32 = *p;
@@ -267,7 +267,7 @@
EXPECT_EQ(expect, got);
}
-TEST_F(GlslSanitizerTest, InlinePtrLetsComplexChain) {
+TEST_F(GlslSanitizerTest, SimplifyPointersComplexChain) {
// var a : array<mat4x4<f32>, 4u>;
// let ap : ptr<function, array<mat4x4<f32>, 4u>> = &a;
// let mp : ptr<function, mat4x4<f32>> = &(*ap)[3i];
diff --git a/src/tint/writer/hlsl/generator_impl.cc b/src/tint/writer/hlsl/generator_impl.cc
index 0dc6586..4d07b4f 100644
--- a/src/tint/writer/hlsl/generator_impl.cc
+++ b/src/tint/writer/hlsl/generator_impl.cc
@@ -281,15 +281,14 @@
// TODO(crbug.com/tint/1752): This is only necessary when FXC is being used.
manager.Add<transform::DemoteToHelper>();
- // ArrayLengthFromUniform must come after InlinePointerLets and Simplify, as
- // it assumes that the form of the array length argument is &var.array.
+ // ArrayLengthFromUniform must come after SimplifyPointers as it assumes that the form of the
+ // array length argument is &var.array.
manager.Add<transform::ArrayLengthFromUniform>();
data.Add<transform::ArrayLengthFromUniform::Config>(std::move(array_length_from_uniform_cfg));
// DecomposeMemoryAccess must come after:
- // * InlinePointerLets, as we cannot take the address of calls to
- // DecomposeMemoryAccess::Intrinsic.
- // * Simplify, as we need to fold away the address-of and dereferences of
- // `*(&(intrinsic_load()))` expressions.
+ // * SimplifyPointers, as we cannot take the address of calls to
+ // DecomposeMemoryAccess::Intrinsic and we need to fold away the address-of and dereferences
+ // of `*(&(intrinsic_load()))` expressions.
// * RemovePhonies, as phonies can be assigned a pointer to a
// non-constructible buffer, or dynamic array, which DMA cannot cope with.
manager.Add<transform::DecomposeMemoryAccess>();
@@ -4070,9 +4069,8 @@
return true;
},
[&](const type::Pointer*) {
- TINT_ICE(Writer, diagnostics_)
- << "Attempting to emit pointer type. These should have been "
- "removed with the InlinePointerLets transform";
+ TINT_ICE(Writer, diagnostics_) << "Attempting to emit pointer type. These should have "
+ "been removed with the SimplifyPointers transform";
return false;
},
[&](const type::Sampler* sampler) {
diff --git a/src/tint/writer/hlsl/generator_impl_sanitizer_test.cc b/src/tint/writer/hlsl/generator_impl_sanitizer_test.cc
index cc8c253..c511a83 100644
--- a/src/tint/writer/hlsl/generator_impl_sanitizer_test.cc
+++ b/src/tint/writer/hlsl/generator_impl_sanitizer_test.cc
@@ -237,7 +237,7 @@
EXPECT_EQ(expect, got);
}
-TEST_F(HlslSanitizerTest, InlinePtrLetsBasic) {
+TEST_F(HlslSanitizerTest, SimplifyPointersBasic) {
// var v : i32;
// let p : ptr<function, i32> = &v;
// let x : i32 = *p;
@@ -269,7 +269,7 @@
EXPECT_EQ(expect, got);
}
-TEST_F(HlslSanitizerTest, InlinePtrLetsComplexChain) {
+TEST_F(HlslSanitizerTest, SimplifyPointersComplexChain) {
// var a : array<mat4x4<f32>, 4u>;
// let ap : ptr<function, array<mat4x4<f32>, 4u>> = &a;
// let mp : ptr<function, mat4x4<f32>> = &(*ap)[3i];
diff --git a/test/tint/bug/chromium/1433499.wgsl b/test/tint/bug/chromium/1433499.wgsl
new file mode 100644
index 0000000..3fa9454
--- /dev/null
+++ b/test/tint/bug/chromium/1433499.wgsl
@@ -0,0 +1,3 @@
+fn f(p : ptr<function, f32>) {
+ let x = p;
+}
diff --git a/test/tint/bug/chromium/1433499.wgsl.expected.dxc.hlsl b/test/tint/bug/chromium/1433499.wgsl.expected.dxc.hlsl
new file mode 100644
index 0000000..099b83c
--- /dev/null
+++ b/test/tint/bug/chromium/1433499.wgsl.expected.dxc.hlsl
@@ -0,0 +1,7 @@
+[numthreads(1, 1, 1)]
+void unused_entry_point() {
+ return;
+}
+
+void f(inout float p) {
+}
diff --git a/test/tint/bug/chromium/1433499.wgsl.expected.fxc.hlsl b/test/tint/bug/chromium/1433499.wgsl.expected.fxc.hlsl
new file mode 100644
index 0000000..099b83c
--- /dev/null
+++ b/test/tint/bug/chromium/1433499.wgsl.expected.fxc.hlsl
@@ -0,0 +1,7 @@
+[numthreads(1, 1, 1)]
+void unused_entry_point() {
+ return;
+}
+
+void f(inout float p) {
+}
diff --git a/test/tint/bug/chromium/1433499.wgsl.expected.glsl b/test/tint/bug/chromium/1433499.wgsl.expected.glsl
new file mode 100644
index 0000000..410e9f4
--- /dev/null
+++ b/test/tint/bug/chromium/1433499.wgsl.expected.glsl
@@ -0,0 +1,9 @@
+#version 310 es
+
+layout(local_size_x = 1, local_size_y = 1, local_size_z = 1) in;
+void unused_entry_point() {
+ return;
+}
+void f(inout float p) {
+}
+
diff --git a/test/tint/bug/chromium/1433499.wgsl.expected.msl b/test/tint/bug/chromium/1433499.wgsl.expected.msl
new file mode 100644
index 0000000..8ceb0e0
--- /dev/null
+++ b/test/tint/bug/chromium/1433499.wgsl.expected.msl
@@ -0,0 +1,6 @@
+#include <metal_stdlib>
+
+using namespace metal;
+void f(thread float* const p) {
+}
+
diff --git a/test/tint/bug/chromium/1433499.wgsl.expected.spvasm b/test/tint/bug/chromium/1433499.wgsl.expected.spvasm
new file mode 100644
index 0000000..471f877
--- /dev/null
+++ b/test/tint/bug/chromium/1433499.wgsl.expected.spvasm
@@ -0,0 +1,26 @@
+; SPIR-V
+; Version: 1.3
+; Generator: Google Tint Compiler; 0
+; Bound: 11
+; Schema: 0
+ OpCapability Shader
+ OpMemoryModel Logical GLSL450
+ OpEntryPoint GLCompute %unused_entry_point "unused_entry_point"
+ OpExecutionMode %unused_entry_point LocalSize 1 1 1
+ OpName %unused_entry_point "unused_entry_point"
+ OpName %f "f"
+ OpName %p "p"
+ %void = OpTypeVoid
+ %1 = OpTypeFunction %void
+ %float = OpTypeFloat 32
+%_ptr_Function_float = OpTypePointer Function %float
+ %5 = OpTypeFunction %void %_ptr_Function_float
+%unused_entry_point = OpFunction %void None %1
+ %4 = OpLabel
+ OpReturn
+ OpFunctionEnd
+ %f = OpFunction %void None %5
+ %p = OpFunctionParameter %_ptr_Function_float
+ %10 = OpLabel
+ OpReturn
+ OpFunctionEnd
diff --git a/test/tint/bug/chromium/1433499.wgsl.expected.wgsl b/test/tint/bug/chromium/1433499.wgsl.expected.wgsl
new file mode 100644
index 0000000..3fa9454
--- /dev/null
+++ b/test/tint/bug/chromium/1433499.wgsl.expected.wgsl
@@ -0,0 +1,3 @@
+fn f(p : ptr<function, f32>) {
+ let x = p;
+}
