Re: [Tarantool-patches] [PATCH] build: add missing module for jit.dump on ARM64

[Igor Munkin via Tarantool-patches <tarantool-patches@dev.tarantool.org>]

Sergey,

On 18.05.21, Sergey Kaplun 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.

It was rebased prior to sending the updates. The reason this patch fails
to build on M1 is that you're missing the other patches (also not in
master branch yet).

> 
> > 
> > ================================================================================
> > 

<snipped>

> > 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:)?

Yes, of course. I'm not that bad in math: 3 is less than 100 and 2 is
less than 57. Furthermore, I don't need the trace being executed, only
recorded, so only 3 iterations are executed. Is it faster? Faster than
light it is! ;)

> 
> > +jit.opt.start('hotloop=1')
> 
> Also, this line leads to test failure:

I know. This is fixed here[1].

> 
> | $ ./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.

Why? As I see below, even after your changes the test doesn't work.

> 
> 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?

Testing this patchset on M1. Use GNU/Linux ARM64 hosts for now.

> 
> > +-- Record primitive loop.
> > +for _ = 1, 3 do end
> > 
> > ================================================================================
> > 
> > CI is green[1] now.
> > 

<snipped>

> > 
> > [1]: https://github.com/tarantool/tarantool/commit/18a4018
> > 
> > -- 
> > Best regards,
> > IM
> 
> -- 
> Best regards,
> Sergey Kaplun

[1]: https://lists.tarantool.org/tarantool-patches/cover.1620678384.git.imun@tarantool.org/T/#t

-- 
Best regards,
IM