diff --git a/docs/origin-trial-changes.md b/docs/origin-trial-changes.md index 6da54e07f7..5b9e5049ae 100644 --- a/docs/origin-trial-changes.md +++ b/docs/origin-trial-changes.md @@ -3,11 +3,12 @@ ## Changes for M95 ### New Features + * The size of an array can now be defined using a non-overridable module-scope constant * The `num_workgroups` builtin is now supported. ### Fixes -* Hex floats: issue an error when the magnitude is non-zero, and the exponent would cause - overflow. https://crbug.com/tint/1150 https://crbug.com/tint/1166 -* Reject identifiers beginning with an underscore. https://crbug.com/tint/1179 +* Hex floats: now correctly errors when the magnitude is non-zero, and the exponent would cause overflow. [tint:1150](https://crbug.com/tint/1150), [tint:1166](https://crbug.com/tint/1166) +* Identifers beginning with an underscore are now correctly rejected. [tint:1179](https://crbug.com/tint/1179) +* `abs()` fixed for unsigned integers on SPIR-V backend [tint:1179](https://crbug.com/tint/1194) diff --git a/src/writer/spirv/builder.cc b/src/writer/spirv/builder.cc index 33d64bd8c8..cba04382c6 100644 --- a/src/writer/spirv/builder.cc +++ b/src/writer/spirv/builder.cc @@ -124,12 +124,6 @@ const sem::Matrix* GetNestedMatrixType(const sem::Type* type) { uint32_t intrinsic_to_glsl_method(const sem::Intrinsic* intrinsic) { switch (intrinsic->Type()) { - case IntrinsicType::kAbs: - if (intrinsic->ReturnType()->is_float_scalar_or_vector()) { - return GLSLstd450FAbs; - } else { - return GLSLstd450SAbs; - } case IntrinsicType::kAcos: return GLSLstd450Acos; case IntrinsicType::kAsin: @@ -2338,8 +2332,17 @@ uint32_t Builder::GenerateIntrinsic(ast::CallExpression* call, }; OperandList params = {Operand::Int(result_type_id), result}; - spv::Op op = spv::Op::OpNop; + + // Pushes the parameters for a GlslStd450 extended instruction, and sets op + // to OpExtInst. + auto glsl_std450 = [&](uint32_t inst_id) { + auto set_id = GetGLSLstd450Import(); + params.push_back(Operand::Int(set_id)); + params.push_back(Operand::Int(inst_id)); + op = spv::Op::OpExtInst; + }; + switch (intrinsic->Type()) { case IntrinsicType::kAny: op = spv::Op::OpAny; @@ -2611,18 +2614,25 @@ uint32_t Builder::GenerateIntrinsic(ast::CallExpression* call, case IntrinsicType::kTranspose: op = spv::Op::OpTranspose; break; + case IntrinsicType::kAbs: + if (intrinsic->ReturnType()->is_unsigned_scalar_or_vector()) { + // abs() only operates on *signed* integers. + // This is a no-op for unsigned integers. + return get_param_as_value_id(0); + } + if (intrinsic->ReturnType()->is_float_scalar_or_vector()) { + glsl_std450(GLSLstd450FAbs); + } else { + glsl_std450(GLSLstd450SAbs); + } + break; default: { - auto set_id = GetGLSLstd450Import(); auto inst_id = intrinsic_to_glsl_method(intrinsic); if (inst_id == 0) { error_ = "unknown method " + std::string(intrinsic->str()); return 0; } - - params.push_back(Operand::Int(set_id)); - params.push_back(Operand::Int(inst_id)); - - op = spv::Op::OpExtInst; + glsl_std450(inst_id); break; } } diff --git a/src/writer/spirv/builder_intrinsic_test.cc b/src/writer/spirv/builder_intrinsic_test.cc index fe7608d72e..2a11dfd603 100644 --- a/src/writer/spirv/builder_intrinsic_test.cc +++ b/src/writer/spirv/builder_intrinsic_test.cc @@ -1133,12 +1133,10 @@ INSTANTIATE_TEST_SUITE_P(IntrinsicBuilderTest, Intrinsic_Builtin_SingleParam_Sint_Test, testing::Values(IntrinsicData{"abs", "SAbs"})); -using Intrinsic_Builtin_SingleParam_Uint_Test = - IntrinsicBuilderTestWithParam; -TEST_P(Intrinsic_Builtin_SingleParam_Uint_Test, Call_Scalar) { - auto param = GetParam(); - - auto* expr = Call(param.name, 1u); +// Calling abs() on an unsigned integer scalar / vector is a no-op. +using Intrinsic_Builtin_Abs_Uint_Test = IntrinsicBuilderTest; +TEST_F(Intrinsic_Builtin_Abs_Uint_Test, Call_Scalar) { + auto* expr = Call("abs", 1u); WrapInFunction(expr); auto* func = Func("a_func", ast::VariableList{}, ty.void_(), @@ -1148,26 +1146,21 @@ TEST_P(Intrinsic_Builtin_SingleParam_Uint_Test, Call_Scalar) { ASSERT_TRUE(b.GenerateFunction(func)) << b.error(); - EXPECT_EQ(b.GenerateCallExpression(expr), 5u) << b.error(); - EXPECT_EQ(DumpBuilder(b), R"(%7 = OpExtInstImport "GLSL.std.450" -OpName %3 "a_func" + EXPECT_EQ(b.GenerateCallExpression(expr), 7u) << b.error(); + EXPECT_EQ(DumpBuilder(b), R"(OpName %3 "a_func" %2 = OpTypeVoid %1 = OpTypeFunction %2 %6 = OpTypeInt 32 0 -%8 = OpConstant %6 1 +%7 = OpConstant %6 1 %3 = OpFunction %2 None %1 %4 = OpLabel -%5 = OpExtInst %6 %7 )" + param.op + - R"( %8 OpReturn OpFunctionEnd )"); } -TEST_P(Intrinsic_Builtin_SingleParam_Uint_Test, Call_Vector) { - auto param = GetParam(); - - auto* expr = Call(param.name, vec2(1u, 1u)); +TEST_F(Intrinsic_Builtin_Abs_Uint_Test, Call_Vector) { + auto* expr = Call("abs", vec2(1u, 1u)); WrapInFunction(expr); auto* func = Func("a_func", ast::VariableList{}, ty.void_(), @@ -1177,26 +1170,20 @@ TEST_P(Intrinsic_Builtin_SingleParam_Uint_Test, Call_Vector) { ASSERT_TRUE(b.GenerateFunction(func)) << b.error(); - EXPECT_EQ(b.GenerateCallExpression(expr), 5u) << b.error(); - EXPECT_EQ(DumpBuilder(b), R"(%8 = OpExtInstImport "GLSL.std.450" -OpName %3 "a_func" + EXPECT_EQ(b.GenerateCallExpression(expr), 9u) << b.error(); + EXPECT_EQ(DumpBuilder(b), R"(OpName %3 "a_func" %2 = OpTypeVoid %1 = OpTypeFunction %2 %7 = OpTypeInt 32 0 %6 = OpTypeVector %7 2 -%9 = OpConstant %7 1 -%10 = OpConstantComposite %6 %9 %9 +%8 = OpConstant %7 1 +%9 = OpConstantComposite %6 %8 %8 %3 = OpFunction %2 None %1 %4 = OpLabel -%5 = OpExtInst %6 %8 )" + param.op + - R"( %10 OpReturn OpFunctionEnd )"); } -INSTANTIATE_TEST_SUITE_P(IntrinsicBuilderTest, - Intrinsic_Builtin_SingleParam_Uint_Test, - testing::Values(IntrinsicData{"abs", "SAbs"})); using Intrinsic_Builtin_DualParam_SInt_Test = IntrinsicBuilderTestWithParam; diff --git a/test/intrinsics/gen/abs/1ce782.wgsl.expected.spvasm b/test/intrinsics/gen/abs/1ce782.wgsl.expected.spvasm index 793b4ccf0c..f39c38cd8f 100644 --- a/test/intrinsics/gen/abs/1ce782.wgsl.expected.spvasm +++ b/test/intrinsics/gen/abs/1ce782.wgsl.expected.spvasm @@ -1,10 +1,9 @@ ; SPIR-V ; Version: 1.3 ; Generator: Google Tint Compiler; 0 -; Bound: 34 +; Bound: 33 ; Schema: 0 OpCapability Shader - %16 = OpExtInstImport "GLSL.std.450" OpMemoryModel Logical GLSL450 OpEntryPoint Vertex %vertex_main "vertex_main" %value %vertex_point_size OpEntryPoint Fragment %fragment_main "fragment_main" @@ -33,36 +32,35 @@ %9 = OpTypeFunction %void %uint = OpTypeInt 32 0 %v4uint = OpTypeVector %uint 4 - %17 = OpConstantNull %v4uint + %16 = OpConstantNull %v4uint %_ptr_Function_v4uint = OpTypePointer Function %v4uint - %20 = OpTypeFunction %v4float + %19 = OpTypeFunction %v4float %float_1 = OpConstant %float 1 %abs_1ce782 = OpFunction %void None %9 %12 = OpLabel - %res = OpVariable %_ptr_Function_v4uint Function %17 - %13 = OpExtInst %v4uint %16 SAbs %17 - OpStore %res %13 + %res = OpVariable %_ptr_Function_v4uint Function %16 + OpStore %res %16 OpReturn OpFunctionEnd -%vertex_main_inner = OpFunction %v4float None %20 - %22 = OpLabel - %23 = OpFunctionCall %void %abs_1ce782 +%vertex_main_inner = OpFunction %v4float None %19 + %21 = OpLabel + %22 = OpFunctionCall %void %abs_1ce782 OpReturnValue %5 OpFunctionEnd %vertex_main = OpFunction %void None %9 - %25 = OpLabel - %26 = OpFunctionCall %v4float %vertex_main_inner - OpStore %value %26 + %24 = OpLabel + %25 = OpFunctionCall %v4float %vertex_main_inner + OpStore %value %25 OpStore %vertex_point_size %float_1 OpReturn OpFunctionEnd %fragment_main = OpFunction %void None %9 - %29 = OpLabel - %30 = OpFunctionCall %void %abs_1ce782 + %28 = OpLabel + %29 = OpFunctionCall %void %abs_1ce782 OpReturn OpFunctionEnd %compute_main = OpFunction %void None %9 - %32 = OpLabel - %33 = OpFunctionCall %void %abs_1ce782 + %31 = OpLabel + %32 = OpFunctionCall %void %abs_1ce782 OpReturn OpFunctionEnd diff --git a/test/intrinsics/gen/abs/467cd1.wgsl.expected.spvasm b/test/intrinsics/gen/abs/467cd1.wgsl.expected.spvasm index d11d39efc8..b5c27c3551 100644 --- a/test/intrinsics/gen/abs/467cd1.wgsl.expected.spvasm +++ b/test/intrinsics/gen/abs/467cd1.wgsl.expected.spvasm @@ -1,10 +1,9 @@ ; SPIR-V ; Version: 1.3 ; Generator: Google Tint Compiler; 0 -; Bound: 34 +; Bound: 33 ; Schema: 0 OpCapability Shader - %15 = OpExtInstImport "GLSL.std.450" OpMemoryModel Logical GLSL450 OpEntryPoint Vertex %vertex_main "vertex_main" %value %vertex_point_size OpEntryPoint Fragment %fragment_main "fragment_main" @@ -34,35 +33,34 @@ %uint = OpTypeInt 32 0 %uint_1 = OpConstant %uint 1 %_ptr_Function_uint = OpTypePointer Function %uint - %19 = OpConstantNull %uint - %20 = OpTypeFunction %v4float + %18 = OpConstantNull %uint + %19 = OpTypeFunction %v4float %float_1 = OpConstant %float 1 %abs_467cd1 = OpFunction %void None %9 %12 = OpLabel - %res = OpVariable %_ptr_Function_uint Function %19 - %13 = OpExtInst %uint %15 SAbs %uint_1 - OpStore %res %13 + %res = OpVariable %_ptr_Function_uint Function %18 + OpStore %res %uint_1 OpReturn OpFunctionEnd -%vertex_main_inner = OpFunction %v4float None %20 - %22 = OpLabel - %23 = OpFunctionCall %void %abs_467cd1 +%vertex_main_inner = OpFunction %v4float None %19 + %21 = OpLabel + %22 = OpFunctionCall %void %abs_467cd1 OpReturnValue %5 OpFunctionEnd %vertex_main = OpFunction %void None %9 - %25 = OpLabel - %26 = OpFunctionCall %v4float %vertex_main_inner - OpStore %value %26 + %24 = OpLabel + %25 = OpFunctionCall %v4float %vertex_main_inner + OpStore %value %25 OpStore %vertex_point_size %float_1 OpReturn OpFunctionEnd %fragment_main = OpFunction %void None %9 - %29 = OpLabel - %30 = OpFunctionCall %void %abs_467cd1 + %28 = OpLabel + %29 = OpFunctionCall %void %abs_467cd1 OpReturn OpFunctionEnd %compute_main = OpFunction %void None %9 - %32 = OpLabel - %33 = OpFunctionCall %void %abs_467cd1 + %31 = OpLabel + %32 = OpFunctionCall %void %abs_467cd1 OpReturn OpFunctionEnd diff --git a/test/intrinsics/gen/abs/7326de.wgsl.expected.spvasm b/test/intrinsics/gen/abs/7326de.wgsl.expected.spvasm index 066ce7b48c..3891964afc 100644 --- a/test/intrinsics/gen/abs/7326de.wgsl.expected.spvasm +++ b/test/intrinsics/gen/abs/7326de.wgsl.expected.spvasm @@ -1,10 +1,9 @@ ; SPIR-V ; Version: 1.3 ; Generator: Google Tint Compiler; 0 -; Bound: 34 +; Bound: 33 ; Schema: 0 OpCapability Shader - %16 = OpExtInstImport "GLSL.std.450" OpMemoryModel Logical GLSL450 OpEntryPoint Vertex %vertex_main "vertex_main" %value %vertex_point_size OpEntryPoint Fragment %fragment_main "fragment_main" @@ -33,36 +32,35 @@ %9 = OpTypeFunction %void %uint = OpTypeInt 32 0 %v3uint = OpTypeVector %uint 3 - %17 = OpConstantNull %v3uint + %16 = OpConstantNull %v3uint %_ptr_Function_v3uint = OpTypePointer Function %v3uint - %20 = OpTypeFunction %v4float + %19 = OpTypeFunction %v4float %float_1 = OpConstant %float 1 %abs_7326de = OpFunction %void None %9 %12 = OpLabel - %res = OpVariable %_ptr_Function_v3uint Function %17 - %13 = OpExtInst %v3uint %16 SAbs %17 - OpStore %res %13 + %res = OpVariable %_ptr_Function_v3uint Function %16 + OpStore %res %16 OpReturn OpFunctionEnd -%vertex_main_inner = OpFunction %v4float None %20 - %22 = OpLabel - %23 = OpFunctionCall %void %abs_7326de +%vertex_main_inner = OpFunction %v4float None %19 + %21 = OpLabel + %22 = OpFunctionCall %void %abs_7326de OpReturnValue %5 OpFunctionEnd %vertex_main = OpFunction %void None %9 - %25 = OpLabel - %26 = OpFunctionCall %v4float %vertex_main_inner - OpStore %value %26 + %24 = OpLabel + %25 = OpFunctionCall %v4float %vertex_main_inner + OpStore %value %25 OpStore %vertex_point_size %float_1 OpReturn OpFunctionEnd %fragment_main = OpFunction %void None %9 - %29 = OpLabel - %30 = OpFunctionCall %void %abs_7326de + %28 = OpLabel + %29 = OpFunctionCall %void %abs_7326de OpReturn OpFunctionEnd %compute_main = OpFunction %void None %9 - %32 = OpLabel - %33 = OpFunctionCall %void %abs_7326de + %31 = OpLabel + %32 = OpFunctionCall %void %abs_7326de OpReturn OpFunctionEnd diff --git a/test/intrinsics/gen/abs/7f28e6.wgsl.expected.spvasm b/test/intrinsics/gen/abs/7f28e6.wgsl.expected.spvasm index c840c80092..caed8a0c52 100644 --- a/test/intrinsics/gen/abs/7f28e6.wgsl.expected.spvasm +++ b/test/intrinsics/gen/abs/7f28e6.wgsl.expected.spvasm @@ -1,10 +1,9 @@ ; SPIR-V ; Version: 1.3 ; Generator: Google Tint Compiler; 0 -; Bound: 34 +; Bound: 33 ; Schema: 0 OpCapability Shader - %16 = OpExtInstImport "GLSL.std.450" OpMemoryModel Logical GLSL450 OpEntryPoint Vertex %vertex_main "vertex_main" %value %vertex_point_size OpEntryPoint Fragment %fragment_main "fragment_main" @@ -33,36 +32,35 @@ %9 = OpTypeFunction %void %uint = OpTypeInt 32 0 %v2uint = OpTypeVector %uint 2 - %17 = OpConstantNull %v2uint + %16 = OpConstantNull %v2uint %_ptr_Function_v2uint = OpTypePointer Function %v2uint - %20 = OpTypeFunction %v4float + %19 = OpTypeFunction %v4float %float_1 = OpConstant %float 1 %abs_7f28e6 = OpFunction %void None %9 %12 = OpLabel - %res = OpVariable %_ptr_Function_v2uint Function %17 - %13 = OpExtInst %v2uint %16 SAbs %17 - OpStore %res %13 + %res = OpVariable %_ptr_Function_v2uint Function %16 + OpStore %res %16 OpReturn OpFunctionEnd -%vertex_main_inner = OpFunction %v4float None %20 - %22 = OpLabel - %23 = OpFunctionCall %void %abs_7f28e6 +%vertex_main_inner = OpFunction %v4float None %19 + %21 = OpLabel + %22 = OpFunctionCall %void %abs_7f28e6 OpReturnValue %5 OpFunctionEnd %vertex_main = OpFunction %void None %9 - %25 = OpLabel - %26 = OpFunctionCall %v4float %vertex_main_inner - OpStore %value %26 + %24 = OpLabel + %25 = OpFunctionCall %v4float %vertex_main_inner + OpStore %value %25 OpStore %vertex_point_size %float_1 OpReturn OpFunctionEnd %fragment_main = OpFunction %void None %9 - %29 = OpLabel - %30 = OpFunctionCall %void %abs_7f28e6 + %28 = OpLabel + %29 = OpFunctionCall %void %abs_7f28e6 OpReturn OpFunctionEnd %compute_main = OpFunction %void None %9 - %32 = OpLabel - %33 = OpFunctionCall %void %abs_7f28e6 + %31 = OpLabel + %32 = OpFunctionCall %void %abs_7f28e6 OpReturn OpFunctionEnd