Re: [Tarantool-patches] [PATCH luajit v2] Mark CONV as non-weak, to prevent elimination of its side-effect.

[Sergey Bronnikov via Tarantool-patches <tarantool-patches@dev.tarantool.org>]

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