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

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

Hi, Sergey!
Thanks for the review!
Fixed your comment, and force-pushed the branch.

On 15.09.23, Sergey Bronnikov wrote:
> Hi, Sergey
> 
> thanks for the patch! LGTM
> 
> And see my minor comments below.
> 
> 
> On 8/29/23 12:54, Sergey Kaplun wrote:
> 
> <snipped>
> 
> >   -- 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
> Nit: assert is triggered with 3 iterations too

Yes, but I also want to test the trace execution. So, left it as is.

> >     slot = _ % 2 ~= 0 and stored_tab or {true}
> Nit: I would left a comment with explanation why exactly this value is 
> assigned to slot.

Added the corresponding comment.

===================================================================
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 6e2cf5ed..f240bdd2 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
@@ -40,6 +40,8 @@ for _ = 1, 5 do
   -- LJ_TRERR_GFAIL isn't triggered here.
   -- See `fwd_ahload()` in <src/lj_opt_mem.c> for details.
   result = t[1]
+  -- The constant table should contain the key with a different
+  -- type.
   slot = _ % 2 ~= 0 and stored_tab or {true}
 end
 test:is(result, true, 'TDUP load forwarding was successful')
===================================================================

> >   end
> >   test:is(result, true, 'TDUP load forwarding was successful')
> > -]]
> >   
> >   test:done(true)

-- 
Best regards,
Sergey Kaplun