[Tarantool-patches] [PATCH luajit v4 3/3] Cleanup math function compilation and fix inconsistencies.
Sergey Kaplun
skaplun at tarantool.org
Thu Apr 14 11:53:13 MSK 2022
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
<snipped>
> 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
<snipped>
> 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
<snipped>
> 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
<snipped>
> 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
<snipped>
> 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
<snipped>
> 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
<snipped>
> 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
<snipped>
> 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
<snipped>
> 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 {
<snipped>
> 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
<snipped>
> 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 <utils.lua> 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
More information about the Tarantool-patches
mailing list