[Tarantool-patches] [PATCH] build: add missing module for jit.dump on ARM64
Sergey Kaplun
skaplun at tarantool.org
Tue May 18 10:14:03 MSK 2021
Igor,
On 17.05.21, Igor Munkin wrote:
> Oops, sorry guys, but the test seems to be noop for this fix (in other
> works it's OK even without patch), so I've reimplemented it. Consider
> the description in the test. Diff is below:
Thanks for update!
Please consider my comments below.
Side note: Please, rebase your branch to master to make it buildable.
>
> ================================================================================
>
> diff --git a/test/app-tap/gh-5983-jit-library-smoke-tests.skipcond b/test/app-tap/gh-5983-jit-library-smoke-tests.skipcond
> new file mode 100644
> index 000000000..2a2ec4d97
> --- /dev/null
> +++ b/test/app-tap/gh-5983-jit-library-smoke-tests.skipcond
> @@ -0,0 +1,7 @@
> +import platform
> +
> +# Disabled on FreeBSD due to #4819.
> +if platform.system() == 'FreeBSD':
> + self.skip = 1
> +
> +# vim: set ft=python:
> 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..72caec2f9
> --- /dev/null
> +++ b/test/app-tap/gh-5983-jit-library-smoke-tests.test.lua
> @@ -0,0 +1,44 @@
> +#!/usr/bin/env tarantool
> +
> +-- Just check whether all Lua sources related to jit.dump are
> +-- bundled to the binary. Otherwise, jit.dump module raises
> +-- an error that is handled via <pcall>.
> +-- XXX: pure require for jit.dump doesn't cover all the cases,
> +-- since dis_<arch>.lua are loaded at runtime. Furthermore, this
> +-- error is handled by JIT engine, so we can't use <pcall> for it.
> +-- Hence, simply sniff the output of the test to check that all
> +-- phases of trace compilation are dumped.
> +
> +if #arg == 0 then
> + local tap = require('tap')
> + local test = tap.test('gh-5983-jit-library-smoke-tests')
> +
> + test:plan(1)
> +
> + -- XXX: Shell argument <test> is necessary to differ test case
> + -- from the test runner.
> + local cmd = string.gsub('<LUABIN> 2>&1 <SCRIPT> test', '%<(%w+)>', {
> + LUABIN = arg[-1],
> + SCRIPT = arg[0],
> + })
> + local proc = io.popen(cmd)
> + local got = proc:read('*all'):gsub('^%s+', ''):gsub('%s+$', '')
> + local expected = table.concat({
> + '---- TRACE %d start',
> + '---- TRACE %d IR',
> + '---- TRACE %d mcode',
> + '---- TRACE %d stop',
> + '---- TRACE %d exit',
> + }, '.+')
> +
> + test:like(got, expected , 'jit.dump smoke tests')
> +
> + os.exit(test:check() and 0 or 1)
> +end
> +
> +-- Use *all* jit.dump options, so the test can check them all.
> +require('jit.dump').start('+tbisrmXaT')
> +-- Tune JIT engine to make the test faster and more robust.
Side note: Do you check that it is really faster than 57 iterations:)?
> +jit.opt.start('hotloop=1')
Also, this line leads to test failure:
| $ ./test-run.py app-tap/gh-5983-jit-library-smoke-tests.test.lua
| ...
| [001] app-tap/gh-5983-jit-library-smoke-tests.test.l> [ fail ]
| [001] Test failed! Output from reject file var/rejects/app-tap/gh-5983-jit-library-smoke-tests.reject:
| [001] TAP version 13
| [001] 1..1
| [001] not ok - jit.dump smoke tests
| [001] ---
| [001] filename: /Users/tntmac07.tarantool.i/s.kaplun/tarantool/master/test/app-tap/gh-5983-jit-library-smoke-tests.test.lua
| [001] trace:
| [001] - line: 0
| [001] source: '@/Users/tntmac07.tarantool.i/s.kaplun/tarantool/master/test/app-tap/gh-5983-jit-library-smoke-tests.test.lua'
| [001] filename: /Users/tntmac07.tarantool.i/s.kaplun/tarantool/master/test/app-tap/gh-5983-jit-library-smoke-tests.test.lua
| [001] what: main
| [001] namewhat:
| [001] src: '...er/test/app-tap/gh-5983-jit-library-smoke-tests.test.lua'
| [001] line: 0
| [001] expected: '---- TRACE %d start.+---- TRACE %d IR.+---- TRACE %d mcode.+---- TRACE
| [001] %d stop.+---- TRACE %d exit'
| [001] got: 'LuajitError: ...er/test/app-tap/gh-5983-jit-library-smoke-tests.test.lua:42:
| [001] attempt to index field ''opt'' (a nil value)
| [001] fatal error, exiting the event loop'
| [001] ...
| [001] # failed subtest: 1
So, I suggest to drop it for now.
But even with the following patch
===================================================================
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
index 72caec2f9..8ff234a12 100755
--- a/test/app-tap/gh-5983-jit-library-smoke-tests.test.lua
+++ b/test/app-tap/gh-5983-jit-library-smoke-tests.test.lua
@@ -39,6 +39,5 @@ end
-- Use *all* jit.dump options, so the test can check them all.
require('jit.dump').start('+tbisrmXaT')
-- Tune JIT engine to make the test faster and more robust.
-jit.opt.start('hotloop=1')
-- Record primitive loop.
-for _ = 1, 3 do end
+for _ = 1, 100 do end
===================================================================
this test fails:
| $ ./test-run.py app-tap/gh-5983-jit-library-smoke-tests.test.lua
| ...
| [001] app-tap/gh-5983-jit-library-smoke-tests.test.l> [ fail ]
| [001] Test failed! Output from reject file var/rejects/app-tap/gh-5983-jit-library-smoke-tests.reject:
| [001] TAP version 13
| [001] 1..1
| [001] not ok - jit.dump smoke tests
| [001] ---
| [001] filename: /Users/tntmac07.tarantool.i/s.kaplun/tarantool/master/test/app-tap/gh-5983-jit-library-smoke-tests.test.lua
| [001] trace:
| [001] - line: 0
| [001] source: '@/Users/tntmac07.tarantool.i/s.kaplun/tarantool/master/test/app-tap/gh-5983-jit-library-smoke-tests.test.lua'
| [001] filename: /Users/tntmac07.tarantool.i/s.kaplun/tarantool/master/test/app-tap/gh-5983-jit-library-smoke-tests.test.lua
| [001] what: main
| [001] namewhat:
| [001] src: '...er/test/app-tap/gh-5983-jit-library-smoke-tests.test.lua'
| [001] line: 0
| [001] expected: '---- TRACE %d start.+---- TRACE %d IR.+---- TRACE %d mcode.+---- TRACE
| [001] %d stop.+---- TRACE %d exit'
| [001] got:
| [001] ...
| [001] # failed subtest: 1
What am I doing wrong?
> +-- Record primitive loop.
> +for _ = 1, 3 do end
>
> ================================================================================
>
> CI is green[1] now.
>
> On 04.05.21, Igor Munkin 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 at 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
> >
>
> [1]: https://github.com/tarantool/tarantool/commit/18a4018
>
> --
> Best regards,
> IM
--
Best regards,
Sergey Kaplun
More information about the Tarantool-patches
mailing list