[Tarantool-patches] [PATCH] build: add missing module for jit.dump on ARM64
Sergey Ostanevich
sergos at tarantool.org
Tue May 18 18:34:17 MSK 2021
Hi!
Thanks for the patch!
My test run passes:
$ ./test-run.py 5983
Statistics:
* pass: 1
As for comment on opt.start - I’m not sure if we will ever change the threshold
for optimizations - better to keep it under control and do not rely on a magic 100
loop counter.
But the essence of the test is not clear to me: we just verify that on the host
platform (the one testing is done) jit.dump is operable? I believe it should be
put into original message then - ’to enable jit.dump on ARM64…'
Otherwise - LGTM.
Sergos
> On 18 May 2021, at 10:14, Sergey Kaplun <skaplun at tarantool.org> wrote:
>
> 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
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://lists.tarantool.org/pipermail/tarantool-patches/attachments/20210518/213c5947/attachment.htm>
More information about the Tarantool-patches
mailing list