writer/spirv: Fix abs() on unsigned integers
GLSLstd450SAbs expects a *signed* integer.
abs() of an unsigned number is now a no-op.
Fixes WebGPU CTS tests:
webgpu:shader,execution,robust_access_vertex:vertex_buffer_access:*
Bug: tint:1194
Change-Id: I65c5e9f2f03aac0b788b9ba88c383cbec136d7c6
Reviewed-on: https://dawn-review.googlesource.com/c/tint/+/65620
Commit-Queue: Ben Clayton <bclayton@chromium.org>
Kokoro: Ben Clayton <bclayton@chromium.org>
Reviewed-by: David Neto <dneto@google.com>
diff --git a/docs/origin-trial-changes.md b/docs/origin-trial-changes.md
index 6da54e0..5b9e504 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 33d64bd..cba0438 100644
--- a/src/writer/spirv/builder.cc
+++ b/src/writer/spirv/builder.cc
@@ -124,12 +124,6 @@
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 @@
};
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 @@
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 fe7608d..2a11dfd 100644
--- a/src/writer/spirv/builder_intrinsic_test.cc
+++ b/src/writer/spirv/builder_intrinsic_test.cc
@@ -1133,12 +1133,10 @@
Intrinsic_Builtin_SingleParam_Sint_Test,
testing::Values(IntrinsicData{"abs", "SAbs"}));
-using Intrinsic_Builtin_SingleParam_Uint_Test =
- IntrinsicBuilderTestWithParam<IntrinsicData>;
-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 @@
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<u32>(1u, 1u));
+TEST_F(Intrinsic_Builtin_Abs_Uint_Test, Call_Vector) {
+ auto* expr = Call("abs", vec2<u32>(1u, 1u));
WrapInFunction(expr);
auto* func = Func("a_func", ast::VariableList{}, ty.void_(),
@@ -1177,26 +1170,20 @@
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<IntrinsicData>;
diff --git a/test/intrinsics/gen/abs/1ce782.wgsl.expected.spvasm b/test/intrinsics/gen/abs/1ce782.wgsl.expected.spvasm
index 793b4cc..f39c38c 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 d11d39e..b5c27c3 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 066ce7b..3891964 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 c840c80..caed8a0 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