diff --git a/src/reader/spirv/function.cc b/src/reader/spirv/function.cc index 9cde282c7c..c1be6f18f5 100644 --- a/src/reader/spirv/function.cc +++ b/src/reader/spirv/function.cc @@ -2309,20 +2309,6 @@ TypedExpression FunctionEmitter::MakeExpression(uint32_t id) { Fail() << "unhandled use of a pointer to the PointSize builtin, with ID: " << id; return {}; - case SkipReason::kSampleIdBuiltinPointer: - Fail() << "unhandled use of a pointer to the SampleId builtin, with ID: " - << id; - return {}; - case SkipReason::kVertexIndexBuiltinPointer: - Fail() - << "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: " @@ -3423,22 +3409,6 @@ bool FunctionEmitter::EmitStatement(const spvtools::opt::Instruction& inst) { GetDefInfo(inst.result_id())->skip = SkipReason::kPointSizeBuiltinValue; return true; - case SkipReason::kSampleIdBuiltinPointer: - case SkipReason::kVertexIndexBuiltinPointer: - case SkipReason::kInstanceIndexBuiltinPointer: { - auto name = NameForSpecialInputBuiltin(skip_reason); - if (name.empty()) { - return Fail() << "internal error: unhandled special input builtin " - "variable: " - << inst.PrettyPrint(); - } - ast::Expression* id_expr = create( - Source{}, builder_.Symbols().Register(name)); - - auto* loaded_type = parser_impl_.ConvertType(inst.type_id()); - auto expr = TypedExpression{loaded_type, id_expr}; - return EmitConstDefinition(inst, expr); - } case SkipReason::kSampleMaskInBuiltinPointer: { auto name = namer_.Name(sample_mask_in_id); ast::Expression* id_expr = create( @@ -3573,28 +3543,6 @@ bool FunctionEmitter::EmitStatement(const spvtools::opt::Instruction& inst) { << 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) { @@ -4279,15 +4227,6 @@ bool FunctionEmitter::RegisterSpecialBuiltInVariables() { case SpvBuiltInPointSize: def->skip = SkipReason::kPointSizeBuiltinPointer; break; - case SpvBuiltInSampleId: - def->skip = SkipReason::kSampleIdBuiltinPointer; - break; - 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 = @@ -4301,6 +4240,9 @@ bool FunctionEmitter::RegisterSpecialBuiltInVariables() { } break; } + case SpvBuiltInSampleId: + case SpvBuiltInInstanceIndex: + case SpvBuiltInVertexIndex: case SpvBuiltInLocalInvocationIndex: case SpvBuiltInLocalInvocationId: case SpvBuiltInGlobalInvocationId: diff --git a/src/reader/spirv/function.h b/src/reader/spirv/function.h index 944afd54a3..72a357f5a2 100644 --- a/src/reader/spirv/function.h +++ b/src/reader/spirv/function.h @@ -214,19 +214,6 @@ enum class SkipReason { /// supported by WebGPU. kPointSizeBuiltinValue, - /// `kSampleIdBuiltinPointer`: the value is a pointer to the SampleId builtin - /// variable. Don't generate its address. - kSampleIdBuiltinPointer, - - /// `kVertexIndexBuiltinPointer`: the value is a pointer to the VertexIndex - /// 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, @@ -339,15 +326,6 @@ inline std::ostream& operator<<(std::ostream& o, const DefInfo& di) { case SkipReason::kPointSizeBuiltinValue: o << " skip:pointsize_value"; break; - case SkipReason::kSampleIdBuiltinPointer: - o << " skip:sampleid_pointer"; - break; - case SkipReason::kVertexIndexBuiltinPointer: - o << " skip:vertexindex_pointer"; - break; - case SkipReason::kInstanceIndexBuiltinPointer: - o << " skip:instanceindex_pointer"; - break; case SkipReason::kSampleMaskInBuiltinPointer: o << " skip:samplemaskin_pointer"; break; @@ -795,13 +773,6 @@ class FunctionEmitter { 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_module_var_test.cc b/src/reader/spirv/parser_impl_module_var_test.cc index c4f3679d2e..c32fb8f105 100644 --- a/src/reader/spirv/parser_impl_module_var_test.cc +++ b/src/reader/spirv/parser_impl_module_var_test.cc @@ -2443,18 +2443,31 @@ TEST_F(SpvModuleScopeVarParserTest, SampleId_I32_Load_CopyObject) { 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"( + const std::string expected = + R"(Module{ Variable{ x_1 private undefined __i32 - })")) < __void + () + { + VariableDeclStatement{ + VariableConst{ + x_11 + none + undefined + __ptr_none__i32 + { + UnaryOp[not set]{ + address-of + Identifier[not set]{x_1} + } + } + } + } VariableDeclStatement{ VariableConst{ x_2 @@ -2462,14 +2475,15 @@ TEST_F(SpvModuleScopeVarParserTest, SampleId_I32_Load_CopyObject) { undefined __i32 { - Identifier[not set]{x_1} + UnaryOp[not set]{ + indirection + Identifier[not set]{x_11} + } } } - })")) - << module_str; - - // Correct parameter on entry point - EXPECT_THAT(module_str, HasSubstr(R"( + } + Return{} + } Function main -> __void StageDecoration{fragment} ( @@ -2489,8 +2503,15 @@ TEST_F(SpvModuleScopeVarParserTest, SampleId_I32_Load_CopyObject) { Bitcast[not set]<__i32>{ Identifier[not set]{x_1_param} } - })")) - << module_str; + } + Call[not set]{ + Identifier[not set]{main_1} + ( + ) + } + } +} +)"; } TEST_F(SpvModuleScopeVarParserTest, SampleId_I32_Load_AccessChain) { @@ -2668,6 +2689,20 @@ TEST_F(SpvModuleScopeVarParserTest, SampleId_U32_Load_CopyObject) { Function main_1 -> __void () { + VariableDeclStatement{ + VariableConst{ + x_11 + none + undefined + __ptr_none__u32 + { + UnaryOp[not set]{ + address-of + Identifier[not set]{x_1} + } + } + } + } VariableDeclStatement{ VariableConst{ x_2 @@ -2675,7 +2710,10 @@ TEST_F(SpvModuleScopeVarParserTest, SampleId_U32_Load_CopyObject) { undefined __u32 { - Identifier[not set]{x_1} + UnaryOp[not set]{ + indirection + Identifier[not set]{x_11} + } } } } @@ -2944,18 +2982,16 @@ TEST_F(SpvModuleScopeVarParserTest, SampleMask_In_U32_CopyObject) { 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"( + const std::string expected = R"(Module{ Variable{ x_1 private undefined __array__u32_1 - })")) < __void + () + { VariableDeclStatement{ VariableConst{ x_4 @@ -2969,10 +3005,9 @@ TEST_F(SpvModuleScopeVarParserTest, SampleMask_In_U32_CopyObject) { } } } - })")); - - // Correct parameter on entry point - EXPECT_THAT(module_str, HasSubstr(R"( + } + Return{} + } Function main -> __void StageDecoration{fragment} ( @@ -2993,8 +3028,16 @@ TEST_F(SpvModuleScopeVarParserTest, SampleMask_In_U32_CopyObject) { ScalarConstructor[not set]{0} } Identifier[not set]{x_1_param} - })")) - << module_str; + } + Call[not set]{ + Identifier[not set]{main_1} + ( + ) + } + } +} +)"; + EXPECT_EQ(module_str, expected) << module_str; } TEST_F(SpvModuleScopeVarParserTest, SampleMask_In_U32_AccessChain) { @@ -3955,6 +3998,20 @@ TEST_F(SpvModuleScopeVarParserTest, VertexIndex_I32_Load_CopyObject) { Function main_1 -> __void () { + VariableDeclStatement{ + VariableConst{ + x_11 + none + undefined + __ptr_none__i32 + { + UnaryOp[not set]{ + address-of + Identifier[not set]{x_1} + } + } + } + } VariableDeclStatement{ VariableConst{ x_2 @@ -3962,7 +4019,10 @@ TEST_F(SpvModuleScopeVarParserTest, VertexIndex_I32_Load_CopyObject) { undefined __i32 { - Identifier[not set]{x_1} + UnaryOp[not set]{ + indirection + Identifier[not set]{x_11} + } } } } @@ -4066,34 +4126,6 @@ TEST_F(SpvModuleScopeVarParserTest, VertexIndex_I32_Load_AccessChain) { EXPECT_EQ(module_str, expected); } -TEST_F(SpvModuleScopeVarParserTest, VertexIndex_I32_FunctParam) { - // TODO(dneto): Passing by pointer-to-input is not allowed. - // Remove this test. - const std::string assembly = VertexIndexPreamble("%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 VertexIndex builtin, with ID: 1")); -} - TEST_F(SpvModuleScopeVarParserTest, VertexIndex_U32_Load_Direct) { const std::string assembly = VertexIndexPreamble("%uint") + R"( %main = OpFunction %void None %voidfn @@ -4181,6 +4213,20 @@ TEST_F(SpvModuleScopeVarParserTest, VertexIndex_U32_Load_CopyObject) { Function main_1 -> __void () { + VariableDeclStatement{ + VariableConst{ + x_11 + none + undefined + __ptr_none__u32 + { + UnaryOp[not set]{ + address-of + Identifier[not set]{x_1} + } + } + } + } VariableDeclStatement{ VariableConst{ x_2 @@ -4188,7 +4234,10 @@ TEST_F(SpvModuleScopeVarParserTest, VertexIndex_U32_Load_CopyObject) { undefined __u32 { - Identifier[not set]{x_1} + UnaryOp[not set]{ + indirection + Identifier[not set]{x_11} + } } } } @@ -4421,6 +4470,20 @@ TEST_F(SpvModuleScopeVarParserTest, InstanceIndex_I32_Load_CopyObject) { Function main_1 -> __void () { + VariableDeclStatement{ + VariableConst{ + x_11 + none + undefined + __ptr_none__i32 + { + UnaryOp[not set]{ + address-of + Identifier[not set]{x_1} + } + } + } + } VariableDeclStatement{ VariableConst{ x_2 @@ -4428,7 +4491,10 @@ TEST_F(SpvModuleScopeVarParserTest, InstanceIndex_I32_Load_CopyObject) { undefined __i32 { - Identifier[not set]{x_1} + UnaryOp[not set]{ + indirection + Identifier[not set]{x_11} + } } } } @@ -4643,6 +4709,20 @@ TEST_F(SpvModuleScopeVarParserTest, InstanceIndex_U32_Load_CopyObject) { Function main_1 -> __void () { + VariableDeclStatement{ + VariableConst{ + x_11 + none + undefined + __ptr_none__u32 + { + UnaryOp[not set]{ + address-of + Identifier[not set]{x_1} + } + } + } + } VariableDeclStatement{ VariableConst{ x_2 @@ -4650,7 +4730,10 @@ TEST_F(SpvModuleScopeVarParserTest, InstanceIndex_U32_Load_CopyObject) { undefined __u32 { - Identifier[not set]{x_1} + UnaryOp[not set]{ + indirection + Identifier[not set]{x_11} + } } } }