Re: [Tarantool-patches] [PATCH luajit] Maintain chain invariant in DCE.

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

Hi, Maxim!
Thanks for the review!

On 26.05.24, Maxim Kokryashkin wrote:
> Hi, Sergey!
> Thanks for the patch!
> Please consider my comments below.
> 
> On Fri, May 17, 2024 at 04:29:18PM UTC, Sergey Kaplun wrote:
> > From: Mike Pall <mike>
> >
> > Thanks to Peter Cawley.
> >
> > (cherry picked from commit f72c19e482b6f918b7cf42b0436e2b117d160a29)
> >
> > Instructions with strong guards that are sometimes emitted with a guard
> > and sometimes emitted without a guard (like HREFK, CONV, or SLOAD) may
> > be eliminated from the IR chain and replaced with the NOP IR. If the
> > next IR of the same kind on the trace is not eliminated, it may
> > reference the IR NOP instead of an instruction of the same type. This
> > may lead to the corresponding assertion failure in the `rec_check_ir()`.
> >
> > This patch unconditionally links the IRs during chain maintenance in
> > DCE.
> >
> > Sergey Kaplun:
> > * added the description and the test for the problem
> >
> > Part of tarantool/tarantool#9924
> > ---
> >
> > Be aware that the reproducer from the ticket does not lead to the
> > assertion failure (this is why it is omitted in the test). I suppose it
> > just illustrates the situation when the IR is left off the chain.
> > Although the reproducer is clumsy, I can't simplify it or make it less
> > tricky. Please, ideas are welcome :).
> 
> The test doesn't reproduce before the patch on ARM64. Tested on M1 with
> the following flags:
> | -DCMAKE_BUILD_TYPE=Debug -DLUA_USE_ASSERT=ON \
> | -DLUAJIT_ENABLE_CHECKHOOK=ON`

I suppose this is due to the enabled by default DUALNUM mode, so there
are no needed CONV IRs, so there are no IR chains built incorrectly.

<snipped>

-- 
Best regards,
Sergey Kaplun