Tarantool development patches archive
 help / color / mirror / Atom feed
From: Maxim Kokryashkin via Tarantool-patches <tarantool-patches@dev.tarantool.org>
To: tarantool-patches@dev.tarantool.org, skaplun@tarantool.org,
	sergos@tarantool.org
Subject: [Tarantool-patches] [PATCH luajit v2 5/5] Fix IR_RENAME snapshot number. Follow-up fix for a32aeadc.
Date: Tue, 14 Mar 2023 15:01:47 +0300	[thread overview]
Message-ID: <4d5e9afef83bdbacb2b5f5ca140e941ced04b36f.1678794695.git.m.kokryashkin@tarantool.org> (raw)
In-Reply-To: <cover.1678794695.git.m.kokryashkin@tarantool.org>

From: Mike Pall <mike>

Reported by Victor Bombi, analyzed by XmiliaH. Thanks!

(cherry-picked from commit bf51d3535109c4745bfbbe19a5587a9eac00259a)

If the `snapalloc` flag is set, then the allocation hasn't
occurred yet, meaning that rename is applied to the next
snapshot. Otherwise, refs are already allocated and rename
is applied to the current snapshot.

Maxim Kokryashkin:
* added the description and the test for the problem

Part of tarantool/tarantool#7745
Part of tarantool/tarantool#8069
---
Note: The part with os.exit(test:check() and 0 or 1) replaced by
os.exit(test:check()) is required, since in the first case, when
the trace is recorded, arithmetic register is not allocated yet
by the time th `ra_left` is called. Because of that, the PHI
optimization occurs differently and there is no RENAME.

 src/lj_asm.c                                  |  9 ++++++++-
 .../gh-7745-ir-rename.test.lua                | 20 +++++++++++++++++++
 2 files changed, 28 insertions(+), 1 deletion(-)
 create mode 100644 test/tarantool-tests/gh-7745-ir-rename.test.lua

diff --git a/src/lj_asm.c b/src/lj_asm.c
index adfaf286..929a6da6 100644
--- a/src/lj_asm.c
+++ b/src/lj_asm.c
@@ -682,7 +682,14 @@ static void ra_rename(ASMState *as, Reg down, Reg up)
   RA_DBGX((as, "rename    $f $r $r", regcost_ref(as->cost[up]), down, up));
   emit_movrr(as, ir, down, up);  /* Backwards codegen needs inverse move. */
   if (!ra_hasspill(IR(ref)->s)) {  /* Add the rename to the IR. */
-    ra_addrename(as, down, ref, as->snapno);
+    /*
+    ** The rename is effective at the subsequent (already emitted) exit
+    ** branch. This is for the current snapshot (as->snapno). Except if we
+    ** haven't yet allocated any refs for the snapshot (as->snapalloc == 1),
+    ** then it belongs to the next snapshot.
+    ** See also the discussion at asm_snap_checkrename().
+    */
+    ra_addrename(as, down, ref, as->snapno + as->snapalloc);
   }
 }
 
diff --git a/test/tarantool-tests/gh-7745-ir-rename.test.lua b/test/tarantool-tests/gh-7745-ir-rename.test.lua
new file mode 100644
index 00000000..d03f6da5
--- /dev/null
+++ b/test/tarantool-tests/gh-7745-ir-rename.test.lua
@@ -0,0 +1,20 @@
+local tap = require('tap')
+local test = tap.test('IR_RENAME on snapshot allocation'):skipcond({
+  ['Test requires JIT enabled'] = not jit.status(),
+})
+test:plan(1)
+
+jit.opt.start('hotloop=1')
+
+local vals = {-0.1, 0.1, -0.1, 0.1}
+local i = 1
+local _
+while i < 5 do
+    assert(i ~= 1.1)
+    local l1 = vals[i]
+    _ = l1 > 0
+    i = i + 1
+end
+
+test:ok(true, 'IR_RENAME is fine')
+os.exit(test:check())
-- 
2.39.0


      parent reply	other threads:[~2023-03-14 12:04 UTC|newest]

Thread overview: 8+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2023-03-14 12:01 [Tarantool-patches] [PATCH luajit v2 0/5] jit: add exception unwinding Maxim Kokryashkin via Tarantool-patches
2023-03-14 12:01 ` [Tarantool-patches] [PATCH luajit v2 1/5] test: disable `lj-603-snap-restore` test Maxim Kokryashkin via Tarantool-patches
2023-03-17 11:25   ` Sergey Kaplun via Tarantool-patches
2023-03-20 14:09     ` Maxim Kokryashkin via Tarantool-patches
2023-03-14 12:01 ` [Tarantool-patches] [PATCH luajit v2 2/5] Handle on-trace OOM errors from helper functions Maxim Kokryashkin via Tarantool-patches
2023-03-14 12:01 ` [Tarantool-patches] [PATCH luajit v2 3/5] Disable unreliable assertion for external frame unwinding Maxim Kokryashkin via Tarantool-patches
2023-03-14 12:01 ` [Tarantool-patches] [PATCH luajit v2 4/5] OSX: " Maxim Kokryashkin via Tarantool-patches
2023-03-14 12:01 ` Maxim Kokryashkin 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:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

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

  git send-email \
    --in-reply-to=4d5e9afef83bdbacb2b5f5ca140e941ced04b36f.1678794695.git.m.kokryashkin@tarantool.org \
    --to=tarantool-patches@dev.tarantool.org \
    --cc=max.kokryashkin@gmail.com \
    --cc=sergos@tarantool.org \
    --cc=skaplun@tarantool.org \
    --subject='Re: [Tarantool-patches] [PATCH luajit v2 5/5] Fix IR_RENAME snapshot number. Follow-up fix for a32aeadc.' \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* 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