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