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 0720582888B; Thu, 2 Nov 2023 15:40:51 +0300 (MSK) DKIM-Filter: OpenDKIM Filter v2.11.0 dev.tarantool.org 0720582888B DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=tarantool.org; s=dev; t=1698928851; bh=wz6Z44yCozIUiOEptbNZOi4/QumVffmL/sOtsj6Wk/Q=; h=To:Date:Subject:List-Id:List-Unsubscribe:List-Archive:List-Post: List-Help:List-Subscribe:From:Reply-To:Cc:From; b=CUwyAQ3RdpidBHMyDHcdmwIZf2W8RXck8gNuthyOgC+3ROwPSO8Tic3o6J2OnZssS gI6LRBI4dwE56EFCbVcBGBKZEijYnQ2pCVXiPI5QNhyDWMjMslDbiCZcqKJ/GsUR5K E7IjvLNqFcaxVVE82FPaS/OejV+giClU1kqQIfEU= 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 96892660901 for ; Thu, 2 Nov 2023 15:40:49 +0300 (MSK) DKIM-Filter: OpenDKIM Filter v2.11.0 dev.tarantool.org 96892660901 Received: by smtpng1.m.smailru.net with esmtpa (envelope-from ) id 1qyX0C-0007T8-CO; Thu, 02 Nov 2023 15:40:48 +0300 To: Maxim Kokryashkin , Sergey Bronnikov Date: Thu, 2 Nov 2023 15:36:16 +0300 Message-ID: <20231102123616.10527-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: 646B95376F6C166E X-77F55803: 4F1203BC0FB41BD9C931D0C4A91E7130F333D6F3C33D48C9F7A892AE3F7E098900894C459B0CD1B91E400F071F7B1127186804085BBB23E234958104EB922F7CBB3D1907BEA9FCEE X-7FA49CB5: FF5795518A3D127A4AD6D5ED66289B5278DA827A17800CE7646B74825E00C605EA1F7E6F0F101C67BD4B6F7A4D31EC0BCC500DACC3FED6E28638F802B75D45FF8AA50765F7900637835928C62272F24E8638F802B75D45FF36EB9D2243A4F8B5A6FCA7DBDB1FC311F39EFFDF887939037866D6147AF826D81B6407581E854C070655CC0F3E5CE610117882F4460429724CE54428C33FAD305F5C1EE8F4F765FC9FC99A4BA45EE8B4A471835C12D1D9774AD6D5ED66289B52BA9C0B312567BB23117882F4460429728776938767073520B1593CA6EC85F86DCB629EEF1311BF91D2E47CDBA5A96583BA9C0B312567BB231DD303D21008E29813377AFFFEAFD269176DF2183F8FC7C0406C186E56A1B26068655334FD4449CB9ECD01F8117BC8BEAAAE862A0553A39223F8577A6DFFEA7CB04B26703878C8DC43847C11F186F3C59DAA53EE0834AAEE X-C1DE0DAB: 0D63561A33F958A57F98637BE9A3BE0D902BE19F214969AC69A3A6FBC1DDD8A1F87CCE6106E1FC07E67D4AC08A07B9B065B78C30F681404D9C5DF10A05D560A950611B66E3DA6D700B0A020F03D25A092FFDA4F57982C5F4CB5012B2E24CD356 X-C8649E89: 1C3962B70DF3F0ADE00A9FD3E00BEEDF77DD89D51EBB7742D3581295AF09D3DF87807E0823442EA2ED31085941D9CD0AF7F820E7B07EA4CFF41715D3F36E03E4F2B61D84B34714B25CA999770EFF8E2726146AAAE88E51FD8135B9CF208C7BB7D8551B728A548A99777F7998086A1A36AA8E0926E36BA3D6A74DFFEFA5DC0E7F02C26D483E81D6BE5EF9655DD6DEA7D65774BB76CC95456EEC5B5AD62611EEC62B5AFB4261A09AF0 X-D57D3AED: 3ZO7eAau8CL7WIMRKs4sN3D3tLDjz0dLbV79QFUyzQ2Ujvy7cMT6pYYqY16iZVKkSc3dCLJ7zSJH7+u4VD18S7Vl4ZUrpaVfd2+vE6kuoey4m4VkSEu530nj6fImhcD4MUrOEAnl0W826KZ9Q+tr5ycPtXkTV4k65bRjmOUUP8cvGozZ33TWg5HZplvhhXbhDGzqmQDTd6OAevLeAnq3Ra9uf7zvY2zzsIhlcp/Y7m53TZgf2aB4JOg4gkr2biojgXFjEIoBlhB3S+soUDFsTg== X-DA7885C5: 5E43F917B0C728282D3CEBEDC662C5B66CCBAAD36E3D9D60946DF2E392898830262E2D401490A4A0DB037EFA58388B346E8BC1A9835FDE71 X-Mailru-Sender: 689FA8AB762F73930F533AC2B33E986B4294C93429AB368D1051B2E886E20CFF0FBE9A32752B8C9C2AA642CC12EC09F1FB559BB5D741EB962F61BD320559CF1EFD657A8799238ED55FEEDEB644C299C0ED14614B50AE0675 X-Mras: Ok Subject: [Tarantool-patches] [PATCH luajit] x86/x64: Fix math.ceil(-0.9) result sign. 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 Reported by minoki. (cherry-picked from commit 674afcd4e21d0cf64de3219d347557a0aed8ecc7) The `ceil` (`floor`) math function implementation calculates (|x| + 2^52) - 2^52 for its argument to determine the fractional part of x , so it will be rounded to the nearest integer and its sign is restored. After that, if the original value is < (>) than the result, the -1 (1) is subtracted from it. Take a look at the `ceil()` case. The result of the operation `-1 - (-1)` is +0 for FP arithmetic, against -0 expected as a result. This patch changes the `- (-1)` operation to `+ 1` and restores sign after it again. NB: Since in DUALNUM mode on x86/x64 all results are tried to be converted to integers the sign of 0 is neglected. 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-859-math-ceil-sign Tarantool PR: https://github.com/tarantool/tarantool/pull/9326 Related issues: * https://github.com/LuaJIT/LuaJIT/issues/859 * https://github.com/tarantool/tarantool/issues/9145 src/vm_x64.dasc | 13 ++++++------ src/vm_x86.dasc | 13 ++++++------ .../lj-859-math-ceil-sign.test.lua | 20 +++++++++++++++++++ 3 files changed, 32 insertions(+), 14 deletions(-) create mode 100644 test/tarantool-tests/lj-859-math-ceil-sign.test.lua diff --git a/src/vm_x64.dasc b/src/vm_x64.dasc index 399dfcbf..fc4c6259 100644 --- a/src/vm_x64.dasc +++ b/src/vm_x64.dasc @@ -400,9 +400,6 @@ |.macro sseconst_1, reg, tmp // Synthesize 1.0. | sseconst_hi reg, tmp, 3ff00000 |.endmacro -|.macro sseconst_m1, reg, tmp // Synthesize -1.0. -| sseconst_hi reg, tmp, bff00000 -|.endmacro |.macro sseconst_2p52, reg, tmp // Synthesize 2^52. | sseconst_hi reg, tmp, 43300000 |.endmacro @@ -2598,15 +2595,17 @@ static void build_subroutines(BuildCtx *ctx) | addsd xmm1, xmm3 // (|x| + 2^52) - 2^52 | subsd xmm1, xmm3 | orpd xmm1, xmm2 // Merge sign bit back in. + | sseconst_1 xmm3, RD | .if mode == 1 // ceil(x)? - | sseconst_m1 xmm2, RD // Must subtract -1 to preserve -0. | cmpsd xmm0, xmm1, 6 // x > result? + | andpd xmm0, xmm3 + | addsd xmm1, xmm0 // If yes, add 1. + | orpd xmm1, xmm2 // Merge sign bit back in (again). | .else // floor(x)? - | sseconst_1 xmm2, RD | cmpsd xmm0, xmm1, 1 // x < result? + | andpd xmm0, xmm3 + | subsd xmm1, xmm0 // If yes, subtract 1. | .endif - | andpd xmm0, xmm2 - | subsd xmm1, xmm0 // If yes, subtract +-1. |.endif | movaps xmm0, xmm1 |1: diff --git a/src/vm_x86.dasc b/src/vm_x86.dasc index 9fa9a3f7..05a8267b 100644 --- a/src/vm_x86.dasc +++ b/src/vm_x86.dasc @@ -533,9 +533,6 @@ |.macro sseconst_1, reg, tmp // Synthesize 1.0. | sseconst_hi reg, tmp, 3ff00000 |.endmacro -|.macro sseconst_m1, reg, tmp // Synthesize -1.0. -| sseconst_hi reg, tmp, bff00000 -|.endmacro |.macro sseconst_2p52, reg, tmp // Synthesize 2^52. | sseconst_hi reg, tmp, 43300000 |.endmacro @@ -3089,15 +3086,17 @@ static void build_subroutines(BuildCtx *ctx) | addsd xmm1, xmm3 // (|x| + 2^52) - 2^52 | subsd xmm1, xmm3 | orpd xmm1, xmm2 // Merge sign bit back in. + | sseconst_1 xmm3, RDa | .if mode == 1 // ceil(x)? - | sseconst_m1 xmm2, RDa // Must subtract -1 to preserve -0. | cmpsd xmm0, xmm1, 6 // x > result? + | andpd xmm0, xmm3 + | addsd xmm1, xmm0 // If yes, add 1. + | orpd xmm1, xmm2 // Merge sign bit back in (again). | .else // floor(x)? - | sseconst_1 xmm2, RDa | cmpsd xmm0, xmm1, 1 // x < result? + | andpd xmm0, xmm3 + | subsd xmm1, xmm0 // If yes, subtract 1. | .endif - | andpd xmm0, xmm2 - | subsd xmm1, xmm0 // If yes, subtract +-1. |.endif | movaps xmm0, xmm1 |1: diff --git a/test/tarantool-tests/lj-859-math-ceil-sign.test.lua b/test/tarantool-tests/lj-859-math-ceil-sign.test.lua new file mode 100644 index 00000000..df0ca329 --- /dev/null +++ b/test/tarantool-tests/lj-859-math-ceil-sign.test.lua @@ -0,0 +1,20 @@ +local tap = require('tap') + +-- Test file to demonstrate the incorrect LuaJIT's behaviour +-- for `math.ceil(x)` when argument `x`: -1 < x < -0.5. +-- See also https://github.com/LuaJIT/LuaJIT/issues/859. + +local test = tap.test('lj-859-math-ceil-sign') + +test:plan(1) + +local IS_DUALNUM = tostring(tonumber('-0')) ~= tostring(-0) +local IS_X86_64 = jit.arch == 'x86' or jit.arch == 'x64' + +-- Use `tostirng()` to compare sign of the returned value. +-- Take any value from the mentioned range. Chosen one is +-- mentioned in the commit message. +test:ok((IS_DUALNUM and IS_X86_64) or tostring(math.ceil(-0.9)) == '-0', + 'correct zero sign') + +test:done(true) -- 2.42.0