From: sergos via Tarantool-patches <tarantool-patches@dev.tarantool.org>
To: Sergey Kaplun <skaplun@tarantool.org>
Cc: tarantool-patches@dev.tarantool.org
Subject: Re: [Tarantool-patches] [PATCH luajit] FFI: Add tonumber() specialization for failed conversions.
Date: Mon, 26 Sep 2022 00:37:31 +0300 [thread overview]
Message-ID: <304DA549-389D-468A-80F5-7424FF84FCF9@tarantool.org> (raw)
In-Reply-To: <YyxGy4TDF3BzoBo9@root>
[-- Attachment #1: Type: text/plain, Size: 2193 bytes --]
Since we fallback to interpreter it looks safe to keep the test for GC64.
Still you’d better mention it in commit message.
LGTM with above.
Sergos
> On 22 Sep 2022, at 14:28, Sergey Kaplun <skaplun@tarantool.org> wrote:
>
> Hi, Sergos!
>
> On 21.09.22, sergos wrote:
>> Hi!
>>
>> I see four ‘ok’s as a result of run.
>> See the full output for the run with dump() below:
>>
>>
>> s.ostanevich@s-ostanevich2:~/workspaces/t.sergos/third_party/luajit/test/tarantool-tests % ../../../../build-debug/src/tarantool -e 'require"jit.dump".start("ib")' lj-408-tonumber-cdata-record.test.lua
>>
>> TAP version 13
>> 1..4
>> ---- TRACE 1 start lj-408-tonumber-cdata-record.test.lua:17
>> 0001 GGET 1 0 ; "tonumber"
>> 0002 MOV 3 0
>> 0003 CALL 1 2 2
>> 0000 . FUNCC ; tonumber
>> 0004 RET1 1 2
>> ---- TRACE 1 IR
>> 0001 fun SLOAD #0 R
>> 0002 tab FLOAD 0001 func.env
>> 0003 int FLOAD 0002 tab.hmask
>> 0004 > int EQ 0003 +63
>> 0005 p64 FLOAD 0002 tab.node
>> 0006 > p64 HREFK 0005 "tonumber" @8
>> 0007 > fun HLOAD 0006
>> 0008 > cdt SLOAD #2 T
>> 0009 > fun EQ 0007 tonumber
>> ---- TRACE 1 stop -> return
>
> Gotcha!
> It's a funny thing:
> This is happening due to GC64 mode on x86_x64.
>
> There is an invalid type check for SLOAD IR with the following emitted
> mcode:
>
> | 55557f6bffc9 mov rdi, [rdx+0x4]
> | 55557f6bffcd sar rdi, 0x2f
> | 55557f6bffd1 cmp edi, -0x0b
> | 55557f6bffd4 jnz 0x55557f6b0010 ->0
>
> This 0x4 addiction is crucial:
> We got the invalid irtype value to compare (due wrong addressing) -- so
> the assertion guard is always failed and we always exit from the trace.
> This is why we fallback into the interpreter and the test is passed.
>
> In LuaJIT upstream this issue fixed via the following commit:
> https://github.com/LuaJIT/LuaJIT/commit/05fbdf56 <https://github.com/LuaJIT/LuaJIT/commit/05fbdf56>
>
> See also https://github.com/LuaJIT/LuaJIT/pull/350 <https://github.com/LuaJIT/LuaJIT/pull/350>
>
> <snipped>
>
>>
>
> --
> Best regards,
> Sergey Kaplun
[-- Attachment #2: Type: text/html, Size: 23682 bytes --]
next prev parent reply other threads:[~2022-09-25 21:37 UTC|newest]
Thread overview: 12+ messages / expand[flat|nested] mbox.gz Atom feed top
2022-09-12 8:01 Sergey Kaplun via Tarantool-patches
2022-09-19 7:50 ` Maxim Kokryashkin via Tarantool-patches
2022-09-20 8:53 ` sergos via Tarantool-patches
2022-09-20 9:58 ` Maxim Kokryashkin via Tarantool-patches
2022-09-20 10:10 ` sergos via Tarantool-patches
2022-09-21 12:03 ` Sergey Kaplun via Tarantool-patches
2022-09-21 12:19 ` sergos via Tarantool-patches
2022-09-22 11:28 ` Sergey Kaplun via Tarantool-patches
2022-09-25 21:37 ` sergos via Tarantool-patches [this message]
2022-09-28 7:37 ` Sergey Kaplun via Tarantool-patches
2022-09-24 14:49 ` Sergey Kaplun via Tarantool-patches
2022-11-11 8:54 ` 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=304DA549-389D-468A-80F5-7424FF84FCF9@tarantool.org \
--to=tarantool-patches@dev.tarantool.org \
--cc=sergos@tarantool.org \
--cc=skaplun@tarantool.org \
--subject='Re: [Tarantool-patches] [PATCH luajit] FFI: Add tonumber() specialization for failed conversions.' \
/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