Re: [Tarantool-patches] [PATCH luajit] Fix handling of instable types in TNEW/TDUP load forwarding.

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

Hi, Sergey!
Thanks for the patch!
LGTM
On Tue, Aug 29, 2023 at 12:54:40PM +0300, Sergey Kaplun wrote:
> From: Mike Pall <mike>
> 
> Analyzed by Sergey Kaplun.
> 
> (cherry-picked from commit 9f452bbef5031afc506d8615f5e720c45acd6fdf)
> 
> This is a follow-up for the commit
> a9d183b2be63fd91be4b8c9494c213c56c491092 ("Fix TNEW load forwarding with
> instable types."). It fixes a similar issue, but for TDUP load
> forwarding.
> 
> Sergey Kaplun:
> * added the description and the test for the problem
> 
> Part of tarantool/tarantool#8825
> ---
> 
> Branch: https://github.com/tarantool/luajit/tree/skaplun/lj-994-load-fwd-instable-types-tdup
> Tarantool PR: https://github.com/tarantool/tarantool/pull/9053
> Related issues:
> * https://github.com/LuaJIT/LuaJIT/issues/994
> * https://github.com/tarantool/tarantool/issues/8825
> 
>  src/lj_opt_mem.c                                          | 4 ++--
>  .../lj-994-instable-types-during-loop-unroll.test.lua     | 8 +-------
>  2 files changed, 3 insertions(+), 9 deletions(-)
> 
> diff --git a/src/lj_opt_mem.c b/src/lj_opt_mem.c
> index 59fddbdd..3c58d342 100644
> --- a/src/lj_opt_mem.c
> +++ b/src/lj_opt_mem.c
> @@ -194,8 +194,8 @@ static TRef fwd_ahload(jit_State *J, IRRef xref)
>  	if (key->o == IR_KSLOT) key = IR(key->op1);
>  	lj_ir_kvalue(J->L, &keyv, key);
>  	tv = lj_tab_get(J->L, ir_ktab(IR(ir->op1)), &keyv);
> -	lj_assertJ(itype2irt(tv) == irt_type(fins->t),
> -		   "mismatched type in constant table");
> +	if (itype2irt(tv) != irt_type(fins->t))
> +	  return 0;  /* Type instability in loop-carried dependency. */
>  	if (irt_isnum(fins->t))
>  	  return lj_ir_knum_u64(J, tv->u64);
>  	else if (LJ_DUALNUM && irt_isint(fins->t))
> diff --git a/test/tarantool-tests/lj-994-instable-types-during-loop-unroll.test.lua b/test/tarantool-tests/lj-994-instable-types-during-loop-unroll.test.lua
> index 730b0e61..6e2cf5ed 100644
> --- a/test/tarantool-tests/lj-994-instable-types-during-loop-unroll.test.lua
> +++ b/test/tarantool-tests/lj-994-instable-types-during-loop-unroll.test.lua
> @@ -10,7 +10,7 @@ local test = tap.test('lj-994-instable-types-during-loop-unroll'):skipcond({
>  
>  -- TODO: test that compiled traces don't always exit by the type
>  -- guard. See also the comment for the TDUP test chunk.
> -test:plan(1)
> +test:plan(2)
>  
>  -- TNEW.
>  local result
> @@ -34,11 +34,6 @@ end
>  test:is(result, nil, 'TNEW load forwarding was successful')
>  
>  -- TDUP.
> ---[[
> --- FIXME: Disable test case for now. Enable, with another
> --- backported commit with a fix for the aforementioned issue
> --- (and after patch "Improve assertions.").
> --- Test taken trace exits too.
>  for _ = 1, 5 do
>    local t = slot
>    -- Now use constant key slot to get necessary branch.
> @@ -48,6 +43,5 @@ for _ = 1, 5 do
>    slot = _ % 2 ~= 0 and stored_tab or {true}
>  end
>  test:is(result, true, 'TDUP load forwarding was successful')
> -]]
>  
>  test:done(true)
> -- 
> 2.42.0
>