Re: [Tarantool-patches] [PATCH luajit] Fix FOLD rule for BUFHDR append.

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

Hi, Maxim!
Thanks for the review!
Fixed your comments and force-pushed the branch.

On 17.11.23, Maxim Kokryashkin wrote:
> Hi, Sergey!
> LGTM, except for a few nits below.
> On Tue, Nov 14, 2023 at 06:04:55PM +0300, Sergey Kaplun wrote:

<snipped>

> > +-- XXX: Use 5 iterations to run variant part of the loop.
> Typo: s/variant/the variant/

Fixed.

> > +for _ = 1, 5 do
> > +  result = prefix .. 'a'
> > +  -- We need a non-constant string to be appended to prevent more
> > +  -- aggressive optimizations. Use an empty string for
> > +  -- convenience. Also, use a constant string in the first operand
> > +  -- in the concatenation operator for more readable `jit.dump`
> > +  -- output.
> > +  prefix = 'Lu' .. EMPTY_STR
> > +end
> > +
> > +test:is(result, 'Lua', 'skipped BUFPUT APPEND optimization for PHIs')
> The test description is a bit misleading. At first glance it reads like
> the `BUFPUT APPEND` itself is skipped. I suggest paraphrasing it like
> this:
> "BUFPUT APPEND optimization is not applied for PHIs"

Fixed, thanks!
See the iterative patch below:

===================================================================
diff --git a/test/tarantool-tests/lj-791-fold-bufhdr-append.test.lua b/test/tarantool-tests/lj-791-fold-bufhdr-append.test.lua
index b2422159..19be91a4 100644
--- a/test/tarantool-tests/lj-791-fold-bufhdr-append.test.lua
+++ b/test/tarantool-tests/lj-791-fold-bufhdr-append.test.lua
@@ -38,7 +38,7 @@ jit.opt.start('hotloop=1')
 -- Which appends to buffer instead of reseting, so the resulting
 -- string contains one more symbol.
 
--- XXX: Use 5 iterations to run variant part of the loop.
+-- XXX: Use 5 iterations to run the variant part of the loop.
 for _ = 1, 5 do
   result = prefix .. 'a'
   -- We need a non-constant string to be appended to prevent more
@@ -49,6 +49,6 @@ for _ = 1, 5 do
   prefix = 'Lu' .. EMPTY_STR
 end
 
-test:is(result, 'Lua', 'skipped BUFPUT APPEND optimization for PHIs')
+test:is(result, 'Lua', 'BUFPUT APPEND optimization is not applied for PHIs')
 
 test:done(true)
===================================================================

Branch is force-pushed.

> > +
> > +test:done(true)
> > --
> > 2.42.0
> >

-- 
Best regards,
Sergey Kaplun