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>
This commit is contained in:
David Neto 2021-02-08 16:12:09 +00:00 committed by Commit Bot service account
parent 61306b882d
commit 7efea888fa
4 changed files with 432 additions and 6 deletions

View File

@ -2040,6 +2040,11 @@ TypedExpression FunctionEmitter::MakeExpression(uint32_t id) {
<< "unhandled use of a pointer to the VertexIndex builtin, with ID: " << "unhandled use of a pointer to the VertexIndex builtin, with ID: "
<< id; << id;
return {}; return {};
case SkipReason::kInstanceIndexBuiltinPointer:
Fail() << "unhandled use of a pointer to the InstanceIndex builtin, with "
"ID: "
<< id;
return {};
case SkipReason::kSampleMaskInBuiltinPointer: case SkipReason::kSampleMaskInBuiltinPointer:
Fail() Fail()
<< "unhandled use of a pointer to the SampleMask builtin, with ID: " << "unhandled use of a pointer to the SampleMask builtin, with ID: "
@ -3048,13 +3053,15 @@ bool FunctionEmitter::EmitStatement(const spvtools::opt::Instruction& inst) {
SkipReason::kPointSizeBuiltinValue; SkipReason::kPointSizeBuiltinValue;
return true; return true;
case SkipReason::kSampleIdBuiltinPointer: case SkipReason::kSampleIdBuiltinPointer:
case SkipReason::kVertexIndexBuiltinPointer: { case SkipReason::kVertexIndexBuiltinPointer:
case SkipReason::kInstanceIndexBuiltinPointer: {
// The SPIR-V variable is i32, but WGSL requires u32. // The SPIR-V variable is i32, but WGSL requires u32.
auto var_id = parser_impl_.IdForSpecialBuiltIn( auto name = NameForSpecialInputBuiltin(skip_reason);
(skip_reason == SkipReason::kSampleIdBuiltinPointer) if (name.empty()) {
? SpvBuiltInSampleId return Fail() << "internal error: unhandled special input builtin "
: SpvBuiltInVertexIndex); "variable: "
auto name = namer_.Name(var_id); << inst.PrettyPrint();
}
ast::Expression* id_expr = create<ast::IdentifierExpression>( ast::Expression* id_expr = create<ast::IdentifierExpression>(
Source{}, builder_.Symbols().Register(name)); Source{}, builder_.Symbols().Register(name));
auto expr = TypedExpression{ auto expr = TypedExpression{
@ -3133,6 +3140,28 @@ bool FunctionEmitter::EmitStatement(const spvtools::opt::Instruction& inst) {
<< inst.PrettyPrint(); << 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( TypedExpression FunctionEmitter::MakeOperand(
const spvtools::opt::Instruction& inst, const spvtools::opt::Instruction& inst,
uint32_t operand_index) { uint32_t operand_index) {
@ -3725,6 +3754,9 @@ bool FunctionEmitter::RegisterSpecialBuiltInVariables() {
case SpvBuiltInVertexIndex: case SpvBuiltInVertexIndex:
def->skip = SkipReason::kVertexIndexBuiltinPointer; def->skip = SkipReason::kVertexIndexBuiltinPointer;
break; break;
case SpvBuiltInInstanceIndex:
def->skip = SkipReason::kInstanceIndexBuiltinPointer;
break;
case SpvBuiltInSampleMask: { case SpvBuiltInSampleMask: {
// Distinguish between input and output variable. // Distinguish between input and output variable.
const auto storage_class = const auto storage_class =

View File

@ -233,6 +233,11 @@ enum class SkipReason {
/// builtin variable. Don't generate its address. /// builtin variable. Don't generate its address.
kVertexIndexBuiltinPointer, 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 /// `kSampleMaskInBuiltinPointer`: the value is a pointer to the SampleMaskIn
/// builtin input variable. Don't generate its address. /// builtin input variable. Don't generate its address.
kSampleMaskInBuiltinPointer, kSampleMaskInBuiltinPointer,
@ -351,6 +356,9 @@ inline std::ostream& operator<<(std::ostream& o, const DefInfo& di) {
case SkipReason::kVertexIndexBuiltinPointer: case SkipReason::kVertexIndexBuiltinPointer:
o << " skip:vertexindex_pointer"; o << " skip:vertexindex_pointer";
break; break;
case SkipReason::kInstanceIndexBuiltinPointer:
o << " skip:instanceindex_pointer";
break;
case SkipReason::kSampleMaskInBuiltinPointer: case SkipReason::kSampleMaskInBuiltinPointer:
o << " skip:samplemaskin_pointer"; o << " skip:samplemaskin_pointer";
break; break;
@ -762,6 +770,13 @@ class FunctionEmitter {
return SkipReason::kDontSkip; 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 /// Returns the most deeply nested structured construct which encloses the
/// WGSL scopes of names declared in both block positions. Each position must /// WGSL scopes of names declared in both block positions. Each position must
/// be a valid index into the function block order array. /// be a valid index into the function block order array.

View File

@ -1294,6 +1294,7 @@ ast::Variable* ParserImpl::MakeVariable(
return nullptr; return nullptr;
case SpvBuiltInSampleId: case SpvBuiltInSampleId:
case SpvBuiltInVertexIndex: case SpvBuiltInVertexIndex:
case SpvBuiltInInstanceIndex:
// The SPIR-V variable is likely to be signed (because GLSL // The SPIR-V variable is likely to be signed (because GLSL
// requires signed), but WGSL requires unsigned. Handle specially // requires signed), but WGSL requires unsigned. Handle specially
// so we always perform the conversion at load and store. // so we always perform the conversion at load and store.

View File

@ -3349,6 +3349,384 @@ TEST_F(SpvModuleScopeVarParserTest, VertexIndex_U32_FunctParam) {
})")) << module_str; })")) << 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, // TODO(dneto): Test passing pointer to SampleMask as function parameter,
// both input case and output case. // both input case and output case.