From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from [87.239.111.99] (localhost [127.0.0.1]) by dev.tarantool.org (Postfix) with ESMTP id 704E66A8C65; Tue, 24 Oct 2023 13:18:54 +0300 (MSK) DKIM-Filter: OpenDKIM Filter v2.11.0 dev.tarantool.org 704E66A8C65 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=tarantool.org; s=dev; t=1698142734; bh=B3uPD3Mge9WlWegML1L/AEPBOj9l3eIgIVqvixXt18U=; h=To:Date:Subject:List-Id:List-Unsubscribe:List-Archive:List-Post: List-Help:List-Subscribe:From:Reply-To:Cc:From; b=EhO+aVYRhUsX1CT0io8V8dhpMOhULODpOPGoLec+OWuy3cE9rT4wDz6PXZbQErrj3 k04dEny4bW44vWlHT5M64l8nhsOvD2wnyeDazxMvT09i9WOLHoGaVTleOqQJJA0Luu IIubpiirGX2V8pc/ysTlPGJk0OnWAtE1fcuIPQaE= Received: from smtpng1.i.mail.ru (smtpng1.i.mail.ru [94.100.181.251]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by dev.tarantool.org (Postfix) with ESMTPS id E64315D3046 for ; Tue, 24 Oct 2023 13:18:51 +0300 (MSK) DKIM-Filter: OpenDKIM Filter v2.11.0 dev.tarantool.org E64315D3046 Received: by smtpng1.m.smailru.net with esmtpa (envelope-from ) id 1qvEUs-0002PL-Nm; Tue, 24 Oct 2023 13:18:51 +0300 To: Maxim Kokryashkin , Sergey Bronnikov Date: Tue, 24 Oct 2023 13:14:20 +0300 Message-ID: <20231024101420.12398-1-skaplun@tarantool.org> X-Mailer: git-send-email 2.42.0 MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Mailru-Src: smtp X-7564579A: 78E4E2B564C1792B X-77F55803: 4F1203BC0FB41BD9D4445358DD2C13E2729A38C26529782B5331BB66BE396ADE182A05F538085040EFFF2942630E56AFD4B6FFAAB18C68E31AFEA8BD95DF199CF5829A8A883D2536 X-7FA49CB5: FF5795518A3D127A4AD6D5ED66289B5278DA827A17800CE776199E130E4EDC53EA1F7E6F0F101C67BD4B6F7A4D31EC0BCC500DACC3FED6E28638F802B75D45FF8AA50765F79006372A285F85D463FE138638F802B75D45FF36EB9D2243A4F8B5A6FCA7DBDB1FC311F39EFFDF887939037866D6147AF826D8821C8B10EC260B56BC3FD7125763646E117882F4460429724CE54428C33FAD305F5C1EE8F4F765FCECADA55FE5B58BB7A471835C12D1D9774AD6D5ED66289B52BA9C0B312567BB23117882F44604297287769387670735201E561CDFBCA1751F2CC0D3CB04F14752D2E47CDBA5A96583BA9C0B312567BB231DD303D21008E29813377AFFFEAFD269176DF2183F8FC7C0406C186E56A1B26068655334FD4449CB33AC447995A7AD1857739F23D657EF2BD5E8D9A59859A8B67122B551D37F75FD089D37D7C0E48F6C5571747095F342E88FB05168BE4CE3AF X-C1DE0DAB: 0D63561A33F958A5A48DBA49DCF510645C8E1E3357777F0592019D26E3D48CA6F87CCE6106E1FC07E67D4AC08A07B9B013BDA61BF53F5E1D9C5DF10A05D560A950611B66E3DA6D700B0A020F03D25A0997E3FB2386030E77 X-C8649E89: 1C3962B70DF3F0ADE00A9FD3E00BEEDF77DD89D51EBB7742D3581295AF09D3DF87807E0823442EA2ED31085941D9CD0AF7F820E7B07EA4CF77321689EE807C947C016336FFF03C51AE4DCC5AD55AD4FCCC0E41D11EC7857E60849098BFF6585AA789783558ABFE9C7E036575F27607EBF30B997157C94F1FA74DFFEFA5DC0E7F02C26D483E81D6BE5EF9655DD6DEA7D65774BB76CC95456EEC5B5AD62611EEC62B5AFB4261A09AF0 X-D57D3AED: 3ZO7eAau8CL7WIMRKs4sN3D3tLDjz0dLbV79QFUyzQ2Ujvy7cMT6pYYqY16iZVKkSc3dCLJ7zSJH7+u4VD18S7Vl4ZUrpaVfd2+vE6kuoey4m4VkSEu530nj6fImhcD4MUrOEAnl0W826KZ9Q+tr5ycPtXkTV4k65bRjmOUUP8cvGozZ33TWg5HZplvhhXbhDGzqmQDTd6OAevLeAnq3Ra9uf7zvY2zzsIhlcp/Y7m53TZgf2aB4JOg4gkr2biojbL9S8ysBdXiQ5HClorPV2ytald3mppVe X-DA7885C5: 49812C93162F5324A497B2563580F13D3D7F19D2B85D8E7FA73A85B637A22F3F262E2D401490A4A0DB037EFA58388B346E8BC1A9835FDE71 X-Mailru-Sender: 689FA8AB762F73930F533AC2B33E986BAD5744D03EF3EE60E7E2C9AEA7D6DCF20FBE9A32752B8C9C2AA642CC12EC09F1FB559BB5D741EB962F61BD320559CF1EFD657A8799238ED55FEEDEB644C299C0ED14614B50AE0675 X-Mras: Ok Subject: [Tarantool-patches] [PATCH luajit] IR_MIN/IR_MAX is non-commutative due to underlying FPU ops. X-BeenThere: tarantool-patches@dev.tarantool.org X-Mailman-Version: 2.1.34 Precedence: list List-Id: Tarantool development patches List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , From: Sergey Kaplun via Tarantool-patches Reply-To: Sergey Kaplun Cc: tarantool-patches@dev.tarantool.org Errors-To: tarantool-patches-bounces@dev.tarantool.org Sender: "Tarantool-patches" From: Mike Pall Thanks to Peter Cawley. (cherry-picked from commit 7a2b83a0c5d980bf3db0aeda33c79e7bb4b3da01) Even after the commit c05d103305da0626e025dbf81370ca9f4f788c83 ("Fix math.min()/math.max() inconsistencies."), some of the corner cases (see tests in the commit for details) for `math.min()`/`math.max()` still inconsistent in the JIT and the VM. This happens because `IR_MIN` and `IR_MAX` are marked as commutative, so their operands were swapped by `asm_swapops()`. As a result, because `minsd`[1]/`maxsd`[2] instructions don't change the sign byte of the destination register, its sign is preserved if we compare 0 with -0. When we compare something with NaN, the second source operand (either a NaN or a valid floating-point value) is written to the result. Hence, swapping the operands changed the resulting value. This patch removes the commutative flag from the aforementioned IRs to prevent swapping of their operands. [1]: https://c9x.me/x86/html/file_module_x86_id_173.html [2]: https://c9x.me/x86/html/file_module_x86_id_168.html Sergey Kaplun: * added the description and the test for the problem Part of tarantool/tarantool#9145 --- Branch: https://github.com/tarantool/luajit/tree/skaplun/lj-1082-min-max-0-commutative Tarantool PR: https://github.com/tarantool/tarantool/pull/9294 Related Issues: * https://github.com/tarantool/tarantool/issues/9145 * https://github.com/LuaJIT/LuaJIT/issues/1082 * https://github.com/LuaJIT/LuaJIT/issues/957 src/lj_ir.h | 4 +- test/tarantool-tests/gh-6163-min-max.test.lua | 23 ++++------- .../lj-1082-min-max-0-commutative.test.lua | 41 +++++++++++++++++++ 3 files changed, 51 insertions(+), 17 deletions(-) create mode 100644 test/tarantool-tests/lj-1082-min-max-0-commutative.test.lua diff --git a/src/lj_ir.h b/src/lj_ir.h index 46af54e4..53e9dfd6 100644 --- a/src/lj_ir.h +++ b/src/lj_ir.h @@ -76,8 +76,8 @@ \ _(ABS, N , ref, ref) \ _(LDEXP, N , ref, ref) \ - _(MIN, C , ref, ref) \ - _(MAX, C , ref, ref) \ + _(MIN, N , ref, ref) \ + _(MAX, N , ref, ref) \ _(FPMATH, N , ref, lit) \ \ /* Overflow-checking arithmetic ops. */ \ diff --git a/test/tarantool-tests/gh-6163-min-max.test.lua b/test/tarantool-tests/gh-6163-min-max.test.lua index 4bc6155c..f52cb5ca 100644 --- a/test/tarantool-tests/gh-6163-min-max.test.lua +++ b/test/tarantool-tests/gh-6163-min-max.test.lua @@ -3,11 +3,6 @@ local test = tap.test('gh-6163-jit-min-max'):skipcond({ ['Test requires JIT enabled'] = not jit.status(), }) -local x86_64 = jit.arch == 'x86' or jit.arch == 'x64' --- XXX: table to use for dummy check for some inconsistent results --- on the x86/64 architecture. -local DUMMY_TAB = {} - test:plan(18) -- -- gh-6163: math.min/math.max inconsistencies. @@ -110,17 +105,17 @@ result = {} for k = 1, 4 do result[k] = min(x, min(x, nan)) end --- FIXME: results are still inconsistent for the x86/64 architecture. +-- See also https://github.com/LuaJIT/LuaJIT/issues/957. -- expected: nan nan nan nan -test:samevalues(x86_64 and DUMMY_TAB or result, 'math.min: comm_dup_minmax') +test:samevalues(result, 'math.min: comm_dup_minmax') result = {} for k = 1, 4 do result[k] = max(x, max(x, nan)) end --- FIXME: results are still inconsistent for the x86/64 architecture. +-- See also https://github.com/LuaJIT/LuaJIT/issues/957. -- expected: nan nan nan nan -test:samevalues(x86_64 and DUMMY_TAB or result, 'math.max: comm_dup_minmax') +test:samevalues(result, 'math.max: comm_dup_minmax') -- The following optimization should be disabled: -- (x o k1) o k2 ==> x o (k1 o k2) @@ -179,10 +174,9 @@ result = {} for k = 1, 4 do result[k] = min(1, max(1, nan)) end --- FIXME: results are still inconsistent for the x86/64 architecture. +-- See also https://github.com/LuaJIT/LuaJIT/issues/957. -- expected: nan nan nan nan -test:samevalues(x86_64 and DUMMY_TAB or result, - 'min-max-case2: reassoc_minmax_right') +test:samevalues(result, 'min-max-case2: reassoc_minmax_right') result = {} for k = 1, 4 do @@ -195,10 +189,9 @@ result = {} for k = 1, 4 do result[k] = max(1, min(1, nan)) end --- FIXME: results are still inconsistent for the x86/64 architecture. +-- See also https://github.com/LuaJIT/LuaJIT/issues/957. -- expected: nan nan nan nan -test:samevalues(x86_64 and DUMMY_TAB or result, - 'max-min-case2: reassoc_minmax_right') +test:samevalues(result, 'max-min-case2: reassoc_minmax_right') -- XXX: If we look into the disassembled code of `lj_vm_foldarith()` -- we can see the following: diff --git a/test/tarantool-tests/lj-1082-min-max-0-commutative.test.lua b/test/tarantool-tests/lj-1082-min-max-0-commutative.test.lua new file mode 100644 index 00000000..99bfa298 --- /dev/null +++ b/test/tarantool-tests/lj-1082-min-max-0-commutative.test.lua @@ -0,0 +1,41 @@ +local tap = require('tap') + +-- Test file to demonstrate LuaJIT inconsistent behaviour for +-- `math.min()` and `math.max()` operations when comparing 0 with +-- -0 in the JIT and the VM. +-- See also: https://github.com/LuaJIT/LuaJIT/issues/1082. + +local test = tap.test('lj-1082-min-max-0-commutative'):skipcond({ + ['Test requires JIT enabled'] = not jit.status(), +}) + +test:plan(4) + +-- XXX: simplify `jit.dump()` output. +local min, max = math.min, math.max + +-- Use local variables to prevent fold optimization. +local minus_zero = -0 +local zero = 0 + +local results_min_pm = {} +local results_min_mp = {} +local results_max_mp = {} +local results_max_pm = {} + +jit.opt.start('hotloop=1') +for i = 1, 4 do + -- The resulting value is the second parameter for comparison. + -- Use `tostring()` to distinguish 0 and -0. + results_min_pm[i] = tostring(min(zero, minus_zero)) + results_min_mp[i] = tostring(min(minus_zero, zero)) + results_max_pm[i] = tostring(max(zero, minus_zero)) + results_max_mp[i] = tostring(max(minus_zero, zero)) +end + +test:samevalues(results_min_pm, 'min(0, -0)') +test:samevalues(results_min_mp, 'min(-0, 0)') +test:samevalues(results_max_pm, 'max(0, -0)') +test:samevalues(results_max_mp, 'max(-0, 0)') + +test:done(true) -- 2.42.0