Tarantool development patches archive
 help / color / mirror / Atom feed
From: Sergey Ostanevich via Tarantool-patches <tarantool-patches@dev.tarantool.org>
To: Igor Munkin <imun@tarantool.org>
Cc: tarantool-patches@dev.tarantool.org
Subject: Re: [Tarantool-patches] [PATCH] build: add missing module for jit.dump on ARM64
Date: Fri, 14 May 2021 16:13:35 +0300	[thread overview]
Message-ID: <A6139E58-89E5-45BB-B877-A17BDF02E10F@tarantool.org> (raw)
In-Reply-To: <72c91a259dd039fc95961992ae06baee820695be.1620072340.git.imun@tarantool.org>

Thanks for the patch,

LGTM.

As for the plurality of dis modules - can we make decision at load time,
same as in dis_x64? So we’d introduce a set of dis_<target>.lua in Tarantool
sources that will make a check if target is valid and load the luajit one.
Definitely - not in this patch.

> On 4 May 2021, at 11:29, Igor Munkin <imun@tarantool.org> wrote:
> 
> Since commit c9d88d5f48054d78727b587fef7567422cdc39a6 ('Fix #984: add
> jit.* library to the binary') all required modules implemented in Lua
> are bundled (i.e. compiled into the executable as a C literal) into
> Tarantool binary. While making Tarantool work on ARM64 platforms, it
> turned out the arch-specific module (namely, jit/dis_arm64.lua) is not
> bundled. Within this patch the missing sources are added and jit.dump
> loads fine on ARM64 hosts as a result.
> 
> Part of #5983
> Relates to #5629
> Follows up #984
> 
> Signed-off-by: Igor Munkin <imun@tarantool.org>
> ---
> 
> Branch: https://github.com/tarantool/tarantool/tree/imun/gh-5983-add-jit-dump-on-arm64
> Issue: https://github.com/tarantool/tarantool/issues/5983
> 
> CI looks to be OK[1] except the known problems with ASAN[2].
> 
> [1]: https://github.com/tarantool/tarantool/commit/be184b2
> [2]: https://github.com/tarantool/tarantool/issues/6031
> 
> src/CMakeLists.txt                                 |  1 +
> src/lua/init.c                                     |  2 ++
> .../gh-5983-jit-library-smoke-tests.test.lua       | 14 ++++++++++++++
> 3 files changed, 17 insertions(+)
> create mode 100755 test/app-tap/gh-5983-jit-library-smoke-tests.test.lua
> 
> diff --git a/src/CMakeLists.txt b/src/CMakeLists.txt
> index 9005a37d6..f7a776986 100644
> --- a/src/CMakeLists.txt
> +++ b/src/CMakeLists.txt
> @@ -54,6 +54,7 @@ lua_source(lua_sources lua/swim.lua)
> # LuaJIT jit.* library
> lua_source(lua_sources ${LUAJIT_SOURCE_ROOT}/src/jit/bc.lua)
> lua_source(lua_sources ${LUAJIT_SOURCE_ROOT}/src/jit/bcsave.lua)
> +lua_source(lua_sources ${LUAJIT_SOURCE_ROOT}/src/jit/dis_arm64.lua)
> lua_source(lua_sources ${LUAJIT_SOURCE_ROOT}/src/jit/dis_x86.lua)
> lua_source(lua_sources ${LUAJIT_SOURCE_ROOT}/src/jit/dis_x64.lua)
> lua_source(lua_sources ${LUAJIT_SOURCE_ROOT}/src/jit/dump.lua)
> diff --git a/src/lua/init.c b/src/lua/init.c
> index 3358b7136..dfae4afb7 100644
> --- a/src/lua/init.c
> +++ b/src/lua/init.c
> @@ -106,6 +106,7 @@ extern char strict_lua[],
> 	vmdef_lua[],
> 	bc_lua[],
> 	bcsave_lua[],
> +	dis_arm64_lua[],
> 	dis_x86_lua[],
> 	dis_x64_lua[],
> 	dump_lua[],
> @@ -167,6 +168,7 @@ static const char *lua_modules[] = {
> 	"jit.vmdef", vmdef_lua,
> 	"jit.bc", bc_lua,
> 	"jit.bcsave", bcsave_lua,
> +	"jit.dis_arm64", dis_arm64_lua,
> 	"jit.dis_x86", dis_x86_lua,
> 	"jit.dis_x64", dis_x64_lua,
> 	"jit.dump", dump_lua,
> diff --git a/test/app-tap/gh-5983-jit-library-smoke-tests.test.lua b/test/app-tap/gh-5983-jit-library-smoke-tests.test.lua
> new file mode 100755
> index 000000000..ab42fbebf
> --- /dev/null
> +++ b/test/app-tap/gh-5983-jit-library-smoke-tests.test.lua
> @@ -0,0 +1,14 @@
> +#!/usr/bin/env tarantool
> +
> +local tap = require('tap')
> +
> +local test = tap.test('gh-5983-jit-library-smoke-tests')
> +test:plan(1)
> +
> +-- Just check whether all Lua sources related to jit.dump are
> +-- bundled to the binary. Otherwise, loading jit.dump module
> +-- raises an error that is handled via <pcall> and passed as a
> +-- second argument to the assertion.
> +test:ok(pcall(require, 'jit.dump'))
> +
> +os.exit(test:check() and 0 or 1)
> -- 
> 2.25.0
> 


  parent reply	other threads:[~2021-05-14 13:13 UTC|newest]

Thread overview: 12+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-05-04  8:29 Igor Munkin via Tarantool-patches
2021-05-11  7:08 ` Sergey Kaplun via Tarantool-patches
2021-05-11  8:28   ` Igor Munkin via Tarantool-patches
2021-05-14 13:13 ` Sergey Ostanevich via Tarantool-patches [this message]
2021-05-17  8:39   ` Igor Munkin via Tarantool-patches
2021-05-17 16:34 ` Igor Munkin via Tarantool-patches
2021-05-18  7:14   ` Sergey Kaplun via Tarantool-patches
2021-05-18 15:34     ` Sergey Ostanevich via Tarantool-patches
2021-05-19 13:04       ` Igor Munkin via Tarantool-patches
2021-05-19 12:19     ` Igor Munkin via Tarantool-patches
2021-05-19 16:55       ` Sergey Kaplun via Tarantool-patches
2021-05-19 17:20 ` Igor Munkin via Tarantool-patches

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=A6139E58-89E5-45BB-B877-A17BDF02E10F@tarantool.org \
    --to=tarantool-patches@dev.tarantool.org \
    --cc=imun@tarantool.org \
    --cc=sergos@tarantool.org \
    --subject='Re: [Tarantool-patches] [PATCH] build: add missing module for jit.dump on ARM64' \
    /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