[spirv-reader][ir] Make sure `position` is used.
In SPIR-V, it's possible to create a `position` var, set it into the
`OpEntryPoint` interface, and then never store to `position`. This will
produce undefined values into the position.
In IR, we _must_ reference the `position` in order for it to be
considered part of the interface for the vertex shader. So, if we detect
that `position` is setup in `OpEntryPoint`, but never referenced, we'll
inject a store of `(0, 0, 0, 0)` into the `position` variable to satisfy
the interface rules.
Bug: 42250952
Change-Id: Ic37a4d6e34842bfd305dca22380df7527c469bed
Reviewed-on: https://dawn-review.googlesource.com/c/dawn/+/245615
Reviewed-by: James Price <jrprice@google.com>
Commit-Queue: dan sinclair <dsinclair@chromium.org>
Auto-Submit: dan sinclair <dsinclair@chromium.org>
diff --git a/src/tint/lang/core/ir/var.h b/src/tint/lang/core/ir/var.h
index e361bd0..cf503fd 100644
--- a/src/tint/lang/core/ir/var.h
+++ b/src/tint/lang/core/ir/var.h
@@ -118,6 +118,8 @@
TINT_ASSERT(!attributes_.builtin.has_value());
attributes_.builtin = val;
}
+ /// Returns the builtin information, if available
+ std::optional<core::BuiltinValue> Builtin() const { return attributes_.builtin; }
/// Resets the IO attributes
void ResetAttributes() { attributes_ = {}; }
diff --git a/src/tint/lang/spirv/reader/parser/function_test.cc b/src/tint/lang/spirv/reader/parser/function_test.cc
index c4df303..2b53807 100644
--- a/src/tint/lang/spirv/reader/parser/function_test.cc
+++ b/src/tint/lang/spirv/reader/parser/function_test.cc
@@ -130,21 +130,130 @@
)");
}
-TEST_F(SpirvParserTest, DISABLED_VertexShader) {
+TEST_F(SpirvParserTest, VertexShader_PositionUnused) {
EXPECT_IR(R"(
OpCapability Shader
OpMemoryModel Logical GLSL450
- OpEntryPoint Vertex %main "main"
+ OpEntryPoint Vertex %main "main" %position
+ OpName %position "position"
+ OpDecorate %position BuiltIn Position
%void = OpTypeVoid
+ %float = OpTypeFloat 32
%ep_type = OpTypeFunction %void
+ %v4float = OpTypeVector %float 4
+ %ptr = OpTypePointer Output %v4float
+ %position = OpVariable %ptr Output
%main = OpFunction %void None %ep_type
%main_start = OpLabel
OpReturn
OpFunctionEnd
)",
R"(
+$B1: { # root
+ %position:ptr<__out, vec4<f32>, read_write> = var undef @builtin(position)
+}
+
%main = @vertex func():void {
- $B1: {
+ $B2: {
+ store %position, vec4<f32>(0.0f)
+ ret
+ }
+}
+)");
+}
+
+TEST_F(SpirvParserTest, VertexShader_PositionUsed) {
+ EXPECT_IR(R"(
+ OpCapability Shader
+ OpMemoryModel Logical GLSL450
+ OpEntryPoint Vertex %main "main" %position
+ OpName %position "position"
+ OpDecorate %position BuiltIn Position
+ %void = OpTypeVoid
+ %float = OpTypeFloat 32
+ %ep_type = OpTypeFunction %void
+ %v4float = OpTypeVector %float 4
+ %ptr = OpTypePointer Output %v4float
+ %f1 = OpConstant %float 1
+ %f2 = OpConstant %float 2
+ %f3 = OpConstant %float 3
+ %f4 = OpConstant %float 4
+ %v4 = OpConstantComposite %v4float %f1 %f2 %f3 %f4
+ %position = OpVariable %ptr Output
+ %main = OpFunction %void None %ep_type
+ %main_start = OpLabel
+ OpStore %position %v4
+ OpReturn
+ OpFunctionEnd
+)",
+ R"(
+$B1: { # root
+ %position:ptr<__out, vec4<f32>, read_write> = var undef @builtin(position)
+}
+
+%main = @vertex func():void {
+ $B2: {
+ store %position, vec4<f32>(1.0f, 2.0f, 3.0f, 4.0f)
+ ret
+ }
+}
+)");
+}
+
+TEST_F(SpirvParserTest, VertexShader_PositionUsed_Transitive) {
+ EXPECT_IR(R"(
+ OpCapability Shader
+ OpMemoryModel Logical GLSL450
+ OpEntryPoint Vertex %main "main" %position
+ OpName %position "position"
+ OpDecorate %position BuiltIn Position
+ %void = OpTypeVoid
+ %float = OpTypeFloat 32
+ %ep_type = OpTypeFunction %void
+ %v4float = OpTypeVector %float 4
+ %ptr = OpTypePointer Output %v4float
+ %f1 = OpConstant %float 1
+ %f2 = OpConstant %float 2
+ %f3 = OpConstant %float 3
+ %f4 = OpConstant %float 4
+ %v4 = OpConstantComposite %v4float %f1 %f2 %f3 %f4
+ %position = OpVariable %ptr Output
+ %c = OpFunction %void None %ep_type
+ %c1 = OpLabel
+ OpStore %position %v4
+ OpReturn
+ OpFunctionEnd
+ %b = OpFunction %void None %ep_type
+ %b1 = OpLabel
+ %b2 = OpFunctionCall %void %c
+ OpReturn
+ OpFunctionEnd
+ %main = OpFunction %void None %ep_type
+ %main_start = OpLabel
+ %a1 = OpFunctionCall %void %b
+ OpReturn
+ OpFunctionEnd
+)",
+ R"(
+$B1: { # root
+ %position:ptr<__out, vec4<f32>, read_write> = var undef @builtin(position)
+}
+
+%2 = func():void {
+ $B2: {
+ store %position, vec4<f32>(1.0f, 2.0f, 3.0f, 4.0f)
+ ret
+ }
+}
+%3 = func():void {
+ $B3: {
+ %4:void = call %2
+ ret
+ }
+}
+%main = @vertex func():void {
+ $B4: {
+ %6:void = call %3
ret
}
}
diff --git a/src/tint/lang/spirv/reader/parser/parser.cc b/src/tint/lang/spirv/reader/parser/parser.cc
index fe9c4e8..d718701 100644
--- a/src/tint/lang/spirv/reader/parser/parser.cc
+++ b/src/tint/lang/spirv/reader/parser/parser.cc
@@ -52,6 +52,7 @@
#include "src/tint/lang/core/ir/builder.h"
#include "src/tint/lang/core/ir/module.h"
+#include "src/tint/lang/core/ir/referenced_module_vars.h"
#include "src/tint/lang/core/type/builtin_structs.h"
#include "src/tint/lang/spirv/builtin_fn.h"
#include "src/tint/lang/spirv/ir/builtin_call.h"
@@ -992,6 +993,41 @@
// Set the entry point name.
ir_.SetName(func, entry_point.GetOperand(2).AsString());
+
+ // For vertex shaders, must make sure the `position` builtin is referenced. In SPIR-V it
+ // can be referenced in the `OpEntryPoint` without any usages. When translating we need
+ // to make sure we insert a fake write if there are no usages.
+ if (func->IsVertex()) {
+ core::ir::ReferencedModuleVars<const core::ir::Module> referenced(ir_);
+ for (uint32_t i = 3; i < entry_point.NumInOperands(); ++i) {
+ auto interface_id = entry_point.GetSingleWordInOperand(i);
+ auto* val = Value(interface_id);
+ TINT_ASSERT(val);
+
+ auto* inst_result = val->As<core::ir::InstructionResult>();
+ TINT_ASSERT(inst_result);
+
+ auto* var = inst_result->Instruction()->As<core::ir::Var>();
+ TINT_ASSERT(var);
+
+ auto builtin = var->Builtin();
+ if (!builtin.has_value() || builtin.value() != core::BuiltinValue::kPosition) {
+ continue;
+ }
+
+ auto refs = referenced.TransitiveReferences(func);
+ // The referenced variables does not contain the var which has the `position`
+ // builtin, then we need to create a fake store.
+ if (!refs.Contains(var)) {
+ auto* store = b_.Store(var, b_.Zero(var->Result()->Type()->UnwrapPtr()));
+ store->InsertBefore(func->Block()->Front());
+ }
+
+ // There should only be one `position` so we're done if we've processed the
+ // first one found.
+ break;
+ }
+ }
}
// Handle OpExecutionMode declarations.
