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 5C6045C2843; Thu, 17 Aug 2023 17:03:58 +0300 (MSK) DKIM-Filter: OpenDKIM Filter v2.11.0 dev.tarantool.org 5C6045C2843 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=tarantool.org; s=dev; t=1692281038; bh=Prk4t4ICWD7DITqwUSnK3JHddIPXKSaPv4rHScChceE=; 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=u2Hxhl/BycpGTAAVTrdcRcn8Dz9gTg/KayZla8zcZIiVMy1EH8ecupFQxk99WFnhW gF3cID/Oph3n8s/EiYL7zcW6OvDxfq3GQJGmi26Yjp+uTV5jkATolGHVR6GlDRoHnX w+AUJZIvvClrzDXt0qopdrzUeUucxLroq0DRsORI= Received: from smtp39.i.mail.ru (smtp39.i.mail.ru [95.163.41.80]) (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 8BF3D5C2843 for ; Thu, 17 Aug 2023 17:03:56 +0300 (MSK) DKIM-Filter: OpenDKIM Filter v2.11.0 dev.tarantool.org 8BF3D5C2843 Received: by smtp39.i.mail.ru with esmtpa (envelope-from ) id 1qWdbP-00AEV3-2I; Thu, 17 Aug 2023 17:03:56 +0300 Date: Thu, 17 Aug 2023 17:03:55 +0300 To: Sergey Kaplun Message-ID: <4hdr4pngl3cxnwraf5jlxw2ozemaafc44blh45eu26z3dh4t34@cpjfru3carsq> References: <28c7aec4df761b06208cc7ccd9055dd444ed2a70.1692089299.git.skaplun@tarantool.org> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <28c7aec4df761b06208cc7ccd9055dd444ed2a70.1692089299.git.skaplun@tarantool.org> X-Mailru-Src: smtp X-7564579A: 78E4E2B564C1792B X-77F55803: 4F1203BC0FB41BD9700E0DCE2907754D6BBDFD16DC5F73F29F7DA2AE4136AA2D182A05F5380850404F08580D8F2A31474C58CF95FCFEDE129CFD9785175BD9F7E9382A930D9FC954 X-C1DE0DAB: 0D63561A33F958A5FB35153606FCB1A2B3714188C72500179CA7FDC079330CF9F87CCE6106E1FC07E67D4AC08A07B9B0B355ED1E20F5346ACB5012B2E24CD356 X-C8649E89: 1C3962B70DF3F0AD5177F0B940C8B66ECE892A7B2722663E91682638B966EB3F662256BEEFA9527FFB4847595069D7AE4C55547A6C8DA52263530F1B36311BA1C0C20708AF31D3DFD2B676ADB121B45337FD76D11AF80A0D4AA327FC61CF1A4E6BF9EE37DD852104EA455F16B58544A2557BDE0DD54B3590965026E5D17F6739C77C69D99B9914278E50E1F0597A6FD5CD72808BE417F3B9E0E7457915DAA85F X-D57D3AED: 3ZO7eAau8CL7WIMRKs4sN3D3tLDjz0dLbV79QFUyzQ2Ujvy7cMT6pYYqY16iZVKkSc3dCLJ7zSJH7+u4VD18S7Vl4ZUrpaVfd2+vE6kuoey4m4VkSEu530nj6fImhcD4MUrOEAnl0W826KZ9Q+tr5ycPtXkTV4k65bRjmOUUP8cvGozZ33TWg5HZplvhhXbhDGzqmQDTd6OAevLeAnq3Ra9uf7zvY2zzsIhlcp/Y7m53TZgf2aB4JOg4gkr2biojFRrmMqSMPxq0KiNXqP3BZQ== X-Mailru-Sender: 11C2EC085EDE56FA38FD4C59F7EFE4071649AAF7D43523BB0E7AEAE677D9DC96194D47AD2E63B27ED51284F0FE6F529ABC7555A253F5B200DF104D74F62EE79D27EC13EC74F6107F4198E0F3ECE9B5443453F38A29522196 X-Mras: OK Subject: Re: [Tarantool-patches] [PATCH luajit 1/5] test: introduce `samevalues()` TAP checker 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: Maxim Kokryashkin via Tarantool-patches Reply-To: Maxim Kokryashkin Cc: tarantool-patches@dev.tarantool.org Errors-To: tarantool-patches-bounces@dev.tarantool.org Sender: "Tarantool-patches" Hi, Sergey! Thanks for the patch! LGTM, except for a few comments below. On Tue, Aug 15, 2023 at 12:36:27PM +0300, Sergey Kaplun wrote: > The introduced `samevalues()` helper checks that values in range from Typo: s/in range/in the range/ > 1, to `table.maxn()` of the given table are exactly the same. It may be > usefull for test consistency of JIT and VM behaviour. Originally, the Typo: s/usefull for test/useful to test the/ > `arr_is_consistent()` function was introduced in the > . `samevalues()` has the same > functionallity (except usage of `table.maxn()` instead `#` operator to Typo: s/functionallity/functionality/ Typo: s/except/except for the/ Typo: s/instead/instead of the/ > be sure, that the table we check isn't a sparse array). > --- > test/tarantool-tests/gh-6163-min-max.test.lua | 52 ++++++++----------- > test/tarantool-tests/tap.lua | 14 +++++ > 2 files changed, 37 insertions(+), 29 deletions(-) > > diff --git a/test/tarantool-tests/gh-6163-min-max.test.lua b/test/tarantool-tests/gh-6163-min-max.test.lua > index 63437955..4bc6155c 100644 > --- a/test/tarantool-tests/gh-6163-min-max.test.lua > +++ b/test/tarantool-tests/gh-6163-min-max.test.lua > @@ -2,25 +2,17 @@ local tap = require('tap') > 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. > -- > > -local function isnan(x) > - return x ~= x > -end > - > -local function array_is_consistent(res) > - for i = 1, #res - 1 do > - if res[i] ~= res[i + 1] and not (isnan(res[i]) and isnan(res[i + 1])) then > - return false > - end > - end > - return true > -end > - > -- This function creates dirty values on the Lua stack. > -- The latter of them is going to be treated as an > -- argument by the `math.min/math.max`. > @@ -91,14 +83,14 @@ for k = 1, 4 do > result[k] = min(min(x, nan), x) > end > -- expected: 1 1 1 1 > -test:ok(array_is_consistent(result), 'math.min: reassoc_dup') > +test:samevalues(result, 'math.min: reassoc_dup') > > result = {} > for k = 1, 4 do > result[k] = max(max(x, nan), x) > end > -- expected: 1 1 1 1 > -test:ok(array_is_consistent(result), 'math.max: reassoc_dup') > +test:samevalues(result, 'math.max: reassoc_dup') > > -- If one gets the expression like `math.min(x, math.min(x, nan))`, > -- and the `comm_dup` optimization is applied, it results in the > @@ -120,7 +112,7 @@ for k = 1, 4 do > end > -- FIXME: results are still inconsistent for the x86/64 architecture. > -- expected: nan nan nan nan > -test:ok(array_is_consistent(result) or x86_64, 'math.min: comm_dup_minmax') > +test:samevalues(x86_64 and DUMMY_TAB or result, 'math.min: comm_dup_minmax') > > result = {} > for k = 1, 4 do > @@ -128,7 +120,7 @@ for k = 1, 4 do > end > -- FIXME: results are still inconsistent for the x86/64 architecture. > -- expected: nan nan nan nan > -test:ok(array_is_consistent(result) or x86_64, 'math.max: comm_dup_minmax') > +test:samevalues(x86_64 and DUMMY_TAB or result, 'math.max: comm_dup_minmax') > > -- The following optimization should be disabled: > -- (x o k1) o k2 ==> x o (k1 o k2) > @@ -139,49 +131,49 @@ for k = 1, 4 do > result[k] = min(min(x, 0/0), 1.3) > end > -- expected: 1.3 1.3 1.3 1.3 > -test:ok(array_is_consistent(result), 'math.min: reassoc_minmax_k') > +test:samevalues(result, 'math.min: reassoc_minmax_k') > > result = {} > for k = 1, 4 do > result[k] = max(max(x, 0/0), 1.1) > end > -- expected: 1.1 1.1 1.1 1.1 > -test:ok(array_is_consistent(result), 'math.max: reassoc_minmax_k') > +test:samevalues(result, 'math.max: reassoc_minmax_k') > > result = {} > for k = 1, 4 do > result[k] = min(max(nan, 1), 1) > end > -- expected: 1 1 1 1 > -test:ok(array_is_consistent(result), 'min-max-case1: reassoc_minmax_left') > +test:samevalues(result, 'min-max-case1: reassoc_minmax_left') > > result = {} > for k = 1, 4 do > result[k] = min(max(1, nan), 1) > end > -- expected: 1 1 1 1 > -test:ok(array_is_consistent(result), 'min-max-case2: reassoc_minmax_left') > +test:samevalues(result, 'min-max-case2: reassoc_minmax_left') > > result = {} > for k = 1, 4 do > result[k] = max(min(nan, 1), 1) > end > -- expected: 1 1 1 1 > -test:ok(array_is_consistent(result), 'max-min-case1: reassoc_minmax_left') > +test:samevalues(result, 'max-min-case1: reassoc_minmax_left') > > result = {} > for k = 1, 4 do > result[k] = max(min(1, nan), 1) > end > -- expected: 1 1 1 1 > -test:ok(array_is_consistent(result), 'max-min-case2: reassoc_minmax_left') > +test:samevalues(result, 'max-min-case2: reassoc_minmax_left') > > result = {} > for k = 1, 4 do > result[k] = min(1, max(nan, 1)) > end > -- expected: 1 1 1 1 > -test:ok(array_is_consistent(result), 'min-max-case1: reassoc_minmax_right') > +test:samevalues(result, 'min-max-case1: reassoc_minmax_right') > > result = {} > for k = 1, 4 do > @@ -189,14 +181,15 @@ for k = 1, 4 do > end > -- FIXME: results are still inconsistent for the x86/64 architecture. > -- expected: nan nan nan nan > -test:ok(array_is_consistent(result) or x86_64, 'min-max-case2: reassoc_minmax_right') > +test:samevalues(x86_64 and DUMMY_TAB or result, > + 'min-max-case2: reassoc_minmax_right') Side note: this skipcond looks complex, but I can't come up with an alternative better than altering the TAP-plan, which is even worse option... > > result = {} > for k = 1, 4 do > result[k] = max(1, min(nan, 1)) > end > -- expected: 1 1 1 1 > -test:ok(array_is_consistent(result), 'max-min-case1: reassoc_minmax_right') > +test:samevalues(result, 'max-min-case1: reassoc_minmax_right') > > result = {} > for k = 1, 4 do > @@ -204,7 +197,8 @@ for k = 1, 4 do > end > -- FIXME: results are still inconsistent for the x86/64 architecture. > -- expected: nan nan nan nan > -test:ok(array_is_consistent(result) or x86_64, 'max-min-case2: reassoc_minmax_right') > +test:samevalues(x86_64 and DUMMY_TAB or result, > + 'max-min-case2: reassoc_minmax_right') > > -- XXX: If we look into the disassembled code of `lj_vm_foldarith()` > -- we can see the following: > @@ -253,13 +247,13 @@ for k = 1, 4 do > result[k] = min(min(7.1, 0/0), 1.1) > end > -- expected: 1.1 1.1 1.1 1.1 > -test:ok(array_is_consistent(result), 'min: fold_kfold_numarith') > +test:samevalues(result, 'min: fold_kfold_numarith') > > result = {} > for k = 1, 4 do > result[k] = max(max(7.1, 0/0), 1.1) > end > -- expected: 1.1 1.1 1.1 1.1 > -test:ok(array_is_consistent(result), 'max: fold_kfold_numarith') > +test:samevalues(result, 'max: fold_kfold_numarith') > > test:done(true) > diff --git a/test/tarantool-tests/tap.lua b/test/tarantool-tests/tap.lua > index 8559ee52..af1d4b20 100644 > --- a/test/tarantool-tests/tap.lua > +++ b/test/tarantool-tests/tap.lua > @@ -254,6 +254,19 @@ local function iscdata(test, v, ctype, message, extra) > return ok(test, ffi.istype(ctype, v), message, extra) > end > > +local function isnan(v) > + return v ~= v > +end > + > +local function samevalues(test, got, message, extra) > + for i = 1, table.maxn(got) - 1 do > + if got[i] ~= got[i + 1] and not (isnan(got[i]) and isnan(got[i + 1])) then > + return fail(test, message, extra) > + end > + end > + return ok(test, true, message, extra) > +end > + > local test_mt > > local function new(parent, name, fun, ...) > @@ -372,6 +385,7 @@ test_mt = { > isudata = isudata, > iscdata = iscdata, > is_deeply = is_deeply, > + samevalues = samevalues, > like = like, > unlike = unlike, > } > -- > 2.41.0 >