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 ED4F2572182; Mon, 21 Aug 2023 11:20:08 +0300 (MSK) DKIM-Filter: OpenDKIM Filter v2.11.0 dev.tarantool.org ED4F2572182 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=tarantool.org; s=dev; t=1692606009; bh=7dNeHfMfodCPbbufok6p+BR31zR/orychj0DI2XQI7Y=; h=Date:To:References:In-Reply-To:Subject:List-Id:List-Unsubscribe: List-Archive:List-Post:List-Help:List-Subscribe:From:Reply-To:Cc: From; b=bcYMljYfpX1BWBao7rwYPe4dyjFdCTSfsIm2nTAERb0GiXQFSNqZsUmEPR9OXJ9nR F8QRgHFuoMkxehdf7z2lWkHcDAoazkV2Z7WHq1AYxwb5mvJINYD7JDCSjOD5RJG06i I4GW2jCq0nNAJvKqOUO36XGGOcaMv+XgIalbcfn4= Received: from smtp52.i.mail.ru (smtp52.i.mail.ru [95.163.41.88]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (2048 bits) server-digest SHA256) (No client certificate requested) by dev.tarantool.org (Postfix) with ESMTPS id EE8E7572182 for ; Mon, 21 Aug 2023 11:20:07 +0300 (MSK) DKIM-Filter: OpenDKIM Filter v2.11.0 dev.tarantool.org EE8E7572182 Received: by smtp52.i.mail.ru with esmtpa (envelope-from ) id 1qY08s-003YKP-2R; Mon, 21 Aug 2023 11:20:07 +0300 Date: Mon, 21 Aug 2023 11:15:22 +0300 To: Maxim Kokryashkin Message-ID: References: <3d4ed7eb3db111ca1571688b281c560bd7a0f674.1692089299.git.skaplun@tarantool.org> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: X-Mailru-Src: smtp X-4EC0790: 10 X-7564579A: 646B95376F6C166E X-77F55803: 4F1203BC0FB41BD93C8852532D76B9E32190268CC65A0CB410EFC76A3388DB0F182A05F538085040F6E959B36E94ECFCFA550FC43DA9E3333A89A78162327BC8D10D1AEAB71D9FC7 X-7FA49CB5: FF5795518A3D127A4AD6D5ED66289B5278DA827A17800CE705C173BDDADFC82AEA1F7E6F0F101C67BD4B6F7A4D31EC0BCC500DACC3FED6E28638F802B75D45FF8AA50765F7900637E25DEE08FA4D750E8638F802B75D45FF36EB9D2243A4F8B5A6FCA7DBDB1FC311F39EFFDF887939037866D6147AF826D839DA59C84550C861748AF64F802E951A117882F4460429724CE54428C33FAD305F5C1EE8F4F765FC11A27A05CE189FECA471835C12D1D9774AD6D5ED66289B52BA9C0B312567BB23117882F446042972877693876707352033AC447995A7AD182CC0D3CB04F14752D2E47CDBA5A96583BA9C0B312567BB2376E601842F6C81A19E625A9149C048EE140C956E756FBB7A7F16001415B11694D8FC6C240DEA76429C9F4D5AE37F343AA9539A8B242431040A6AB1C7CE11FEE32A336C651863509103F1AB874ED89028C4224003CC836476E2F48590F00D11D6E2021AF6380DFAD1A18204E546F3947C2FFDA4F57982C5F42E808ACE2090B5E1725E5C173C3A84C34964A708C60C975A089D37D7C0E48F6C8AA50765F79006377F02F59195295693EFF80C71ABB335746BA297DBC24807EABDAD6C7F3747799A X-C1DE0DAB: 0D63561A33F958A57940F40B64AF3B7DBA63D8AE651DFBD6AC0764A0D20A0778F87CCE6106E1FC07E67D4AC08A07B9B0735DFC8FA7AC1207CB5012B2E24CD356 X-C8649E89: 1C3962B70DF3F0ADE00A9FD3E00BEEDF3FED46C3ACD6F73ED3581295AF09D3DF87807E0823442EA2ED31085941D9CD0AF7F820E7B07EA4CF7F46B54ECB2F50D9B13456174B4B5E4BE81FC82765FBC9F79B619408696A7B4F768602CF99B16996A7CA3A83B0F0804DE44CCE45A9901E8938D1C7A25573EB2FE48CAC7CA610320002C26D483E81D6BE5EF9655DD6DEA7D65774BB76CC95456EEC5B5AD62611EEC62B5AFB4261A09AF0 X-D57D3AED: 3ZO7eAau8CL7WIMRKs4sN3D3tLDjz0dLbV79QFUyzQ2Ujvy7cMT6pYYqY16iZVKkSc3dCLJ7zSJH7+u4VD18S7Vl4ZUrpaVfd2+vE6kuoey4m4VkSEu530nj6fImhcD4MUrOEAnl0W826KZ9Q+tr5ycPtXkTV4k65bRjmOUUP8cvGozZ33TWg5HZplvhhXbhDGzqmQDTd6OAevLeAnq3Ra9uf7zvY2zzsIhlcp/Y7m53TZgf2aB4JOg4gkr2biojHJI2DMjVra3y9aWXvwM9XA== X-Mailru-Sender: 11C2EC085EDE56FAC07928AF2646A769EFF34D6057B3E94EFA550FC43DA9E333F1FEA02A07AA46D6DEDBA653FF35249392D99EB8CC7091A70E183A470755BFD208F19895AA18418972D6B4FCE48DF648AE208404248635DF X-Mras: Ok Subject: Re: [Tarantool-patches] [PATCH luajit 5/5] Revert to trival pow() optimizations to prevent inaccuracies. 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" Hi, Maxim! Thanks for the review! See my answers below. On 20.08.23, Maxim Kokryashkin wrote: > Hi, Sergey! > Thanks for the patch! > Please consider my comments below. > On Tue, Aug 15, 2023 at 12:36:31PM +0300, Sergey Kaplun wrote: > > From: Mike Pall > > > > (cherry-picked from commit 96d6d5032098ea9f0002165394a8774dcaa0c0ce) > > > > This patch fixes different misbehaviour between JIT-compiled code and > Typo: s/misbehaviour/misbehaviours/ Fixed. > > the interpreter for power operator with the following ways: > Typo: s/with/in/ Fixed. > > * Drop folding optimizations for base ^ n => base * base ..., as far as > > pow(base, n) isn't interchangeable with just multiplicity of numbers > > and depends on the implementation. > > * Since the internal power function is inaccurate for very big or small > > powers, it is dropped, and `pow()` from the standard library is used > > instead. To save consistency between JIT behaviour and the VM > Typo: s/VM/VM,/ Fixed. > > narrowing optimization is dropped, and only trivial folding > > optimizations are used. Also, `math_extern2` version with two > > parameters is dropped, since it's no more used. > Typo: s/more/longer/ Fixed. > > > > Also, this fixes failures of the [220/502] lib/string/format/num.lua > > test [1] from LuaJIT-test suite. > Typo: s/from/from the/ Fixed. > > > > [1]: https://www.exploringbinary.com/incorrect-floating-point-to-decimal-conversions/ > > > > Sergey Kaplun: > > * added the description and the test for the problem > > > > Part of tarantool/tarantool#8825 > > --- > > > > +-- -948388 ^ 3 = -0x1.7ad0e8ad7439dp+59. > Same as in the previous patch, we need some additinal commentary for > those magic numbers. See my answer in the previous reply. > > +res = {} > > +-- XXX: use local variable to prevent folding via parser. > > +-- XXX: use stack slot out of trace to prevent constant folding. > > +local corner_case_3 = -948388 > Naming is misleading, it seems like it is the test case number, > which it is not. Please also fix this in the previous patch for `corner_case_5`. Renamed as the following: =================================================================== diff --git a/test/tarantool-tests/lj-684-pow-inconsistencies.test.lua b/test/tarantool-tests/lj-684-pow-inconsistencies.test.lua index 003fe957..418a1557 100644 --- a/test/tarantool-tests/lj-684-pow-inconsistencies.test.lua +++ b/test/tarantool-tests/lj-684-pow-inconsistencies.test.lua @@ -52,10 +52,10 @@ test:samevalues(res, ('consistent results for folding (-inf) ^ 0.5')) res = {} -- XXX: use local variable to prevent folding via parser. -- XXX: use stack slot out of trace to prevent constant folding. -local corner_case_05 = 2921 +local corner_case_pow_05 = 2921 jit.on() for i = 1, 4 do - res[i] = corner_case_05 ^ 0.5 + res[i] = corner_case_pow_05 ^ 0.5 end -- XXX: Prevent hotcount side effects. =================================================================== =================================================================== diff --git a/test/tarantool-tests/lj-684-pow-inconsistencies.test.lua b/test/tarantool-tests/lj-684-pow-inconsistencies.test.lua index 23dd44e8..57685a72 100644 --- a/test/tarantool-tests/lj-684-pow-inconsistencies.test.lua +++ b/test/tarantool-tests/lj-684-pow-inconsistencies.test.lua @@ -69,10 +69,10 @@ test:samevalues(res, ('consistent results for folding 2921 ^ 0.5')) res = {} -- XXX: use local variable to prevent folding via parser. -- XXX: use stack slot out of trace to prevent constant folding. -local corner_case_3 = -948388 +local corner_case_pow_3 = -948388 jit.on() for i = 1, 4 do - res[i] = corner_case_3 ^ 3 + res[i] = corner_case_pow_3 ^ 3 end -- XXX: Prevent hotcount side effects. =================================================================== Branch is force-pushed. > > +jit.on() > > +for i = 1, 4 do > > + res[i] = corner_case_3 ^ 3 > > +end > > + > > +-- XXX: Prevent hotcount side effects. > > +jit.off() > > +jit.flush() > > If you'll succeed making that dedicated function for those test cases in the > previous patch fix ups, this one should be rewritten too. See my answer in the previous reply. > > + > > +test:samevalues(res, ('consistent results for int pow (-948388) ^ 3')) > > + > > -- Narrowing for non-constant base of power operation. > > local function pow(base, power) > > return base ^ power > > -- > > 2.41.0 > > -- Best regards, Sergey Kaplun