Re: [Tarantool-patches] [PATCH luajit] Prevent CSE of a REF_BASE operand across IR_RETF.

[Igor Munkin via Tarantool-patches <tarantool-patches@dev.tarantool.org>]

Sergey,

I've checked the patchset into all long-term branches in
tarantool/luajit and bumped a new version in master, release/2.11 and
release/2.10.

On 28.11.23, Sergey Kaplun via Tarantool-patches wrote:
> From: Mike Pall <mike>
> 
> Reported by XmiliaH.
> 
> (cherry-picked from commit e73916d811710ab02a4dfe447d621c99f4e7186c)
> 
> The RETF IR has a side effect: it shifts base when returning to a lower
> frame, i.e., it affects `REF_BASE` IR (0000) (thus, we can say that this
> IR is violating SSA form). So any optimization of IRs with `REF_BASE` as
> an operand across RETF IR may lead to incorrect optimizations (see
> details in the test file).
> 
> This patch adds rules to the folding engine to prevent CSE across `IR_RETF`
> for all possible IRs containing REF_BASE.
> 
> Sergey Kaplun:
> * added the description and the test for the problem
> 
> Part of tarantool/tarantool#9145
> ---
> 
> Branch: https://github.com/tarantool/luajit/tree/skaplun/lj-784-cse-ref-base-over-retf
> Tarantool PR: https://github.com/tarantool/tarantool/pull/9421
> Related issues:
> * https://github.com/LuaJIT/LuaJIT/issues/784
> * https://github.com/tarantool/tarantool/issues/9145
> 
> Interested reviewers can mention that only the `SUB any BASE` case is
> tested.
> The reason is that other cases are impossible to record in LuaJIT:
> * EQ any BASE: EQ pgc REF_BASE IR for upvalues is emitted when
>   the open upvalue aliases a SSA slot, i.e., it belongs to the frame of
>   the currently executed function. In that case, if we want to emit RETF
>   IR, we need to leave this function. So we need to record the UCLO
>   bytecode, which is NIY in JIT. So, such a type of trace is impossible.
> * SUB BASE any: SUB BASE fr is emitted for the recording of VARG
>   bytecode, in case varargs are undefined on trace. We need a vararg
>   function to call to create an additional frame. But returning to lower
>   frames from a vararg function isn't implemented in LuaJIT -- either
>   the trace recording is stopped or the error is rased and the trace
>   isn't compiled. Also, IINM, fr operands will always be different for
>   different frames, so there is no possible CSE here.
> 
> So, these cases are needed to prevent any regressions in the future.
> 
> Please correct me if I've missed something.
> 
>  src/lj_opt_fold.c                             | 11 +++
>  .../lj-784-cse-ref-base-over-retf.test.lua    | 86 +++++++++++++++++++
>  2 files changed, 97 insertions(+)
>  create mode 100644 test/tarantool-tests/lj-784-cse-ref-base-over-retf.test.lua
> 

<snipped>

> -- 
> 2.42.1
> 

-- 
Best regards,
IM