Re: [Tarantool-patches] [PATCH luajit 1/2] Optimize table.new() with constant args to (sinkable) IR_TNEW.

[Maxim Kokryashkin via Tarantool-patches <tarantool-patches@dev.tarantool.org>]

Hi, Sergey!
Thanks for the patch!

Very thorough testing!

LGTM, except for a minor comment below.

On Wed, Jan 24, 2024 at 05:11:08PM +0300, Sergey Kaplun wrote:
> From: Mike Pall <mike>
>
> Thanks to Peter Cawley.
>
> (cherry picked from commit d1236a4caa999b29e774ef5103df3b424d821d9b)
>
> This patch adds optimization for calls of `table.new()` with constant
> argument refs when asize is in the proper range (i.e., is less than IR
> operand width). The call is replaced with IR TNEW. This opens up
> opportunities for other optimizations in the pipeline.
>
> Sergey Kaplun:
> * added the description and the test for the feature
>
> Part of tarantool/tarantool#9595

<snipped>

> diff --git a/test/tarantool-tests/lj-1128-table-new-opt-tnew.test.lua b/test/tarantool-tests/lj-1128-table-new-opt-tnew.test.lua
> new file mode 100644
> index 00000000..805e6de6
> --- /dev/null
> +++ b/test/tarantool-tests/lj-1128-table-new-opt-tnew.test.lua
> @@ -0,0 +1,112 @@
> +local tap = require('tap')
> +local test = tap.test('lj-1128-table-new-opt-tnew'):skipcond({
> +  ['Test requires JIT enabled'] = not jit.status(),
> +  ['Disabled on *BSD due to #4819'] = jit.os == 'BSD',
> +})
> +
> +-- Test LuaJIT optimization when a call to the `lj_tab_new_ah()`
> +-- is replaced with the corresponding TNEW IR.
> +-- See also https://github.com/LuaJIT/LuaJIT/issues/1128.
> +
> +local jparse = require('utils').jit.parse
> +
> +-- API follows the semantics of `lua_createtable()`.
> +local table_new = require('table.new')
> +
> +-- `hbits` for different `hsizes`, see <lj_tab.h> for details.
> +local HBITS = {
> +  [1] = 1,
> +  [3] = 2,
> +}
> +
> +-- XXX: Avoid any other traces compilation due to hotcount
> +-- collisions for predictable results.
> +jit.off()
> +jit.flush()
> +
> +test:plan(10)
> +
> +jit.on()
> +jit.opt.start('hotloop=1')
> +jparse.start('i')
> +
> +local anchor
> +
> +for _ = 1, 4 do
> +  anchor = table_new(1, 1)
> +end
> +
> +local traces = jparse.finish()
> +jit.off()
> +
> +test:ok(type(anchor) == 'table', 'base result')
I would prefer the assertions for the type to have different
descritions for each test case. This way it is easier to tell
which test you need to focus on, if one of them fails. Here
and below.
> +test:ok(traces[1]:has_ir(('TNEW.*#2.*#%d'):format(HBITS[1])), 'base IR value')
> +
> +jit.flush()
> +jit.on()
> +-- XXX: Reset hotcounters.
> +jit.opt.start('hotloop=1')
> +jparse.start('i')
> +
> +for _ = 1, 4 do
> +  anchor = table_new(0, 0)
> +end
> +
> +traces = jparse.finish()
> +jit.off()
> +
> +test:ok(type(anchor) == 'table', 'base result')
> +test:ok(traces[1]:has_ir('TNEW.*#0.*#0'), '0 asize, 0 hsize')
> +
> +jit.flush()
> +jit.on()
> +-- XXX: Reset hotcounters.
> +jit.opt.start('hotloop=1')
> +jparse.start('i')
> +
> +for _ = 1, 4 do
> +  anchor = table_new(0, 3)
> +end
> +
> +traces = jparse.finish()
> +jit.off()
> +
> +test:ok(type(anchor) == 'table', 'base result')
> +test:ok(traces[1]:has_ir(('TNEW.*#0.*#%d'):format(HBITS[3])),
> +        '3 hsize -> 2 hbits')
> +
> +jit.flush()
> +jit.on()
> +-- XXX: Reset hotcounters.
> +jit.opt.start('hotloop=1')
> +jparse.start('i')
> +
> +for _ = 1, 4 do
> +  anchor = table_new(-1, 0)
> +end
> +
> +traces = jparse.finish()
> +jit.off()
> +
> +test:ok(type(anchor) == 'table', 'base result')
> +test:ok(traces[1]:has_ir('TNEW.*#0.*#0'), 'negative asize')
> +
> +jit.flush()
> +jit.on()
> +-- XXX: Reset hotcounters.
> +jit.opt.start('hotloop=1')
> +jparse.start('i')
> +
> +for _ = 1, 4 do
> +  anchor = table_new(0xffff, 0)
> +end
> +
> +traces = jparse.finish()
> +jit.off()
> +
> +test:ok(type(anchor) == 'table', 'base result')
> +-- Test that TNEW isn't emitted for `asize` bigger than the IR
> +-- operand width (>0x8000).
> +test:ok(not traces[1]:has_ir('TNEW'), 'asize out of range')
> +
> +test:done(true)
> --
> 2.43.0
>