[Tarantool-patches] [PATCH luajit v2] Mark CONV as non-weak, to prevent elimination of its side-effect.
Sergey Bronnikov
sergeyb at tarantool.org
Tue Oct 3 19:24:03 MSK 2023
Hi, Max
thanks for the patch!
On 10/3/23 16:37, Maksim Kokryashkin wrote:
<snipped>
> --- /dev/null
> +++ b/test/tarantool-tests/mark-conv-non-weak.test.lua
> @@ -0,0 +1,115 @@
> +local tap = require('tap')
> +local test = tap.test('mark-conv-non-weak'):skipcond({
> + ['Test requires JIT enabled'] = not jit.status(),
> +})
> +
> +test:plan(1)
> +
> +local data = {0.1, 0, 0.1, 0, 0 / 0}
Is it possible to reduce a number of elements in the table?
I would add a comment where describe why exactly these magic values were
chosen.
> +local sum = 0
> +
> +jit.opt.start('hotloop=2', 'hotexit=2')
Why values are equal to 2 and not 1, that we usually set in tests?
> +
> +-- XXX: The test fails before the patch only
> +-- for `DUALNUM` mode. All of the IRs below are
> +-- produced by the corresponding LuaJIT build.
> +
> +-- When the trace is recorded, the IR
> +-- is the following before the patch:
> +---- TRACE 1 IR
> +-- .... SNAP #0 [ ---- ---- ---- ---- ---- ---- ---- ---- ---- ]
> +-- 0001 u8 XLOAD [0x100dac521] V
> +-- 0002 int BAND 0001 +12
> +-- 0003 > int EQ 0002 +0
> +-- 0004 > int SLOAD #8 T
> +-- .... SNAP #1 [ ---- ---- ---- ---- ---- ---- ---- ---- ---- ]
> +-- 0005 > num SLOAD #3 T
> +-- 0006 num CONV 0004 num.int
> +-- 0007 + num ADD 0006 0005
> +-- 0008 > fun SLOAD #4 T
> +-- 0009 > tab SLOAD #5 T
> +-- 0010 > int SLOAD #6 T
> +-- 0011 > fun EQ 0008 ipairs_aux
> +-- 0012 + int ADD 0010 +1
> +-- 0013 int FLOAD 0009 tab.asize
> +-- 0014 > int ABC 0013 0012
> +-- 0015 p64 FLOAD 0009 tab.array
> +-- 0016 p64 AREF 0015 0012
> +-- 0017 >+ num ALOAD 0016
> +-- .... SNAP #2 [ ---- ---- ---- 0007 ---- ---- 0012 0012 0017 ]
> +-- 0018 ------ LOOP ------------
> +-- 0019 u8 XLOAD [0x100dac521] V
> +-- 0020 int BAND 0019 +12
> +-- 0021 > int EQ 0020 +0
> +-- 0022 > int CONV 0017 int.num
> +-- .... SNAP #3 [ ---- ---- ---- 0007 ---- ---- 0012 0012 0017 ]
> +-- 0023 + num ADD 0017 0007
> +-- 0024 + int ADD 0012 +1
> +-- 0025 > int ABC 0013 0024
> +-- 0026 p64 AREF 0015 0024
> +-- 0027 >+ num ALOAD 0026
> +-- 0028 num PHI 0017 0027
> +-- 0029 num PHI 0007 0023
> +-- 0030 int PHI 0012 0024
> +---- TRACE 1 stop -> loop
> +
> +---- TRACE 1 exit 0
> +---- TRACE 1 exit 3
> +--
> +-- And the following after the patch:
> +---- TRACE 1 IR
> +-- .... SNAP #0 [ ---- ---- ---- ---- ---- ---- ---- ---- ---- ]
> +-- 0001 u8 XLOAD [0x102438521] V
> +-- 0002 int BAND 0001 +12
> +-- 0003 > int EQ 0002 +0
> +-- 0004 > int SLOAD #8 T
> +-- .... SNAP #1 [ ---- ---- ---- ---- ---- ---- ---- ---- ---- ]
> +-- 0005 > num SLOAD #3 T
> +-- 0006 num CONV 0004 num.int
> +-- 0007 + num ADD 0006 0005
> +-- 0008 > fun SLOAD #4 T
> +-- 0009 > tab SLOAD #5 T
> +-- 0010 > int SLOAD #6 T
> +-- 0011 > fun EQ 0008 ipairs_aux
> +-- 0012 + int ADD 0010 +1
> +-- 0013 int FLOAD 0009 tab.asize
> +-- 0014 > int ABC 0013 0012
> +-- 0015 p64 FLOAD 0009 tab.array
> +-- 0016 p64 AREF 0015 0012
> +-- 0017 >+ num ALOAD 0016
> +-- .... SNAP #2 [ ---- ---- ---- 0007 ---- ---- 0012 0012 0017 ]
> +-- 0018 ------ LOOP ------------
> +-- 0019 u8 XLOAD [0x102438521] V
> +-- 0020 int BAND 0019 +12
> +-- 0021 > int EQ 0020 +0
> +-- 0022 > int CONV 0017 int.num
> +-- .... SNAP #3 [ ---- ---- ---- 0007 ---- ---- 0012 0012 0017 ]
> +-- 0023 + num ADD 0017 0007
> +-- 0024 + int ADD 0012 +1
> +-- 0025 > int ABC 0013 0024
> +-- 0026 p64 AREF 0015 0024
> +-- 0027 >+ num ALOAD 0026
> +-- 0028 num PHI 0017 0027
> +-- 0029 num PHI 0007 0023
> +-- 0030 int PHI 0012 0024
> +---- TRACE 1 stop -> loop
> +
> +---- TRACE 1 exit 0
> +---- TRACE 1 exit 2
> +--
> +-- Before the patch, the `0022 > int CONV 0017 int.num`
I see that IR "0022 > int CONV ..." is present in both IR traces...
> +-- instruction is omitted due to DCE, which results in the
> +-- third side exit being taken, instead of the second,
> +-- and, hence, incorrect summation. After the patch, `CONV`
> +-- is left intact and is not omitted; it remains as a guarded
> +-- instruction, so the second side exit is taken and sum is
> +-- performed correctly.
> +
> +for _, val in ipairs(data) do
> + if val == val then
> + sum = sum + val
> + end
> +end
> +
> +test:ok(sum == sum, 'NaN check was not omitted')
> +test:done(true)
> --
> 2.39.3 (Apple Git-145)
>
More information about the Tarantool-patches
mailing list