spirv-reader: instance_index must have u32 store type
Fixed: tint:485
Change-Id: I73613ae916b2d86b25470f292e5bf5cd5c7f288b
Reviewed-on: https://dawn-review.googlesource.com/c/tint/+/40582
Commit-Queue: David Neto <dneto@google.com>
Auto-Submit: David Neto <dneto@google.com>
Reviewed-by: dan sinclair <dsinclair@chromium.org>
diff --git a/src/reader/spirv/function.cc b/src/reader/spirv/function.cc
index 58fa33f..0767da6 100644
--- a/src/reader/spirv/function.cc
+++ b/src/reader/spirv/function.cc
@@ -2040,6 +2040,11 @@
<< "unhandled use of a pointer to the VertexIndex builtin, with ID: "
<< id;
return {};
+ case SkipReason::kInstanceIndexBuiltinPointer:
+ Fail() << "unhandled use of a pointer to the InstanceIndex builtin, with "
+ "ID: "
+ << id;
+ return {};
case SkipReason::kSampleMaskInBuiltinPointer:
Fail()
<< "unhandled use of a pointer to the SampleMask builtin, with ID: "
@@ -3048,13 +3053,15 @@
SkipReason::kPointSizeBuiltinValue;
return true;
case SkipReason::kSampleIdBuiltinPointer:
- case SkipReason::kVertexIndexBuiltinPointer: {
+ case SkipReason::kVertexIndexBuiltinPointer:
+ case SkipReason::kInstanceIndexBuiltinPointer: {
// The SPIR-V variable is i32, but WGSL requires u32.
- auto var_id = parser_impl_.IdForSpecialBuiltIn(
- (skip_reason == SkipReason::kSampleIdBuiltinPointer)
- ? SpvBuiltInSampleId
- : SpvBuiltInVertexIndex);
- auto name = namer_.Name(var_id);
+ auto name = NameForSpecialInputBuiltin(skip_reason);
+ if (name.empty()) {
+ return Fail() << "internal error: unhandled special input builtin "
+ "variable: "
+ << inst.PrettyPrint();
+ }
ast::Expression* id_expr = create<ast::IdentifierExpression>(
Source{}, builder_.Symbols().Register(name));
auto expr = TypedExpression{
@@ -3133,6 +3140,28 @@
<< inst.PrettyPrint();
}
+std::string FunctionEmitter::NameForSpecialInputBuiltin(
+ SkipReason skip_reason) {
+ SpvBuiltIn spv_builtin = SpvBuiltIn(0);
+ switch (skip_reason) {
+ case SkipReason::kSampleIdBuiltinPointer:
+ spv_builtin = SpvBuiltInSampleId;
+ break;
+ case SkipReason::kVertexIndexBuiltinPointer:
+ spv_builtin = SpvBuiltInVertexIndex;
+ break;
+ case SkipReason::kInstanceIndexBuiltinPointer:
+ spv_builtin = SpvBuiltInInstanceIndex;
+ break;
+ default:
+ // Invalid. Issue the error in the caller.
+ return "";
+ }
+ // The SPIR-V variable is i32, but WGSL requires u32.
+ auto var_id = parser_impl_.IdForSpecialBuiltIn(spv_builtin);
+ return namer_.Name(var_id);
+}
+
TypedExpression FunctionEmitter::MakeOperand(
const spvtools::opt::Instruction& inst,
uint32_t operand_index) {
@@ -3725,6 +3754,9 @@
case SpvBuiltInVertexIndex:
def->skip = SkipReason::kVertexIndexBuiltinPointer;
break;
+ case SpvBuiltInInstanceIndex:
+ def->skip = SkipReason::kInstanceIndexBuiltinPointer;
+ break;
case SpvBuiltInSampleMask: {
// Distinguish between input and output variable.
const auto storage_class =
diff --git a/src/reader/spirv/function.h b/src/reader/spirv/function.h
index 27ab12a..b562968 100644
--- a/src/reader/spirv/function.h
+++ b/src/reader/spirv/function.h
@@ -233,6 +233,11 @@
/// builtin variable. Don't generate its address.
kVertexIndexBuiltinPointer,
+ /// `kInstanceIndexBuiltinPointer`: the value is a pointer to the
+ /// InstanceIndex
+ /// builtin variable. Don't generate its address.
+ kInstanceIndexBuiltinPointer,
+
/// `kSampleMaskInBuiltinPointer`: the value is a pointer to the SampleMaskIn
/// builtin input variable. Don't generate its address.
kSampleMaskInBuiltinPointer,
@@ -351,6 +356,9 @@
case SkipReason::kVertexIndexBuiltinPointer:
o << " skip:vertexindex_pointer";
break;
+ case SkipReason::kInstanceIndexBuiltinPointer:
+ o << " skip:instanceindex_pointer";
+ break;
case SkipReason::kSampleMaskInBuiltinPointer:
o << " skip:samplemaskin_pointer";
break;
@@ -762,6 +770,13 @@
return SkipReason::kDontSkip;
}
+ /// Returns the WGSL variable name for an input builtin variable whose
+ /// translation is managed via the SkipReason mechanism.
+ /// @param skip_reason the skip reason for the special variable
+ /// @returns the variable name for a special builtin variable
+ /// that is handled via the "skip" mechanism.
+ std::string NameForSpecialInputBuiltin(SkipReason skip_reason);
+
/// Returns the most deeply nested structured construct which encloses the
/// WGSL scopes of names declared in both block positions. Each position must
/// be a valid index into the function block order array.
diff --git a/src/reader/spirv/parser_impl.cc b/src/reader/spirv/parser_impl.cc
index e4a212e..ebd2ce8 100644
--- a/src/reader/spirv/parser_impl.cc
+++ b/src/reader/spirv/parser_impl.cc
@@ -1294,6 +1294,7 @@
return nullptr;
case SpvBuiltInSampleId:
case SpvBuiltInVertexIndex:
+ case SpvBuiltInInstanceIndex:
// The SPIR-V variable is likely to be signed (because GLSL
// requires signed), but WGSL requires unsigned. Handle specially
// so we always perform the conversion at load and store.
diff --git a/src/reader/spirv/parser_impl_module_var_test.cc b/src/reader/spirv/parser_impl_module_var_test.cc
index 4431509..49e81c3 100644
--- a/src/reader/spirv/parser_impl_module_var_test.cc
+++ b/src/reader/spirv/parser_impl_module_var_test.cc
@@ -3349,6 +3349,384 @@
})")) << module_str;
}
+// Returns the start of a shader for testing InstanceIndex,
+// parameterized by store type of %int or %uint
+std::string InstanceIndexPreamble(std::string store_type) {
+ return R"(
+ OpCapability Shader
+ OpMemoryModel Logical Simple
+ OpEntryPoint Vertex %main "main" %1
+ OpDecorate %1 BuiltIn InstanceIndex
+ %void = OpTypeVoid
+ %voidfn = OpTypeFunction %void
+ %float = OpTypeFloat 32
+ %uint = OpTypeInt 32 0
+ %int = OpTypeInt 32 1
+ %ptr_ty = OpTypePointer Input )" +
+ store_type + R"(
+ %1 = OpVariable %ptr_ty Input
+)";
+}
+
+TEST_F(SpvModuleScopeVarParserTest, InstanceIndex_I32_Load_Direct) {
+ const std::string assembly = InstanceIndexPreamble("%int") + R"(
+ %main = OpFunction %void None %voidfn
+ %entry = OpLabel
+ %2 = OpLoad %int %1
+ OpReturn
+ OpFunctionEnd
+ )";
+ auto p = parser(test::Assemble(assembly));
+ ASSERT_TRUE(p->BuildAndParseInternalModule()) << p->error() << assembly;
+ EXPECT_TRUE(p->error().empty());
+ const auto module_str = p->program().to_str();
+ // Correct declaration
+ EXPECT_THAT(module_str, HasSubstr(R"(
+ Variable{
+ Decorations{
+ BuiltinDecoration{instance_index}
+ }
+ x_1
+ in
+ __u32
+ })"));
+
+ // Correct body
+ EXPECT_THAT(module_str, HasSubstr(R"(
+ VariableDeclStatement{
+ VariableConst{
+ x_2
+ none
+ __i32
+ {
+ TypeConstructor[not set]{
+ __i32
+ Identifier[not set]{x_1}
+ }
+ }
+ }
+ })"))
+ << module_str;
+}
+
+TEST_F(SpvModuleScopeVarParserTest, InstanceIndex_I32_Load_CopyObject) {
+ const std::string assembly = InstanceIndexPreamble("%int") + R"(
+ %main = OpFunction %void None %voidfn
+ %entry = OpLabel
+ %copy_ptr = OpCopyObject %ptr_ty %1
+ %2 = OpLoad %int %copy_ptr
+ OpReturn
+ OpFunctionEnd
+ )";
+ auto p = parser(test::Assemble(assembly));
+ ASSERT_TRUE(p->BuildAndParseInternalModule()) << p->error() << assembly;
+ EXPECT_TRUE(p->error().empty());
+ const auto module_str = p->program().to_str();
+ // Correct declaration
+ EXPECT_THAT(module_str, HasSubstr(R"(
+ Variable{
+ Decorations{
+ BuiltinDecoration{instance_index}
+ }
+ x_1
+ in
+ __u32
+ })"));
+
+ // Correct body
+ EXPECT_THAT(module_str, HasSubstr(R"(
+ VariableDeclStatement{
+ VariableConst{
+ x_2
+ none
+ __i32
+ {
+ TypeConstructor[not set]{
+ __i32
+ Identifier[not set]{x_1}
+ }
+ }
+ }
+ })"))
+ << module_str;
+}
+
+TEST_F(SpvModuleScopeVarParserTest, InstanceIndex_I32_Load_AccessChain) {
+ const std::string assembly = InstanceIndexPreamble("%int") + R"(
+ %main = OpFunction %void None %voidfn
+ %entry = OpLabel
+ %copy_ptr = OpAccessChain %ptr_ty %1
+ %2 = OpLoad %int %copy_ptr
+ OpReturn
+ OpFunctionEnd
+ )";
+ auto p = parser(test::Assemble(assembly));
+ ASSERT_TRUE(p->BuildAndParseInternalModule()) << p->error() << assembly;
+ EXPECT_TRUE(p->error().empty());
+ const auto module_str = p->program().to_str();
+ // Correct declaration
+ EXPECT_THAT(module_str, HasSubstr(R"(
+ Variable{
+ Decorations{
+ BuiltinDecoration{instance_index}
+ }
+ x_1
+ in
+ __u32
+ })"));
+
+ // Correct body
+ EXPECT_THAT(module_str, HasSubstr(R"(
+ VariableDeclStatement{
+ VariableConst{
+ x_2
+ none
+ __i32
+ {
+ TypeConstructor[not set]{
+ __i32
+ Identifier[not set]{x_1}
+ }
+ }
+ }
+ })"))
+ << module_str;
+}
+
+TEST_F(SpvModuleScopeVarParserTest, InstanceIndex_I32_FunctParam) {
+ const std::string assembly = InstanceIndexPreamble("%int") + R"(
+ %helper_ty = OpTypeFunction %int %ptr_ty
+ %helper = OpFunction %int None %helper_ty
+ %param = OpFunctionParameter %ptr_ty
+ %helper_entry = OpLabel
+ %3 = OpLoad %int %param
+ OpReturnValue %3
+ OpFunctionEnd
+
+ %main = OpFunction %void None %voidfn
+ %entry = OpLabel
+ %result = OpFunctionCall %int %helper %1
+ OpReturn
+ OpFunctionEnd
+ )";
+ auto p = parser(test::Assemble(assembly));
+ // TODO(dneto): We can handle this if we make a shadow variable and mutate
+ // the parameter type.
+ ASSERT_FALSE(p->BuildAndParseInternalModule());
+ EXPECT_THAT(p->error(), HasSubstr("unhandled use of a pointer to the "
+ "InstanceIndex builtin, with ID: 1"));
+}
+
+TEST_F(SpvModuleScopeVarParserTest, InstanceIndex_U32_Load_Direct) {
+ const std::string assembly = InstanceIndexPreamble("%uint") + R"(
+ %main = OpFunction %void None %voidfn
+ %entry = OpLabel
+ %2 = OpLoad %uint %1
+ OpReturn
+ OpFunctionEnd
+ )";
+ auto p = parser(test::Assemble(assembly));
+ ASSERT_TRUE(p->BuildAndParseInternalModule()) << p->error() << assembly;
+ EXPECT_TRUE(p->error().empty());
+ const auto module_str = p->program().to_str();
+ // Correct declaration
+ EXPECT_THAT(module_str, HasSubstr(R"(
+ Variable{
+ Decorations{
+ BuiltinDecoration{instance_index}
+ }
+ x_1
+ in
+ __u32
+ })"));
+
+ // Correct body
+ EXPECT_THAT(module_str, HasSubstr(R"(
+ VariableDeclStatement{
+ VariableConst{
+ x_2
+ none
+ __u32
+ {
+ Identifier[not set]{x_1}
+ }
+ }
+ })"))
+ << module_str;
+}
+
+TEST_F(SpvModuleScopeVarParserTest, InstanceIndex_U32_Load_CopyObject) {
+ const std::string assembly = InstanceIndexPreamble("%uint") + R"(
+ %main = OpFunction %void None %voidfn
+ %entry = OpLabel
+ %copy_ptr = OpCopyObject %ptr_ty %1
+ %2 = OpLoad %uint %copy_ptr
+ OpReturn
+ OpFunctionEnd
+ )";
+ auto p = parser(test::Assemble(assembly));
+ ASSERT_TRUE(p->BuildAndParseInternalModule()) << p->error() << assembly;
+ EXPECT_TRUE(p->error().empty());
+ const auto module_str = p->program().to_str();
+ // Correct declaration
+ EXPECT_THAT(module_str, HasSubstr(R"(
+ Variable{
+ Decorations{
+ BuiltinDecoration{instance_index}
+ }
+ x_1
+ in
+ __u32
+ })"));
+
+ // Correct body
+ EXPECT_THAT(module_str, HasSubstr(R"(
+ VariableDeclStatement{
+ VariableConst{
+ x_11
+ none
+ __ptr_in__u32
+ {
+ Identifier[not set]{x_1}
+ }
+ }
+ }
+ VariableDeclStatement{
+ VariableConst{
+ x_2
+ none
+ __u32
+ {
+ Identifier[not set]{x_11}
+ }
+ }
+ })"))
+ << module_str;
+}
+
+TEST_F(SpvModuleScopeVarParserTest, InstanceIndex_U32_Load_AccessChain) {
+ const std::string assembly = InstanceIndexPreamble("%uint") + R"(
+ %main = OpFunction %void None %voidfn
+ %entry = OpLabel
+ %copy_ptr = OpAccessChain %ptr_ty %1
+ %2 = OpLoad %uint %copy_ptr
+ OpReturn
+ OpFunctionEnd
+ )";
+ auto p = parser(test::Assemble(assembly));
+ ASSERT_TRUE(p->BuildAndParseInternalModule()) << p->error() << assembly;
+ EXPECT_TRUE(p->error().empty());
+ const auto module_str = p->program().to_str();
+ // Correct declaration
+ EXPECT_THAT(module_str, HasSubstr(R"(
+ Variable{
+ Decorations{
+ BuiltinDecoration{instance_index}
+ }
+ x_1
+ in
+ __u32
+ })"));
+
+ // Correct body
+ EXPECT_THAT(module_str, HasSubstr(R"(
+ VariableDeclStatement{
+ VariableConst{
+ x_2
+ none
+ __u32
+ {
+ Identifier[not set]{x_1}
+ }
+ }
+ })"))
+ << module_str;
+}
+
+TEST_F(SpvModuleScopeVarParserTest, InstanceIndex_U32_FunctParam) {
+ const std::string assembly = InstanceIndexPreamble("%uint") + R"(
+ %helper_ty = OpTypeFunction %uint %ptr_ty
+ %helper = OpFunction %uint None %helper_ty
+ %param = OpFunctionParameter %ptr_ty
+ %helper_entry = OpLabel
+ %3 = OpLoad %uint %param
+ OpReturnValue %3
+ OpFunctionEnd
+
+ %main = OpFunction %void None %voidfn
+ %entry = OpLabel
+ %result = OpFunctionCall %uint %helper %1
+ OpReturn
+ OpFunctionEnd
+ )";
+ auto p = parser(test::Assemble(assembly));
+ // TODO(dneto): We can handle this if we make a shadow variable and mutate
+ // the parameter type.
+ ASSERT_TRUE(p->BuildAndParseInternalModule()) << p->error() << assembly;
+ EXPECT_TRUE(p->error().empty());
+ const auto module_str = p->program().to_str();
+ // Correct declaration
+ EXPECT_THAT(module_str, HasSubstr(R"(
+ Variable{
+ Decorations{
+ BuiltinDecoration{instance_index}
+ }
+ x_1
+ in
+ __u32
+ })"));
+
+ // Correct bodies
+ EXPECT_THAT(module_str, HasSubstr(R"(
+ Function x_11 -> __u32
+ (
+ VariableConst{
+ x_12
+ none
+ __ptr_in__u32
+ }
+ )
+ {
+ VariableDeclStatement{
+ VariableConst{
+ x_3
+ none
+ __u32
+ {
+ Identifier[not set]{x_12}
+ }
+ }
+ }
+ Return{
+ {
+ Identifier[not set]{x_3}
+ }
+ }
+ }
+ Function main -> __void
+ StageDecoration{vertex}
+ ()
+ {
+ VariableDeclStatement{
+ VariableConst{
+ x_15
+ none
+ __u32
+ {
+ Call[not set]{
+ Identifier[not set]{x_11}
+ (
+ Identifier[not set]{x_1}
+ )
+ }
+ }
+ }
+ }
+ Return{}
+ }
+})")) << module_str;
+}
+
// TODO(dneto): Test passing pointer to SampleMask as function parameter,
// both input case and output case.
