Tarantool development patches archive
 help / color / mirror / Atom feed
From: Igor Munkin via Tarantool-patches <tarantool-patches@dev.tarantool.org>
To: Sergey Kaplun <skaplun@tarantool.org>
Cc: tarantool-patches@dev.tarantool.org
Subject: Re: [Tarantool-patches] [PATCH luajit] Prevent CSE of a REF_BASE operand across IR_RETF.
Date: Wed, 10 Jan 2024 08:51:51 +0000	[thread overview]
Message-ID: <ZZ5apz1VdykXePtv@tarantool.org> (raw)
In-Reply-To: <20231128122112.16229-1-skaplun@tarantool.org>


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

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


> -- 
> 2.42.1

Best regards,

      parent reply	other threads:[~2024-01-10  8:58 UTC|newest]

Thread overview: 6+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2023-11-28 12:21 Sergey Kaplun via Tarantool-patches
2023-11-29 14:26 ` Sergey Bronnikov via Tarantool-patches
2023-11-30  7:34   ` Sergey Kaplun via Tarantool-patches
2023-11-30  8:53     ` Sergey Bronnikov via Tarantool-patches
2023-11-30 17:59     ` Maxim Kokryashkin via Tarantool-patches
2024-01-10  8:51 ` Igor Munkin via Tarantool-patches [this message]

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=ZZ5apz1VdykXePtv@tarantool.org \
    --to=tarantool-patches@dev.tarantool.org \
    --cc=imun@tarantool.org \
    --cc=skaplun@tarantool.org \
    --subject='Re: [Tarantool-patches] [PATCH luajit] Prevent CSE of a REF_BASE operand across IR_RETF.' \


* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox