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 A11CD6ECC0; Thu, 14 Apr 2022 11:55:26 +0300 (MSK) DKIM-Filter: OpenDKIM Filter v2.11.0 dev.tarantool.org A11CD6ECC0 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=tarantool.org; s=dev; t=1649926526; bh=hHd5KKOp0jKxzvC/2E9lyzw+hXQ2wH98gYYRvSvj6VU=; 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=LSJr0gfPZbKdhykdTGjhmQbyx2NScwEOn5rPVT9aFBpLhN8DGpn3V20P4vrSCBlnB Rg2b0NINUgk2U72SyPD4rXOTM6R5MKD5s/MTxGPGyr23PIXu5GoMfHaRJYFqC/wvAl bArw4z+cKhvqqZubX5R5TBML7rnxrdZv2lFg555g= Received: from smtpng3.i.mail.ru (smtpng3.i.mail.ru [94.100.177.149]) (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 670376ECC0 for ; Thu, 14 Apr 2022 11:55:24 +0300 (MSK) DKIM-Filter: OpenDKIM Filter v2.11.0 dev.tarantool.org 670376ECC0 Received: by smtpng3.m.smailru.net with esmtpa (envelope-from ) id 1nevG7-0002nH-Ca; Thu, 14 Apr 2022 11:55:23 +0300 Date: Thu, 14 Apr 2022 11:53:13 +0300 To: Maxim Kokryashkin Message-ID: References: MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: X-4EC0790: 10 X-7564579A: 646B95376F6C166E X-77F55803: 4F1203BC0FB41BD916C41472748AFA04A85EF62FA87B5A9715643A7EFE44EA7F00894C459B0CD1B95C56DE41BBE832385224369393025B208E9A4E6000B476CB6DD001A71ABC03EA X-7FA49CB5: FF5795518A3D127A4AD6D5ED66289B5278DA827A17800CE7607B8778E2C6A697EA1F7E6F0F101C67BD4B6F7A4D31EC0BCC500DACC3FED6E28638F802B75D45FF8AA50765F7900637AE50F465217C6D208638F802B75D45FF36EB9D2243A4F8B5A6FCA7DBDB1FC311F39EFFDF887939037866D6147AF826D86B7EA162BFF57E38A25F5E6CD27698A6117882F4460429724CE54428C33FAD305F5C1EE8F4F765FCAA867293B0326636D2E47CDBA5A96583BD4B6F7A4D31EC0BC014FD901B82EE079FA2833FD35BB23D27C277FBC8AE2E8BAA867293B0326636D2E47CDBA5A96583BA9C0B312567BB231DD303D21008E29813377AFFFEAFD269A417C69337E82CC2E827F84554CEF50127C277FBC8AE2E8BA83251EDC214901ED5E8D9A59859A8B6A45692FFBBD75A6A089D37D7C0E48F6C5571747095F342E88FB05168BE4CE3AF X-8FC586DF: 6EFBBC1D9D64D975 X-C1DE0DAB: 0D63561A33F958A533D7502A9795840CD8A9E22152CA0DABCC4486EF5D7B80C6D59269BC5F550898D99A6476B3ADF6B47008B74DF8BB9EF7333BD3B22AA88B938A852937E12ACA75363A6695AA4CE4EF410CA545F18667F91A7EA1CDA0B5A7A0 X-C8649E89: 4E36BF7865823D7055A7F0CF078B5EC49A30900B95165D34D8C933888226C841337C1BB1E0ACF822408D1705684011C2F4E3F88C3D88D84C24B093B560C3BB9E1D7E09C32AA3244CB6AB66F94CD39F33FBB0B98CA5FF4BFFE3D93501275E802F927AC6DF5659F194 X-D57D3AED: 3ZO7eAau8CL7WIMRKs4sN3D3tLDjz0dLbV79QFUyzQ2Ujvy7cMT6pYYqY16iZVKkSc3dCLJ7zSJH7+u4VD18S7Vl4ZUrpaVfd2+vE6kuoey4m4VkSEu530nj6fImhcD4MUrOEAnl0W826KZ9Q+tr5ycPtXkTV4k65bRjmOUUP8cvGozZ33TWg5HZplvhhXbhDGzqmQDTd6OAevLeAnq3Ra9uf7zvY2zzsIhlcp/Y7m53TZgf2aB4JOg4gkr2biojixb2kBKhBtrysWGfmiN2Bw== X-Mailru-Sender: 689FA8AB762F739339CABD9B3CA9A7D647422FC6BBEDC7BB44513712D00892A00FBE9A32752B8C9C2AA642CC12EC09F1FB559BB5D741EB962F61BD320559CF1EFD657A8799238ED55FEEDEB644C299C0ED14614B50AE0675 X-Mras: Ok Subject: Re: [Tarantool-patches] [PATCH luajit v4 3/3] Cleanup math function compilation and fix inconsistencies. 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 patch! Please consider my comments below. On 30.03.22, Maxim Kokryashkin wrote: > This patch changes 'math_unary', `math_htrig` and `math_atrig` > to `math_call` for math functions compilation. Now all of > math functions in IRs are called with `CALLN` instead of > `FPMATH` (`ATAN2` for `math.atan2`). It should be mentioned that IR ATAN2 is removed in this patch. Also, we can drop some words about new fold optimizations. Side note: Did Mike means by inconsistencies in organization of code structure? Or it was inconsistent behaviour? > > Part of tarantool/tarantool#6163 > --- > src/lib_math.c | 22 +++---- > src/lj_asm.c | 6 -- > src/lj_asm_arm.h | 1 - > src/lj_asm_arm64.h | 1 - > src/lj_asm_x86.h | 2 - > src/lj_ffrecord.c | 19 +----- > src/lj_ir.h | 4 +- > src/lj_ircall.h | 14 +++-- > src/lj_opt_fold.c | 25 +++++++- > src/lj_opt_split.c | 3 - > src/lj_target_x86.h | 6 -- > src/lj_vmmath.c | 6 -- > .../gh-6163-jit-min-max.test.lua | 63 ++++++++++++++++++- > 13 files changed, 109 insertions(+), 63 deletions(-) > > diff --git a/src/lib_math.c b/src/lib_math.c > index 4e6d2458..601655cd 100644 > --- a/src/lib_math.c > +++ b/src/lib_math.c > diff --git a/src/lj_asm.c b/src/lj_asm.c > index 10e5872b..1a7fb0c8 100644 > --- a/src/lj_asm.c > +++ b/src/lj_asm.c > diff --git a/src/lj_asm_arm.h b/src/lj_asm_arm.h > index 84ab06c0..6ae6e2f2 100644 > --- a/src/lj_asm_arm.h > +++ b/src/lj_asm_arm.h > diff --git a/src/lj_asm_arm64.h b/src/lj_asm_arm64.h > index e93e29e3..6a115f2a 100644 > --- a/src/lj_asm_arm64.h > +++ b/src/lj_asm_arm64.h > diff --git a/src/lj_asm_x86.h b/src/lj_asm_x86.h > index 767bf6f3..f75af8a4 100644 > --- a/src/lj_asm_x86.h > +++ b/src/lj_asm_x86.h > diff --git a/src/lj_ffrecord.c b/src/lj_ffrecord.c > index ac9c9ba1..649ac705 100644 > --- a/src/lj_ffrecord.c > +++ b/src/lj_ffrecord.c > diff --git a/src/lj_ir.h b/src/lj_ir.h > index 3059bf65..4bad47ed 100644 > --- a/src/lj_ir.h > +++ b/src/lj_ir.h > diff --git a/src/lj_ircall.h b/src/lj_ircall.h > index 973c36e6..aa06b273 100644 > --- a/src/lj_ircall.h > +++ b/src/lj_ircall.h > diff --git a/src/lj_opt_fold.c b/src/lj_opt_fold.c > index 29b760b4..2c9122c5 100644 > --- a/src/lj_opt_fold.c > +++ b/src/lj_opt_fold.c > @@ -173,7 +173,6 @@ LJFOLD(ADD KNUM KNUM) > LJFOLD(SUB KNUM KNUM) > LJFOLD(MUL KNUM KNUM) > LJFOLD(DIV KNUM KNUM) > -LJFOLD(ATAN2 KNUM KNUM) > LJFOLD(LDEXP KNUM KNUM) > LJFOLD(MIN KNUM KNUM) > LJFOLD(MAX KNUM KNUM) > @@ -213,6 +212,30 @@ LJFOLDF(kfold_fpmath) > return lj_ir_knum(J, y); > } > > +LJFOLD(CALLN KNUM any) > +LJFOLDF(kfold_fpcall1) > +{ > + const CCallInfo *ci = &lj_ir_callinfo[fins->op2]; > + if (CCI_TYPE(ci) == IRT_NUM) { > + double y = ((double (*)(double))ci->func)(knumleft); > + return lj_ir_knum(J, y); > + } > + return NEXTFOLD; > +} > + > +LJFOLD(CALLN CARG IRCALL_atan2) > +LJFOLDF(kfold_fpcall2) > +{ > + if (irref_isk(fleft->op1) && irref_isk(fleft->op2)) { > + const CCallInfo *ci = &lj_ir_callinfo[fins->op2]; > + double a = ir_knum(IR(fleft->op1))->n; > + double b = ir_knum(IR(fleft->op2))->n; > + double y = ((double (*)(double, double))ci->func)(a, b); > + return lj_ir_knum(J, y); > + } > + return NEXTFOLD; > +} > + > LJFOLD(POW KNUM KINT) > LJFOLDF(kfold_numpow) > { > diff --git a/src/lj_opt_split.c b/src/lj_opt_split.c > index fc935204..c0788106 100644 > --- a/src/lj_opt_split.c > +++ b/src/lj_opt_split.c > diff --git a/src/lj_target_x86.h b/src/lj_target_x86.h > index 356f7924..194f8e70 100644 > --- a/src/lj_target_x86.h > +++ b/src/lj_target_x86.h > @@ -228,16 +228,10 @@ typedef enum { > diff --git a/src/lj_vmmath.c b/src/lj_vmmath.c > index 08ccf467..ae4e0f15 100644 > --- a/src/lj_vmmath.c > +++ b/src/lj_vmmath.c > diff --git a/test/tarantool-tests/gh-6163-jit-min-max.test.lua b/test/tarantool-tests/gh-6163-jit-min-max.test.lua > index 4594b968..f9e69842 100644 > --- a/test/tarantool-tests/gh-6163-jit-min-max.test.lua > +++ b/test/tarantool-tests/gh-6163-jit-min-max.test.lua > @@ -1,10 +1,37 @@ These new tests are not related to min/max functions, so they should be moved to the separate test file. Also, I'm not sure about the tests themselves -- what are they checking? Maybe we should check at lease fold optimizations instead? > local tap = require('tap') > local jutil = require('jit.util') > +local bit = require('bit') > +local vmdef = require('jit.vmdef') > + > +local shr = bit.rshift > +local band = bit.band > +local sub = string.sub > +local traceinfo = jutil.traceinfo > +local traceir = jutil.traceir > + > +local function check_ir(tr, funcnm) > + local info = traceinfo(tr) > + assert(info ~= nil) > + > + local nins = info.nins > + local irnames = vmdef.irnames > + > + for ins = 1, nins do > + local m, ot, _, op2, _ = traceir(tr, ins) > + local oidx = 6 * shr(ot, 8) > + local op = sub(irnames, oidx + 1, oidx + 6) > + if sub(op, 1, 5) == "CALLN" and > + band(m, 3 * 4) == 4 and > + vmdef.ircall[op2] == funcnm then return true end > + end > + return false > +end I suggest to move this function to the and change its prototype to the following: | function M.hasir(traceno, ir, irnum) Where irnum is an optional field to check that ir has the corresponding number (may be it should be some callback instead if IR is found). > + > jit.off() > jit.flush() > > local test = tap.test("gh-6163-jit-min-max") > -test:plan(6) > +test:plan(18) > > -- `math.min`/`math.max` success with no args. > local pcall = pcall > @@ -32,4 +59,38 @@ test:ok(tr2_info ~= nil) > test:ok(tr1_info.link == 2) > test:ok(tr1_info.linktype == "stitch") > > +jit.off() > +jit.flush() > +jit.on() > + > +-- All math functions should appear as `CALLN` in IRs. > +for _=1,3 do > + math.sin(1) > + math.cos(1) > + math.tan(1) > + math.exp(1) > + math.log10(1) > + math.asin(0) > + math.acos(0) > + math.atan(1) > + math.atan2(2, 1) > + math.sinh(1) > + math.cosh(1) > + math.tanh(1) > +end > +jit.off() > + > +test:ok(check_ir(1, "sin") == true) > +test:ok(check_ir(1, "cos") == true) > +test:ok(check_ir(1, "tan") == true) > +test:ok(check_ir(1, "exp") == true) > +test:ok(check_ir(1, "log10") == true) > +test:ok(check_ir(1, "asin") == true) > +test:ok(check_ir(1, "acos") == true) > +test:ok(check_ir(1, "atan") == true) > +test:ok(check_ir(1, "atan2") == true) > +test:ok(check_ir(1, "sinh") == true) > +test:ok(check_ir(1, "cosh") == true) > +test:ok(check_ir(1, "tanh") == true) > + > os.exit(test:check() and 0 or 1) > -- > 2.35.1 > -- Best regards, Sergey Kaplun