[Tarantool-patches] [PATCH luajit v3 4/4] Fix IR_RENAME snapshot number. Follow-up fix for a32aeadc.
Sergey Kaplun
skaplun at tarantool.org
Wed Apr 5 08:33:20 MSK 2023
Hi, Maxim!
Thanks for the adding test-case!
LGTM, after fixing comments below.
On 28.03.23, Maksim Kokryashkin wrote:
> From: Mike Pall <mike>
>
> Reported by Victor Bombi, analyzed by XmiliaH. Thanks!
>
> (cherry-picked from commit bf51d3535109c4745bfbbe19a5587a9eac00259a)
>
> If the `snapalloc` flag is set, then the allocation hasn't
> occurred yet, meaning that rename is applied to the next
> snapshot. Otherwise, refs are already allocated and rename
> is applied to the current snapshot.
>
> Maxim Kokryashkin:
> * added the description and the test for the problem
>
> Part of tarantool/tarantool#7745
> Part of tarantool/tarantool#8069
> ---
> src/lj_asm.c | 9 ++++++++-
> .../gh-7745-ir-rename.test.lua | 20 +++++++++++++++++++
> 2 files changed, 28 insertions(+), 1 deletion(-)
> create mode 100644 test/tarantool-tests/gh-7745-ir-rename.test.lua
>
> diff --git a/src/lj_asm.c b/src/lj_asm.c
> index adfaf286..929a6da6 100644
> --- a/src/lj_asm.c
> +++ b/src/lj_asm.c
<snipped>
> diff --git a/test/tarantool-tests/gh-7745-ir-rename.test.lua b/test/tarantool-tests/gh-7745-ir-rename.test.lua
> new file mode 100644
> index 00000000..d03f6da5
> --- /dev/null
> +++ b/test/tarantool-tests/gh-7745-ir-rename.test.lua
Please, rename to mention the original issue, from LuaJIT repo.
Something like the following:
<lj-688-snap-ir-rename.test.lua>
> @@ -0,0 +1,20 @@
> +local tap = require('tap')
> +local test = tap.test('IR_RENAME on snapshot allocation'):skipcond({
Minor: I suggest to name test the same as its filename (just to be
consistent with other tests).
> + ['Test requires JIT enabled'] = not jit.status(),
> +})
> +test:plan(1)
> +
> +jit.opt.start('hotloop=1')
> +
> +local vals = {-0.1, 0.1, -0.1, 0.1}
> +local i = 1
> +local _
> +while i < 5 do
> + assert(i ~= 1.1)
> + local l1 = vals[i]
> + _ = l1 > 0
> + i = i + 1
Please, add comments near the lines emitted to interesting for us SNAP
and RENAME.
> +end
> +
> +test:ok(true, 'IR_RENAME is fine')
> +os.exit(test:check())
Minor: The comment about missing `and 0 or 1` is desirable.
> --
> 2.37.1 (Apple Git-137.1)
>
--
Best regards,
Sergey Kaplun
More information about the Tarantool-patches
mailing list