Tarantool development patches archive
 help / color / mirror / Atom feed
From: Sergey Kaplun via Tarantool-patches <tarantool-patches@dev.tarantool.org>
To: Sergey Ostanevich <sergos@tarantool.org>
Cc: tarantool-patches@dev.tarantool.org
Subject: Re: [Tarantool-patches] [PATCH luajit] Fix bytecode register allocation for comparisons.
Date: Mon, 16 Aug 2021 19:44:39 +0300	[thread overview]
Message-ID: <YRqV968V7c3oisT3@root> (raw)
In-Reply-To: <0950C847-EF9A-4C62-920D-1F04AA7137CC@tarantool.org>

Hi, Sergos!

Thanks for the review!

On 10.08.21, Sergey Ostanevich wrote:
> Hi!
> Thanks for the patch! Just minor grammar, LGTM.

The new commit message is the following:

Fix bytecode register allocation for comparisons.

(cherry picked from commit 2f3f07882fb4ad9c64967d7088461b1ca0a25d3a)

When LuaJIT is built with LJ_FR2 (e.g. with GC64 mode enabled),
information about frame takes two slots -- the first takes the TValue
with the function to be called, the second takes the framelink. The JIT
recording machinery does pretty the same -- the function IR_KGC is
loaded in the first slot, and the second is set to TREF_FRAME value.
This value should be rewritten after return from a callee. This slot is
cleared either by return values or manually (set to zero), when there
are no values to return. The latter case is done by the next bytecode
with RA dst mode. This obliges that the destination of RA takes the next
slot after TREF_FRAME. Hence, this an earlier instruction must use the
smallest possible destination register (see `lj_record_ins()` for the

Bytecode emitter swaps operands for ISGT and ISGE comparisons. As a
result, the aforementioned rule for registers allocations may be
violated. When it happens for a chunk being recorded, the slot with
TREF_FRAME is not rewritten (but the next empty slot after TREF_FRAME
is). This leads to JIT slots inconsistency and assertion failure in
`rec_check_slots()` during recording of the next bytecode instruction.

This patch fixes bytecode register allocation by changing the VM
register allocation order in case of ISGT and ISGE bytecodes.

Sergey Kaplun:
* added the description and the test for the problem

Resolves tarantool/tarantool#6227
Part of tarantool/tarantool#5629

> Sergos


Best regards,
Sergey Kaplun

  reply	other threads:[~2021-08-16 16:45 UTC|newest]

Thread overview: 10+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-07-19  7:36 Sergey Kaplun via Tarantool-patches
2021-08-01 10:43 ` Igor Munkin via Tarantool-patches
2021-08-01 17:10   ` Sergey Kaplun via Tarantool-patches
2021-08-16  7:20     ` Igor Munkin via Tarantool-patches
2021-08-16 16:40       ` Sergey Kaplun via Tarantool-patches
2021-08-16 16:27         ` Igor Munkin via Tarantool-patches
2021-08-17  7:36           ` Vitaliia Ioffe via Tarantool-patches
2021-08-10 17:03 ` Sergey Ostanevich via Tarantool-patches
2021-08-16 16:44   ` Sergey Kaplun via Tarantool-patches [this message]
2021-08-17  9:24 ` Igor Munkin via Tarantool-patches

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=YRqV968V7c3oisT3@root \
    --to=tarantool-patches@dev.tarantool.org \
    --cc=sergos@tarantool.org \
    --cc=skaplun@tarantool.org \
    --subject='Re: [Tarantool-patches] [PATCH luajit] Fix bytecode register allocation for comparisons.' \


* 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