From: Igor Munkin <imun@tarantool.org>
To: Olga Arkhangelskaia <arkholga@tarantool.org>
Cc: tarantool-patches@dev.tarantool.org
Subject: Re: [Tarantool-patches] [PATCH v4] build: introduced LUAJIT_ENABLE_PAIRSMM flag
Date: Fri, 22 Nov 2019 14:21:42 +0300 [thread overview]
Message-ID: <20191122112142.GG18878@tarantool.org> (raw)
In-Reply-To: <20191122100042.5907-1-arkholga@tarantool.org>
Olya,
Thanks for the patch. It LGTM, but please consider to adjust the commit
subject regarding our contribution guide in your branch.
CCed Kirill.
On 22.11.19, Olga Arkhangelskaia wrote:
> Adds LUAJIT_ENABLE_PAIRSMM flag as a build option for luajit.
> If the flag is set, pairs/ipairs metamethods are available in Lua 5.1.
> For tarantool this option is enabled by default.
Minor: I guess the latter fact can be omitted within this comment. There
will be a corresponding patch for luajit.cmake module within
tarantool/tarantool[1] tree, so you can mention it there.
>
> Part of #4560
> ---
> Branch:https://github.com/tarantool/luajit/tree/OKriw/gh-4560-Enable-LUAJIT_ENABLE_LUA52COMPAT-when-bulding-luajit
>
> v1: https://lists.tarantool.org/pipermail/tarantool-patches/2019-November/012466.html
> v2: https://lists.tarantool.org/pipermail/tarantool-patches/2019-November/012489.html
> v3: https://lists.tarantool.org/pipermail/tarantool-patches/2019-November/012527.html
>
> Changes in v2:
> - flag is renamed
> - now flag is used only for pairsmm
> - added test case
>
> Changes in v3:
> - added flag to other vm*
> - changed comments and commit msg
>
> Changes in v4:
> - fixed errors in arm/arm64 vm
> - rewrited commit msg, subject, comment
>
> src/Makefile | 3 +++
> src/lib_base.c | 4 ++--
> src/lj_arch.h | 6 ++++++
> src/lj_obj.h | 2 +-
> src/vm_arm.dasc | 8 +++----
> src/vm_arm64.dasc | 8 +++----
> src/vm_mips.dasc | 4 ++--
> src/vm_mips64.dasc | 4 ++--
> src/vm_ppc.dasc | 4 ++--
> src/vm_x64.dasc | 4 ++--
> src/vm_x86.dasc | 4 ++--
> test/mmpairs_test.lua | 50 +++++++++++++++++++++++++++++++++++++++++++
> 12 files changed, 80 insertions(+), 21 deletions(-)
> create mode 100755 test/mmpairs_test.lua
>
> diff --git a/src/Makefile b/src/Makefile
> index aca656d..a1a39d2 100644
> --- a/src/Makefile
> +++ b/src/Makefile
> @@ -101,6 +101,9 @@ XCFLAGS=
> # Note: this does not provide full compatibility with Lua 5.2 at this time.
> #XCFLAGS+= -DLUAJIT_ENABLE_LUA52COMPAT
> #
> +#Enable pairs/ipairs methametods from Lua 5.2.
> +#XCFLAGS+= -DLUAJIT_ENABLE_PAIRSMM
> +#
> # Disable the JIT compiler, i.e. turn LuaJIT into a pure interpreter.
> #XCFLAGS+= -DLUAJIT_DISABLE_JIT
> #
> diff --git a/src/lib_base.c b/src/lib_base.c
> index 3a75787..950a580 100644
> --- a/src/lib_base.c
> +++ b/src/lib_base.c
> @@ -81,12 +81,12 @@ LJLIB_ASM(next)
> return FFH_UNREACHABLE;
> }
>
> -#if LJ_52 || LJ_HASFFI
> +#if LJ_PAIRSMM || LJ_HASFFI
> static int ffh_pairs(lua_State *L, MMS mm)
> {
> TValue *o = lj_lib_checkany(L, 1);
> cTValue *mo = lj_meta_lookup(L, o, mm);
> - if ((LJ_52 || tviscdata(o)) && !tvisnil(mo)) {
> + if ((LJ_PAIRSMM || tviscdata(o)) && !tvisnil(mo)) {
> L->top = o+1; /* Only keep one argument. */
> copyTV(L, L->base-1-LJ_FR2, mo); /* Replace callable. */
> return FFH_TAILCALL;
> diff --git a/src/lj_arch.h b/src/lj_arch.h
> index c8d7138..0c5230c 100644
> --- a/src/lj_arch.h
> +++ b/src/lj_arch.h
> @@ -564,4 +564,10 @@
> #define LJ_52 0
> #endif
>
> +/* Enabling pairs/ipairs metamethods for Lua 5.1 */
> +#if defined(LUAJIT_ENABLE_PAIRSMM) || defined (LUAJIT_ENABLE_LUA52COMPAT)
> +#define LJ_PAIRSMM 1
> +#else
> +#define LJ_PAIRSMM 0
> +#endif
> #endif
> diff --git a/src/lj_obj.h b/src/lj_obj.h
> index f368578..49467fb 100644
> --- a/src/lj_obj.h
> +++ b/src/lj_obj.h
> @@ -530,7 +530,7 @@ enum {
> #define MMDEF_FFI(_)
> #endif
>
> -#if LJ_52 || LJ_HASFFI
> +#if LJ_PAIRSMM || LJ_HASFFI
> #define MMDEF_PAIRS(_) _(pairs) _(ipairs)
> #else
> #define MMDEF_PAIRS(_)
> diff --git a/src/vm_arm.dasc b/src/vm_arm.dasc
> index d4cdaf5..a98ea12 100644
> --- a/src/vm_arm.dasc
> +++ b/src/vm_arm.dasc
> @@ -1133,12 +1133,12 @@ static void build_subroutines(BuildCtx *ctx)
> |
> |.ffunc_1 pairs
> | checktab CARG2, ->fff_fallback
> -#if LJ_52
> +#if LJ_PAIRSMM
> | ldr TAB:RB, TAB:CARG1->metatable
> #endif
> | ldrd CFUNC:CARG34, CFUNC:CARG3->upvalue[0]
> | ldr PC, [BASE, FRAME_PC]
> -#if LJ_52
> +#if LJ_PAIRSMM
> | cmp TAB:RB, #0
> | bne ->fff_fallback
> #endif
> @@ -1183,12 +1183,12 @@ static void build_subroutines(BuildCtx *ctx)
> |
> |.ffunc_1 ipairs
> | checktab CARG2, ->fff_fallback
> -#if LJ_52
> +#if LJ_PAIRSMM
> | ldr TAB:RB, TAB:CARG1->metatable
> #endif
> | ldrd CFUNC:CARG34, CFUNC:CARG3->upvalue[0]
> | ldr PC, [BASE, FRAME_PC]
> -#if LJ_52
> +#if LJ_PAIRSMM
> | cmp TAB:RB, #0
> | bne ->fff_fallback
> #endif
> diff --git a/src/vm_arm64.dasc b/src/vm_arm64.dasc
> index 3eaf376..fcd9aad 100644
> --- a/src/vm_arm64.dasc
> +++ b/src/vm_arm64.dasc
> @@ -1103,12 +1103,12 @@ static void build_subroutines(BuildCtx *ctx)
> |
> |.ffunc_1 pairs
> | checktp TMP1, CARG1, LJ_TTAB, ->fff_fallback
> -#if LJ_52
> +#if LJ_PAIRSMM
> | ldr TAB:CARG2, TAB:TMP1->metatable
> #endif
> | ldr CFUNC:CARG4, CFUNC:CARG3->upvalue[0]
> | ldr PC, [BASE, FRAME_PC]
> -#if LJ_52
> +#if LJ_PAIRSMM
> | cbnz TAB:CARG2, ->fff_fallback
> #endif
> | mov RC, #(3+1)*8
> @@ -1146,12 +1146,12 @@ static void build_subroutines(BuildCtx *ctx)
> |
> |.ffunc_1 ipairs
> | checktp TMP1, CARG1, LJ_TTAB, ->fff_fallback
> -#if LJ_52
> +#if LJ_PAIRSMM
> | ldr TAB:CARG2, TAB:TMP1->metatable
> #endif
> | ldr CFUNC:CARG4, CFUNC:CARG3->upvalue[0]
> | ldr PC, [BASE, FRAME_PC]
> -#if LJ_52
> +#if LJ_PAIRSMM
> | cbnz TAB:CARG2, ->fff_fallback
> #endif
> | mov RC, #(3+1)*8
> diff --git a/src/vm_mips.dasc b/src/vm_mips.dasc
> index 1afd611..81de610 100644
> --- a/src/vm_mips.dasc
> +++ b/src/vm_mips.dasc
> @@ -1292,7 +1292,7 @@ static void build_subroutines(BuildCtx *ctx)
> | li AT, LJ_TTAB
> | bne SFARG1HI, AT, ->fff_fallback
> |. lw PC, FRAME_PC(BASE)
> -#if LJ_52
> +#if LJ_PAIRSMM
> | lw TAB:TMP2, TAB:SFARG1LO->metatable
> | lw TMP0, CFUNC:RB->upvalue[0].u32.hi
> | lw TMP1, CFUNC:RB->upvalue[0].u32.lo
> @@ -1358,7 +1358,7 @@ static void build_subroutines(BuildCtx *ctx)
> | li AT, LJ_TTAB
> | bne SFARG1HI, AT, ->fff_fallback
> |. lw PC, FRAME_PC(BASE)
> -#if LJ_52
> +#if LJ_PAIRSMM
> | lw TAB:TMP2, TAB:SFARG1LO->metatable
> | lw TMP0, CFUNC:RB->upvalue[0].u32.hi
> | lw TMP1, CFUNC:RB->upvalue[0].u32.lo
> diff --git a/src/vm_mips64.dasc b/src/vm_mips64.dasc
> index c06270a..4cdff5e 100644
> --- a/src/vm_mips64.dasc
> +++ b/src/vm_mips64.dasc
> @@ -1306,7 +1306,7 @@ static void build_subroutines(BuildCtx *ctx)
> |.ffunc_1 pairs
> | checktp TAB:TMP1, CARG1, -LJ_TTAB, ->fff_fallback
> | ld PC, FRAME_PC(BASE)
> -#if LJ_52
> +#if LJ_PAIRSMM
> | ld TAB:TMP2, TAB:TMP1->metatable
> | ld TMP0, CFUNC:RB->upvalue[0]
> | bnez TAB:TMP2, ->fff_fallback
> @@ -1359,7 +1359,7 @@ static void build_subroutines(BuildCtx *ctx)
> |.ffunc_1 ipairs
> | checktp TAB:TMP1, CARG1, -LJ_TTAB, ->fff_fallback
> | ld PC, FRAME_PC(BASE)
> -#if LJ_52
> +#if LJ_PAIRSMM
> | ld TAB:TMP2, TAB:TMP1->metatable
> | ld CFUNC:TMP0, CFUNC:RB->upvalue[0]
> | bnez TAB:TMP2, ->fff_fallback
> diff --git a/src/vm_ppc.dasc b/src/vm_ppc.dasc
> index b4260eb..4570c12 100644
> --- a/src/vm_ppc.dasc
> +++ b/src/vm_ppc.dasc
> @@ -1454,7 +1454,7 @@ static void build_subroutines(BuildCtx *ctx)
> | checktab CARG3
> | lwz PC, FRAME_PC(BASE)
> | bne ->fff_fallback
> -#if LJ_52
> +#if LJ_PAIRSMM
> | lwz TAB:TMP2, TAB:CARG1->metatable
> | lfd f0, CFUNC:RB->upvalue[0]
> | cmplwi TAB:TMP2, 0
> @@ -1540,7 +1540,7 @@ static void build_subroutines(BuildCtx *ctx)
> | checktab CARG3
> | lwz PC, FRAME_PC(BASE)
> | bne ->fff_fallback
> -#if LJ_52
> +#if LJ_PAIRSMM
> | lwz TAB:TMP2, TAB:CARG1->metatable
> | lfd f0, CFUNC:RB->upvalue[0]
> | cmplwi TAB:TMP2, 0
> diff --git a/src/vm_x64.dasc b/src/vm_x64.dasc
> index 80753e0..2785f68 100644
> --- a/src/vm_x64.dasc
> +++ b/src/vm_x64.dasc
> @@ -1391,7 +1391,7 @@ static void build_subroutines(BuildCtx *ctx)
> | mov TAB:RB, [BASE]
> | mov TMPR, TAB:RB
> | checktab TAB:RB, ->fff_fallback
> -#if LJ_52
> +#if LJ_PAIRSMM
> | cmp aword TAB:RB->metatable, 0; jne ->fff_fallback
> #endif
> | mov CFUNC:RD, [BASE-16]
> @@ -1460,7 +1460,7 @@ static void build_subroutines(BuildCtx *ctx)
> | mov TAB:RB, [BASE]
> | mov TMPR, TAB:RB
> | checktab TAB:RB, ->fff_fallback
> -#if LJ_52
> +#if LJ_PAIRSMM
> | cmp aword TAB:RB->metatable, 0; jne ->fff_fallback
> #endif
> | mov CFUNC:RD, [BASE-16]
> diff --git a/src/vm_x86.dasc b/src/vm_x86.dasc
> index 56bee14..eae07f7 100644
> --- a/src/vm_x86.dasc
> +++ b/src/vm_x86.dasc
> @@ -1724,7 +1724,7 @@ static void build_subroutines(BuildCtx *ctx)
> |.ffunc_1 pairs
> | mov TAB:RB, [BASE]
> | cmp dword [BASE+4], LJ_TTAB; jne ->fff_fallback
> -#if LJ_52
> +#if LJ_PAIRSMM
> | cmp dword TAB:RB->metatable, 0; jne ->fff_fallback
> #endif
> | mov CFUNC:RB, [BASE-8]
> @@ -1791,7 +1791,7 @@ static void build_subroutines(BuildCtx *ctx)
> |.ffunc_1 ipairs
> | mov TAB:RB, [BASE]
> | cmp dword [BASE+4], LJ_TTAB; jne ->fff_fallback
> -#if LJ_52
> +#if LJ_PAIRSMM
> | cmp dword TAB:RB->metatable, 0; jne ->fff_fallback
> #endif
> | mov CFUNC:RB, [BASE-8]
> diff --git a/test/mmpairs_test.lua b/test/mmpairs_test.lua
> new file mode 100755
> index 0000000..bb9835d
> --- /dev/null
> +++ b/test/mmpairs_test.lua
> @@ -0,0 +1,50 @@
> +#!/usr/bin/env tarantool
> +
> +tap = require('tap')
> +
> +test = tap.test("PAIRSMM-is-set")
> +test:plan(2)
> +
> +-- There is no Lua way to detect whether LJ_PAIRSMM is enabled. However, in
> +-- tarantool build it's enabled by default. So the test scenario is following:
> +-- while we can overload the pairs/ipairs behaviour via metamethod as designed
> +-- in Lua 5.2, os.execute still preserves the Lua 5.1 interface.
> +
> +local mt = {
> + __pairs = function(self)
> + local function stateless_iter(tbl, k)
> + local v
> + k, v = next(tbl, k)
> + while k and v > 0 do k, v = next(tbl, k) end
> + if v then return k,v end
> + end
> + return stateless_iter, self, nil
> + end,
> + __ipairs = function(self)
> + local function stateless_iter(tbl, k)
> + local v
> + k, v = next(tbl, k)
> + while k and v < 0 do k, v = next(tbl, k) end
> + if v then return k,v end
> + end
> + return stateless_iter, self, nil
> + end
> +}
> +
> +local t = setmetatable({ }, mt)
> +t[1] = 10
> +t[2] = 20
> +t[3] = -10
> +t[4] = -20
> +
> +local pairs_res = 0
> +local ipairs_res = 0
> +for k, v in pairs(t) do
> + pairs_res = v + pairs_res
> +end
> +for k, v in ipairs(t) do
> + ipairs_res = v + ipairs_res
> +end
> +test:is(pairs_res + ipairs_res, 0)
> +os_exec_res = os.execute()
> +test:is(os_exec_res, 1)
> --
> 2.20.1 (Apple Git-117)
>
[1]: https://github.com/tarantool/tarantool/blob/master/cmake/luajit.cmake
--
Best regards,
IM
next prev parent reply other threads:[~2019-11-22 11:23 UTC|newest]
Thread overview: 3+ messages / expand[flat|nested] mbox.gz Atom feed top
2019-11-22 10:00 Olga Arkhangelskaia
2019-11-22 11:21 ` Igor Munkin [this message]
2019-11-22 14:25 ` Kirill Yukhin
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20191122112142.GG18878@tarantool.org \
--to=imun@tarantool.org \
--cc=arkholga@tarantool.org \
--cc=tarantool-patches@dev.tarantool.org \
--subject='Re: [Tarantool-patches] [PATCH v4] build: introduced LUAJIT_ENABLE_PAIRSMM flag' \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox