Tint/builder/spirv: Fix atomicCompareExchangeWeak wrong return
This CL fix the spv code generated for atomicCompareExchangeWeak,
specifically the emulated `exchanged` field of the returned struct.
It should be true if and only if the `old_value`, i.e. the return value
of spv `OpAtomicCompareExchange`, equals to the comparator, rather than
equals to new value.
Fixed: tint:1663
Change-Id: I1cb0de6bb6b90ae681f5053b8bdd6f6b247146f6
Reviewed-on: https://dawn-review.googlesource.com/c/dawn/+/107180
Reviewed-by: Ben Clayton <bclayton@google.com>
Kokoro: Kokoro <noreply+kokoro@google.com>
Commit-Queue: Zhaoming Jiang <zhaoming.jiang@intel.com>
diff --git a/src/tint/writer/spirv/builder.cc b/src/tint/writer/spirv/builder.cc
index 29f2f85..97a29f3 100644
--- a/src/tint/writer/spirv/builder.cc
+++ b/src/tint/writer/spirv/builder.cc
@@ -3238,13 +3238,19 @@
return false;
}
- // values_equal := original_value == value
+ // https://registry.khronos.org/SPIR-V/specs/unified1/SPIRV.html#OpAtomicCompareExchange
+ // According to SPIR-V spec, during the atomic steps of OpAtomicCompareExchange, the new
+ // value will be stored only if original value equals to comparator, and the result of
+ // OpAtomicCompareExchange is the original value. Therefore to check if the exchanging
+ // has been executed, we should compare the result original_value to comparator.
+
+ // values_equal := original_value == comparator
auto values_equal = result_op();
if (!push_function_inst(spv::Op::OpIEqual, {
Operand(bool_type),
values_equal,
original_value,
- value,
+ Operand(comparator),
})) {
return false;
}
diff --git a/src/tint/writer/spirv/builder_builtin_test.cc b/src/tint/writer/spirv/builder_builtin_test.cc
index dddc475..3c97da7 100644
--- a/src/tint/writer/spirv/builder_builtin_test.cc
+++ b/src/tint/writer/spirv/builder_builtin_test.cc
@@ -3622,11 +3622,11 @@
auto* expected_instructions = R"(%17 = OpAccessChain %16 %1 %14
%20 = OpAtomicCompareExchange %4 %17 %13 %14 %14 %18 %19
-%21 = OpIEqual %12 %20 %18
+%21 = OpIEqual %12 %20 %19
%10 = OpCompositeConstruct %11 %20 %21
%26 = OpAccessChain %25 %1 %13
%29 = OpAtomicCompareExchange %5 %26 %13 %14 %14 %27 %28
-%30 = OpIEqual %12 %29 %27
+%30 = OpIEqual %12 %29 %28
%22 = OpCompositeConstruct %23 %29 %30
OpReturn
)";
diff --git a/test/tint/bug/tint/1573.wgsl.expected.spvasm b/test/tint/bug/tint/1573.wgsl.expected.spvasm
index 8475726..c24a87f 100644
--- a/test/tint/bug/tint/1573.wgsl.expected.spvasm
+++ b/test/tint/bug/tint/1573.wgsl.expected.spvasm
@@ -42,7 +42,7 @@
%20 = OpAccessChain %_ptr_StorageBuffer_uint %a %uint_0
%21 = OpLoad %uint %value
%22 = OpAtomicCompareExchange %uint %20 %uint_1 %uint_0 %uint_0 %21 %12
- %23 = OpIEqual %bool %22 %21
+ %23 = OpIEqual %bool %22 %12
%13 = OpCompositeConstruct %__atomic_compare_exchange_resultu32 %22 %23
OpReturn
OpFunctionEnd
diff --git a/test/tint/bug/tint/1574.wgsl.expected.spvasm b/test/tint/bug/tint/1574.wgsl.expected.spvasm
index be3e7f0..79f74c6 100644
--- a/test/tint/bug/tint/1574.wgsl.expected.spvasm
+++ b/test/tint/bug/tint/1574.wgsl.expected.spvasm
@@ -94,59 +94,59 @@
%42 = OpAccessChain %_ptr_StorageBuffer_uint %a_u32 %uint_0
%43 = OpLoad %uint %value
%44 = OpAtomicCompareExchange %uint %42 %uint_1 %uint_0 %uint_0 %43 %29
- %45 = OpIEqual %bool %44 %43
+ %45 = OpIEqual %bool %44 %29
%38 = OpCompositeConstruct %__atomic_compare_exchange_resultu32 %44 %45
%48 = OpAccessChain %_ptr_StorageBuffer_uint %a_u32 %uint_0
%49 = OpLoad %uint %value
%50 = OpAtomicCompareExchange %uint %48 %uint_1 %uint_0 %uint_0 %49 %29
- %51 = OpIEqual %bool %50 %49
+ %51 = OpIEqual %bool %50 %29
%46 = OpCompositeConstruct %__atomic_compare_exchange_resultu32 %50 %51
%54 = OpAccessChain %_ptr_StorageBuffer_uint %a_u32 %uint_0
%55 = OpLoad %uint %value
%56 = OpAtomicCompareExchange %uint %54 %uint_1 %uint_0 %uint_0 %55 %29
- %57 = OpIEqual %bool %56 %55
+ %57 = OpIEqual %bool %56 %29
%52 = OpCompositeConstruct %__atomic_compare_exchange_resultu32 %56 %57
OpStore %value_0 %int_42
%65 = OpAccessChain %_ptr_StorageBuffer_int %a_i32 %uint_0
%66 = OpLoad %int %value_0
%67 = OpAtomicCompareExchange %int %65 %uint_1 %uint_0 %uint_0 %66 %32
- %68 = OpIEqual %bool %67 %66
+ %68 = OpIEqual %bool %67 %32
%61 = OpCompositeConstruct %__atomic_compare_exchange_resulti32 %67 %68
%71 = OpAccessChain %_ptr_StorageBuffer_int %a_i32 %uint_0
%72 = OpLoad %int %value_0
%73 = OpAtomicCompareExchange %int %71 %uint_1 %uint_0 %uint_0 %72 %32
- %74 = OpIEqual %bool %73 %72
+ %74 = OpIEqual %bool %73 %32
%69 = OpCompositeConstruct %__atomic_compare_exchange_resulti32 %73 %74
%77 = OpAccessChain %_ptr_StorageBuffer_int %a_i32 %uint_0
%78 = OpLoad %int %value_0
%79 = OpAtomicCompareExchange %int %77 %uint_1 %uint_0 %uint_0 %78 %32
- %80 = OpIEqual %bool %79 %78
+ %80 = OpIEqual %bool %79 %32
%75 = OpCompositeConstruct %__atomic_compare_exchange_resulti32 %79 %80
OpStore %value_1 %uint_42
%84 = OpLoad %uint %value_1
%85 = OpAtomicCompareExchange %uint %b_u32 %uint_2 %uint_0 %uint_0 %84 %29
- %86 = OpIEqual %bool %85 %84
+ %86 = OpIEqual %bool %85 %29
%82 = OpCompositeConstruct %__atomic_compare_exchange_resultu32 %85 %86
%89 = OpLoad %uint %value_1
%90 = OpAtomicCompareExchange %uint %b_u32 %uint_2 %uint_0 %uint_0 %89 %29
- %91 = OpIEqual %bool %90 %89
+ %91 = OpIEqual %bool %90 %29
%87 = OpCompositeConstruct %__atomic_compare_exchange_resultu32 %90 %91
%94 = OpLoad %uint %value_1
%95 = OpAtomicCompareExchange %uint %b_u32 %uint_2 %uint_0 %uint_0 %94 %29
- %96 = OpIEqual %bool %95 %94
+ %96 = OpIEqual %bool %95 %29
%92 = OpCompositeConstruct %__atomic_compare_exchange_resultu32 %95 %96
OpStore %value_2 %int_42
%100 = OpLoad %int %value_2
%101 = OpAtomicCompareExchange %int %b_i32 %uint_2 %uint_0 %uint_0 %100 %32
- %102 = OpIEqual %bool %101 %100
+ %102 = OpIEqual %bool %101 %32
%98 = OpCompositeConstruct %__atomic_compare_exchange_resulti32 %101 %102
%105 = OpLoad %int %value_2
%106 = OpAtomicCompareExchange %int %b_i32 %uint_2 %uint_0 %uint_0 %105 %32
- %107 = OpIEqual %bool %106 %105
+ %107 = OpIEqual %bool %106 %32
%103 = OpCompositeConstruct %__atomic_compare_exchange_resulti32 %106 %107
%110 = OpLoad %int %value_2
%111 = OpAtomicCompareExchange %int %b_i32 %uint_2 %uint_0 %uint_0 %110 %32
- %112 = OpIEqual %bool %111 %110
+ %112 = OpIEqual %bool %111 %32
%108 = OpCompositeConstruct %__atomic_compare_exchange_resulti32 %111 %112
OpReturn
OpFunctionEnd
diff --git a/test/tint/builtins/atomics/from_gen/var/atomicCompareExchangeWeak/storage_i32.spvasm.expected.spvasm b/test/tint/builtins/atomics/from_gen/var/atomicCompareExchangeWeak/storage_i32.spvasm.expected.spvasm
index 0c17de2..b7e5639 100644
--- a/test/tint/builtins/atomics/from_gen/var/atomicCompareExchangeWeak/storage_i32.spvasm.expected.spvasm
+++ b/test/tint/builtins/atomics/from_gen/var/atomicCompareExchangeWeak/storage_i32.spvasm.expected.spvasm
@@ -66,7 +66,7 @@
%20 = OpLoad %int %arg_1
%28 = OpAccessChain %_ptr_StorageBuffer_int %sb_rw %uint_0
%29 = OpAtomicCompareExchange %int %28 %uint_1 %uint_0 %uint_0 %19 %20
- %30 = OpIEqual %bool %29 %19
+ %30 = OpIEqual %bool %29 %20
%21 = OpCompositeConstruct %__atomic_compare_exchange_resulti32 %29 %30
%31 = OpCompositeExtract %int %21 0
%32 = OpIEqual %bool %31 %19
diff --git a/test/tint/builtins/atomics/from_gen/var/atomicCompareExchangeWeak/storage_u32.spvasm.expected.spvasm b/test/tint/builtins/atomics/from_gen/var/atomicCompareExchangeWeak/storage_u32.spvasm.expected.spvasm
index 279b70f..4ea5912 100644
--- a/test/tint/builtins/atomics/from_gen/var/atomicCompareExchangeWeak/storage_u32.spvasm.expected.spvasm
+++ b/test/tint/builtins/atomics/from_gen/var/atomicCompareExchangeWeak/storage_u32.spvasm.expected.spvasm
@@ -64,7 +64,7 @@
%20 = OpLoad %uint %arg_1
%26 = OpAccessChain %_ptr_StorageBuffer_uint %sb_rw %uint_0
%27 = OpAtomicCompareExchange %uint %26 %uint_1 %uint_0 %uint_0 %19 %20
- %28 = OpIEqual %bool %27 %19
+ %28 = OpIEqual %bool %27 %20
%21 = OpCompositeConstruct %__atomic_compare_exchange_resultu32 %27 %28
%29 = OpCompositeExtract %uint %21 0
%30 = OpIEqual %bool %29 %19
diff --git a/test/tint/builtins/atomics/from_gen/var/atomicCompareExchangeWeak/workgroup_i32.spvasm.expected.spvasm b/test/tint/builtins/atomics/from_gen/var/atomicCompareExchangeWeak/workgroup_i32.spvasm.expected.spvasm
index b6cb96d..2c9ca9f 100644
--- a/test/tint/builtins/atomics/from_gen/var/atomicCompareExchangeWeak/workgroup_i32.spvasm.expected.spvasm
+++ b/test/tint/builtins/atomics/from_gen/var/atomicCompareExchangeWeak/workgroup_i32.spvasm.expected.spvasm
@@ -67,7 +67,7 @@
%24 = OpLoad %int %arg_2
%25 = OpLoad %int %arg_1
%31 = OpAtomicCompareExchange %int %arg_0 %uint_2 %uint_0 %uint_0 %24 %25
- %32 = OpIEqual %bool %31 %24
+ %32 = OpIEqual %bool %31 %25
%26 = OpCompositeConstruct %__atomic_compare_exchange_resulti32 %31 %32
%33 = OpCompositeExtract %int %26 0
%34 = OpIEqual %bool %33 %24
diff --git a/test/tint/builtins/atomics/from_gen/var/atomicCompareExchangeWeak/workgroup_u32.spvasm.expected.spvasm b/test/tint/builtins/atomics/from_gen/var/atomicCompareExchangeWeak/workgroup_u32.spvasm.expected.spvasm
index 14223f7..89af02b 100644
--- a/test/tint/builtins/atomics/from_gen/var/atomicCompareExchangeWeak/workgroup_u32.spvasm.expected.spvasm
+++ b/test/tint/builtins/atomics/from_gen/var/atomicCompareExchangeWeak/workgroup_u32.spvasm.expected.spvasm
@@ -65,7 +65,7 @@
%22 = OpLoad %uint %arg_2
%23 = OpLoad %uint %arg_1
%29 = OpAtomicCompareExchange %uint %arg_0 %uint_2 %uint_0 %uint_0 %22 %23
- %30 = OpIEqual %bool %29 %22
+ %30 = OpIEqual %bool %29 %23
%24 = OpCompositeConstruct %__atomic_compare_exchange_resultu32 %29 %30
%31 = OpCompositeExtract %uint %24 0
%32 = OpIEqual %bool %31 %22
diff --git a/test/tint/builtins/gen/var/atomicCompareExchangeWeak/1bd40a.wgsl.expected.spvasm b/test/tint/builtins/gen/var/atomicCompareExchangeWeak/1bd40a.wgsl.expected.spvasm
index 0999969..5ae726d 100644
--- a/test/tint/builtins/gen/var/atomicCompareExchangeWeak/1bd40a.wgsl.expected.spvasm
+++ b/test/tint/builtins/gen/var/atomicCompareExchangeWeak/1bd40a.wgsl.expected.spvasm
@@ -55,7 +55,7 @@
%23 = OpLoad %int %arg_2
%24 = OpLoad %int %arg_1
%25 = OpAtomicCompareExchange %int %22 %uint_1 %uint_0 %uint_0 %23 %24
- %26 = OpIEqual %bool %25 %23
+ %26 = OpIEqual %bool %25 %24
%14 = OpCompositeConstruct %__atomic_compare_exchange_resulti32 %25 %26
OpStore %res %14
OpReturn
diff --git a/test/tint/builtins/gen/var/atomicCompareExchangeWeak/63d8e6.wgsl.expected.spvasm b/test/tint/builtins/gen/var/atomicCompareExchangeWeak/63d8e6.wgsl.expected.spvasm
index 3723643..57368c4 100644
--- a/test/tint/builtins/gen/var/atomicCompareExchangeWeak/63d8e6.wgsl.expected.spvasm
+++ b/test/tint/builtins/gen/var/atomicCompareExchangeWeak/63d8e6.wgsl.expected.spvasm
@@ -53,7 +53,7 @@
%21 = OpLoad %uint %arg_2
%22 = OpLoad %uint %arg_1
%23 = OpAtomicCompareExchange %uint %20 %uint_1 %uint_0 %uint_0 %21 %22
- %24 = OpIEqual %bool %23 %21
+ %24 = OpIEqual %bool %23 %22
%14 = OpCompositeConstruct %__atomic_compare_exchange_resultu32 %23 %24
OpStore %res %14
OpReturn
diff --git a/test/tint/builtins/gen/var/atomicCompareExchangeWeak/83580d.wgsl.expected.spvasm b/test/tint/builtins/gen/var/atomicCompareExchangeWeak/83580d.wgsl.expected.spvasm
index b163b6f..05cd15a 100644
--- a/test/tint/builtins/gen/var/atomicCompareExchangeWeak/83580d.wgsl.expected.spvasm
+++ b/test/tint/builtins/gen/var/atomicCompareExchangeWeak/83580d.wgsl.expected.spvasm
@@ -50,7 +50,7 @@
%21 = OpLoad %uint %arg_2
%22 = OpLoad %uint %arg_1
%23 = OpAtomicCompareExchange %uint %arg_0 %uint_2 %uint_0 %uint_0 %21 %22
- %24 = OpIEqual %bool %23 %21
+ %24 = OpIEqual %bool %23 %22
%15 = OpCompositeConstruct %__atomic_compare_exchange_resultu32 %23 %24
OpStore %res %15
OpReturn
diff --git a/test/tint/builtins/gen/var/atomicCompareExchangeWeak/e88938.wgsl.expected.spvasm b/test/tint/builtins/gen/var/atomicCompareExchangeWeak/e88938.wgsl.expected.spvasm
index b4e3102..531d468 100644
--- a/test/tint/builtins/gen/var/atomicCompareExchangeWeak/e88938.wgsl.expected.spvasm
+++ b/test/tint/builtins/gen/var/atomicCompareExchangeWeak/e88938.wgsl.expected.spvasm
@@ -51,7 +51,7 @@
%22 = OpLoad %int %arg_2
%23 = OpLoad %int %arg_1
%24 = OpAtomicCompareExchange %int %arg_0 %uint_2 %uint_0 %uint_0 %22 %23
- %25 = OpIEqual %bool %24 %22
+ %25 = OpIEqual %bool %24 %23
%16 = OpCompositeConstruct %__atomic_compare_exchange_resulti32 %24 %25
OpStore %res %16
OpReturn
