From 875d116a873624816f02c3fdfc1b6b60d82efcd1 Mon Sep 17 00:00:00 2001 From: Antonio Maiorano Date: Mon, 5 Dec 2022 17:16:15 +0000 Subject: [PATCH] tint: fix signed overflow in const eval modulo Bug: chromium:1395241 Bug: tint:1581 Change-Id: I6f6084749d9bc5c0d493476b251eeec5543d8621 Reviewed-on: https://dawn-review.googlesource.com/c/dawn/+/112701 Reviewed-by: Ben Clayton Kokoro: Kokoro Auto-Submit: Antonio Maiorano Commit-Queue: Ben Clayton --- src/tint/number.h | 7 ++++- src/tint/number_test.cc | 30 +++++++++++++++---- test/tint/bug/chromium/1395241.wgsl | 4 +++ .../chromium/1395241.wgsl.expected.dxc.hlsl | 8 +++++ .../chromium/1395241.wgsl.expected.fxc.hlsl | 8 +++++ .../bug/chromium/1395241.wgsl.expected.glsl | 10 +++++++ .../bug/chromium/1395241.wgsl.expected.msl | 7 +++++ .../bug/chromium/1395241.wgsl.expected.spvasm | 28 +++++++++++++++++ .../bug/chromium/1395241.wgsl.expected.wgsl | 3 ++ 9 files changed, 99 insertions(+), 6 deletions(-) create mode 100644 test/tint/bug/chromium/1395241.wgsl create mode 100644 test/tint/bug/chromium/1395241.wgsl.expected.dxc.hlsl create mode 100644 test/tint/bug/chromium/1395241.wgsl.expected.fxc.hlsl create mode 100644 test/tint/bug/chromium/1395241.wgsl.expected.glsl create mode 100644 test/tint/bug/chromium/1395241.wgsl.expected.msl create mode 100644 test/tint/bug/chromium/1395241.wgsl.expected.spvasm create mode 100644 test/tint/bug/chromium/1395241.wgsl.expected.wgsl diff --git a/src/tint/number.h b/src/tint/number.h index 9b7ce43405..975ce79dce 100644 --- a/src/tint/number.h +++ b/src/tint/number.h @@ -547,7 +547,12 @@ namespace detail { /// @returns the remainder of e1 / e2 template inline T Mod(T e1, T e2) { - return e1 - e2 * static_cast(std::trunc(e1 / e2)); + if constexpr (IsIntegral) { + return e1 % e2; + + } else { + return e1 - e2 * std::trunc(e1 / e2); + } } } // namespace detail diff --git a/src/tint/number_test.cc b/src/tint/number_test.cc index 040fd76703..a795840dbd 100644 --- a/src/tint/number_test.cc +++ b/src/tint/number_test.cc @@ -704,6 +704,15 @@ INSTANTIATE_TEST_SUITE_P( {AInt(2), AInt(10), AInt(4)}, {AInt(0), AInt::Highest(), AInt::Highest()}, {AInt(0), AInt::Lowest(), AInt::Lowest()}, + {AInt(2), AInt::Highest(), AInt(5)}, + {AInt(1), AInt::Highest(), AInt(6)}, + {AInt(0), AInt::Highest(), AInt(7)}, + {-AInt{1}, -AInt{10}, AInt{3}}, + {-AInt{2}, -AInt{10}, AInt{4}}, + {AInt{1}, AInt{10}, -AInt{3}}, + {AInt{2}, AInt{10}, -AInt{4}}, + {-AInt{1}, -AInt{10}, -AInt{3}}, + {-AInt{2}, -AInt{10}, -AInt{4}}, {OVERFLOW, AInt::Highest(), AInt(0)}, {OVERFLOW, AInt::Lowest(), AInt(0)}, //////////////////////////////////////////////////////////////////////// @@ -725,11 +734,22 @@ TEST_P(CheckedModTest_Float, Test) { template std::vector CheckedModTest_FloatCases() { return { - {T(0.5), T(10.5), T(1)}, {T(0.5), T(10.5), T(2)}, - {T(1.5), T(10.5), T(3)}, {T(2.5), T(10.5), T(4)}, - {T(0.5), T(10.5), T(5)}, {T(0), T::Highest(), T::Highest()}, - {T(0), T::Lowest(), T::Lowest()}, {Overflow, T(123), T(0)}, - {Overflow, T(123), T(-0)}, {Overflow, T(-123), T(0)}, + {T(0.5), T(10.5), T(1)}, // + {T(0.5), T(10.5), T(2)}, // + {T(1.5), T(10.5), T(3)}, // + {T(2.5), T(10.5), T(4)}, // + {T(0.5), T(10.5), T(5)}, // + {T(0), T::Highest(), T::Highest()}, // + {T(0), T::Lowest(), T::Lowest()}, // + {-T{1}, -T{10}, T{3}}, // + {-T{2}, -T{10}, T{4}}, // + {T{1}, T{10}, -T{3}}, // + {T{2}, T{10}, -T{4}}, // + {-T{1}, -T{10}, -T{3}}, // + {-T{2}, -T{10}, -T{4}}, // + {Overflow, T(123), T(0)}, // + {Overflow, T(123), T(-0)}, // + {Overflow, T(-123), T(0)}, // {Overflow, T(-123), T(-0)}, }; } diff --git a/test/tint/bug/chromium/1395241.wgsl b/test/tint/bug/chromium/1395241.wgsl new file mode 100644 index 0000000000..31f8f6af08 --- /dev/null +++ b/test/tint/bug/chromium/1395241.wgsl @@ -0,0 +1,4 @@ +fn fr6snorm() { + var bzbxttch=~~~~~~~~~~~~ ~~~~~-~~~~~~~~~~~~~~~~~~~~~ ~~~-~~~~~~~~~~~~~~~~~~~~~~~~~~~9223372036854775807% ( +5); +} diff --git a/test/tint/bug/chromium/1395241.wgsl.expected.dxc.hlsl b/test/tint/bug/chromium/1395241.wgsl.expected.dxc.hlsl new file mode 100644 index 0000000000..dadbf687f9 --- /dev/null +++ b/test/tint/bug/chromium/1395241.wgsl.expected.dxc.hlsl @@ -0,0 +1,8 @@ +[numthreads(1, 1, 1)] +void unused_entry_point() { + return; +} + +void fr6snorm() { + int bzbxttch = 2; +} diff --git a/test/tint/bug/chromium/1395241.wgsl.expected.fxc.hlsl b/test/tint/bug/chromium/1395241.wgsl.expected.fxc.hlsl new file mode 100644 index 0000000000..dadbf687f9 --- /dev/null +++ b/test/tint/bug/chromium/1395241.wgsl.expected.fxc.hlsl @@ -0,0 +1,8 @@ +[numthreads(1, 1, 1)] +void unused_entry_point() { + return; +} + +void fr6snorm() { + int bzbxttch = 2; +} diff --git a/test/tint/bug/chromium/1395241.wgsl.expected.glsl b/test/tint/bug/chromium/1395241.wgsl.expected.glsl new file mode 100644 index 0000000000..d26c89ea6a --- /dev/null +++ b/test/tint/bug/chromium/1395241.wgsl.expected.glsl @@ -0,0 +1,10 @@ +#version 310 es + +layout(local_size_x = 1, local_size_y = 1, local_size_z = 1) in; +void unused_entry_point() { + return; +} +void fr6snorm() { + int bzbxttch = 2; +} + diff --git a/test/tint/bug/chromium/1395241.wgsl.expected.msl b/test/tint/bug/chromium/1395241.wgsl.expected.msl new file mode 100644 index 0000000000..19e242e7b3 --- /dev/null +++ b/test/tint/bug/chromium/1395241.wgsl.expected.msl @@ -0,0 +1,7 @@ +#include + +using namespace metal; +void fr6snorm() { + int bzbxttch = 2; +} + diff --git a/test/tint/bug/chromium/1395241.wgsl.expected.spvasm b/test/tint/bug/chromium/1395241.wgsl.expected.spvasm new file mode 100644 index 0000000000..5ca875d1c6 --- /dev/null +++ b/test/tint/bug/chromium/1395241.wgsl.expected.spvasm @@ -0,0 +1,28 @@ +; SPIR-V +; Version: 1.3 +; Generator: Google Tint Compiler; 0 +; Bound: 12 +; Schema: 0 + OpCapability Shader + OpMemoryModel Logical GLSL450 + OpEntryPoint GLCompute %unused_entry_point "unused_entry_point" + OpExecutionMode %unused_entry_point LocalSize 1 1 1 + OpName %unused_entry_point "unused_entry_point" + OpName %fr6snorm "fr6snorm" + OpName %bzbxttch "bzbxttch" + %void = OpTypeVoid + %1 = OpTypeFunction %void + %int = OpTypeInt 32 1 + %int_2 = OpConstant %int 2 +%_ptr_Function_int = OpTypePointer Function %int + %11 = OpConstantNull %int +%unused_entry_point = OpFunction %void None %1 + %4 = OpLabel + OpReturn + OpFunctionEnd + %fr6snorm = OpFunction %void None %1 + %6 = OpLabel + %bzbxttch = OpVariable %_ptr_Function_int Function %11 + OpStore %bzbxttch %int_2 + OpReturn + OpFunctionEnd diff --git a/test/tint/bug/chromium/1395241.wgsl.expected.wgsl b/test/tint/bug/chromium/1395241.wgsl.expected.wgsl new file mode 100644 index 0000000000..702ffa1539 --- /dev/null +++ b/test/tint/bug/chromium/1395241.wgsl.expected.wgsl @@ -0,0 +1,3 @@ +fn fr6snorm() { + var bzbxttch = (~(~(~(~(~(~(~(~(~(~(~(~(~(~(~(~(~(-(~(~(~(~(~(~(~(~(~(~(~(~(~(~(~(~(~(~(~(~(~(~(~(~(-(~(~(~(~(~(~(~(~(~(~(~(~(~(~(~(~(~(~(~(~(~(~(~(~(~(~(~(9223372036854775807)))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))) % 5); +}