Tarantool development patches archive
 help / color / mirror / Atom feed
From: Sergey Kaplun via Tarantool-patches <tarantool-patches@dev.tarantool.org>
To: Igor Munkin <imun@tarantool.org>,
	Sergey Bronnikov <sergeyb@tarantool.org>
Cc: tarantool-patches@dev.tarantool.org
Subject: [Tarantool-patches] [PATCH luajit 03/19] MIPS: Fix handling of spare long-range jump slots.
Date: Wed,  9 Aug 2023 18:35:52 +0300	[thread overview]
Message-ID: <7179245cf38c56a88bb8f3aa1bbeaf15402fcd1a.1691592488.git.skaplun@tarantool.org> (raw)
In-Reply-To: <cover.1691592488.git.skaplun@tarantool.org>

From: Mike Pall <mike>

Contributed by Djordje Kovacevic and Stefan Pejic.

(cherry-picked from commit c7c3c4da432ddb543d4b0a9abbb245f11b26afd0)

`asm_setup_jump()` in <src/lj_asm_mips.h> presumes that `sizeof(MCLink)`
is 8 bytes, but for MIPS64 its size is 16 bytes. This leads to incorrect
check in `asm_sparejump_setup()`, so mcode bottom is not updated.

This patch fixes check of the MCLink offset from the mcbot.
Nevertheless, the emitting of spare jump slots is still incorrect, so
the introduced test still fails due to incorrect iteration through the
sparce table (the last slot is out of mcode range).

This should be fixed via backporting of the commit
dbb78630169a8106b355a5be8af627e98c362f1e ("MIPS: Fix handling of
long-range spare jumps."). But it triggers the new unconditional
assert, that is added in this patch, mentioning that sizemcode is too
bit. So some workaround should be found, when this test will be enabled
for MIPS.

Since test also validates the behaviour of long-range jumps to side
traces for arm64 and x64, and we have no testing for MIPS64 (yet), we
can leave it as is without a skipcond.

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

Part of tarantool/tarantool#8825
---
 src/lj_asm_mips.h                             |  9 +--
 src/lj_jit.h                                  |  6 ++
 src/lj_mcode.c                                |  6 --
 ...x-mips64-spare-side-exit-patching.test.lua | 65 +++++++++++++++++++
 4 files changed, 76 insertions(+), 10 deletions(-)
 create mode 100644 test/tarantool-tests/fix-mips64-spare-side-exit-patching.test.lua

diff --git a/src/lj_asm_mips.h b/src/lj_asm_mips.h
index 03215821..0e60fc07 100644
--- a/src/lj_asm_mips.h
+++ b/src/lj_asm_mips.h
@@ -65,10 +65,9 @@ static Reg ra_alloc2(ASMState *as, IRIns *ir, RegSet allow)
 static void asm_sparejump_setup(ASMState *as)
 {
   MCode *mxp = as->mcbot;
-  /* Assumes sizeof(MCLink) == 8. */
-  if (((uintptr_t)mxp & (LJ_PAGESIZE-1)) == 8) {
+  if (((uintptr_t)mxp & (LJ_PAGESIZE-1)) == sizeof(MCLink)) {
     lua_assert(MIPSI_NOP == 0);
-    memset(mxp+2, 0, MIPS_SPAREJUMP*8);
+    memset(mxp, 0, MIPS_SPAREJUMP*2*sizeof(MCode));
     mxp += MIPS_SPAREJUMP*2;
     lua_assert(mxp < as->mctop);
     lj_mcode_sync(as->mcbot, mxp);
@@ -2486,7 +2485,9 @@ void lj_asm_patchexit(jit_State *J, GCtrace *T, ExitNo exitno, MCode *target)
 	  if (!cstart) cstart = p-1;
 	} else {  /* Branch out of range. Use spare jump slot in mcarea. */
 	  int i;
-	  for (i = 2; i < 2+MIPS_SPAREJUMP*2; i += 2) {
+	  for (i = (int)(sizeof(MCLink)/sizeof(MCode));
+	       i < (int)(sizeof(MCLink)/sizeof(MCode)+MIPS_SPAREJUMP*2);
+	       i += 2) {
 	    if (mcarea[i] == tjump) {
 	      delta = mcarea+i - p;
 	      goto patchbranch;
diff --git a/src/lj_jit.h b/src/lj_jit.h
index f2ad3c6e..cc8efd20 100644
--- a/src/lj_jit.h
+++ b/src/lj_jit.h
@@ -158,6 +158,12 @@ typedef uint8_t MCode;
 typedef uint32_t MCode;
 #endif
 
+/* Linked list of MCode areas. */
+typedef struct MCLink {
+  MCode *next;		/* Next area. */
+  size_t size;		/* Size of current area. */
+} MCLink;
+
 /* Stack snapshot header. */
 typedef struct SnapShot {
   uint32_t mapofs;	/* Offset into snapshot map. */
diff --git a/src/lj_mcode.c b/src/lj_mcode.c
index 7184d3b4..c6361018 100644
--- a/src/lj_mcode.c
+++ b/src/lj_mcode.c
@@ -272,12 +272,6 @@ static void *mcode_alloc(jit_State *J, size_t sz)
 
 /* -- MCode area management ----------------------------------------------- */
 
-/* Linked list of MCode areas. */
-typedef struct MCLink {
-  MCode *next;		/* Next area. */
-  size_t size;		/* Size of current area. */
-} MCLink;
-
 /* Allocate a new MCode area. */
 static void mcode_allocarea(jit_State *J)
 {
diff --git a/test/tarantool-tests/fix-mips64-spare-side-exit-patching.test.lua b/test/tarantool-tests/fix-mips64-spare-side-exit-patching.test.lua
new file mode 100644
index 00000000..fdc826cb
--- /dev/null
+++ b/test/tarantool-tests/fix-mips64-spare-side-exit-patching.test.lua
@@ -0,0 +1,65 @@
+local tap = require('tap')
+local test = tap.test('fix-mips64-spare-side-exit-patching'):skipcond({
+  ['Test requires JIT enabled'] = not jit.status(),
+  ['Disabled on *BSD due to #4819'] = jit.os == 'BSD',
+  -- Need to fix the MIPS behaviour first.
+  ['Disabled for MIPS architectures'] = jit.arch:match('mips'),
+})
+
+local generators = require('utils').jit.generators
+local frontend = require('utils').frontend
+
+test:plan(1)
+
+-- Make compiler work hard.
+jit.opt.start(
+  -- No optimizations at all to produce more mcode.
+  0,
+  -- Try to compile all compiled paths as early as JIT can.
+  'hotloop=1',
+  'hotexit=1',
+  -- Allow to use 2000 traces to avoid flushes.
+  'maxtrace=2000',
+  -- Allow to compile 8Mb of mcode to be sure the issue occurs.
+  'maxmcode=8192',
+  -- Use big mcode area for traces to avoid using different
+  -- spare slots.
+  'sizemcode=256'
+)
+
+local MAX_SPARE_SLOT = 4
+local function parent(marker)
+  -- Use several side exit to fill spare exit space (default is
+  -- 4 slots, each slot has 2 instructions -- jump and nop).
+  -- luacheck: ignore
+  if marker > MAX_SPARE_SLOT then end
+  if marker > 3 then end
+  if marker > 2 then end
+  if marker > 1 then end
+  if marker > 0 then end
+  -- XXX: use `fmod()` to avoid leaving the function and use
+  -- stitching here.
+  return math.fmod(1, 1)
+end
+
+-- Compile parent trace first.
+parent(0)
+parent(0)
+
+local parent_traceno = frontend.gettraceno(parent)
+local last_traceno = parent_traceno
+
+-- Now generate some mcode to forcify long jump with a spare slot.
+-- Each iteration provide different addresses and uses a different
+-- spare slot. After it compile and execute new side trace.
+for i = 1, MAX_SPARE_SLOT + 1 do
+  generators.fillmcode(last_traceno, 1024 * 1024)
+  parent(i)
+  parent(i)
+  parent(i)
+  last_traceno = misc.getmetrics().jit_trace_num
+end
+
+test:ok(true, 'all traces executed correctly')
+
+test:done(true)
-- 
2.41.0


  parent reply	other threads:[~2023-08-09 15:42 UTC|newest]

Thread overview: 97+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2023-08-09 15:35 [Tarantool-patches] [PATCH luajit 00/19] Prerequisites for improve assertions Sergey Kaplun via Tarantool-patches
2023-08-09 15:35 ` [Tarantool-patches] [PATCH luajit 01/19] MIPS: Use precise search for exit jump patching Sergey Kaplun via Tarantool-patches
2023-08-15  9:36   ` Maxim Kokryashkin via Tarantool-patches
2023-08-16 12:40     ` Sergey Kaplun via Tarantool-patches
2023-08-16 13:25   ` Sergey Bronnikov via Tarantool-patches
2023-08-09 15:35 ` [Tarantool-patches] [PATCH luajit 02/19] test: introduce mcode generator for tests Sergey Kaplun via Tarantool-patches
2023-08-15 10:14   ` Maxim Kokryashkin via Tarantool-patches
2023-08-16 12:55     ` Sergey Kaplun via Tarantool-patches
2023-08-16 13:06       ` Maxim Kokryashkin via Tarantool-patches
2023-08-16 14:32   ` Sergey Bronnikov via Tarantool-patches
2023-08-16 15:20     ` Sergey Kaplun via Tarantool-patches
2023-08-16 16:08       ` Sergey Bronnikov via Tarantool-patches
2023-08-09 15:35 ` Sergey Kaplun via Tarantool-patches [this message]
2023-08-15 11:13   ` [Tarantool-patches] [PATCH luajit 03/19] MIPS: Fix handling of spare long-range jump slots Maxim Kokryashkin via Tarantool-patches
2023-08-16 13:05     ` Sergey Kaplun via Tarantool-patches
2023-08-16 15:02   ` Sergey Bronnikov via Tarantool-patches
2023-08-16 15:32     ` Sergey Kaplun via Tarantool-patches
2023-08-16 16:08       ` Sergey Bronnikov via Tarantool-patches
2023-08-09 15:35 ` [Tarantool-patches] [PATCH luajit 04/19] MIPS64: Add soft-float support to JIT compiler backend Sergey Kaplun via Tarantool-patches
2023-08-15 11:27   ` Maxim Kokryashkin via Tarantool-patches
2023-08-16 13:10     ` Sergey Kaplun via Tarantool-patches
2023-08-16 16:07   ` Sergey Bronnikov via Tarantool-patches
2023-08-09 15:35 ` [Tarantool-patches] [PATCH luajit 05/19] PPC: Add soft-float support to interpreter Sergey Kaplun via Tarantool-patches
2023-08-15 11:40   ` Maxim Kokryashkin via Tarantool-patches
2023-08-16 13:13     ` Sergey Kaplun via Tarantool-patches
2023-08-17 14:53   ` Sergey Bronnikov via Tarantool-patches
2023-08-09 15:35 ` [Tarantool-patches] [PATCH luajit 06/19] PPC: Add soft-float support to JIT compiler backend Sergey Kaplun via Tarantool-patches
2023-08-15 11:46   ` Maxim Kokryashkin via Tarantool-patches
2023-08-16 13:21     ` Sergey Kaplun via Tarantool-patches
2023-08-17 14:33   ` Sergey Bronnikov via Tarantool-patches
2023-08-09 15:35 ` [Tarantool-patches] [PATCH luajit 07/19] build: fix non-Linux/macOS builds Sergey Kaplun via Tarantool-patches
2023-08-15 11:58   ` Maxim Kokryashkin via Tarantool-patches
2023-08-16 13:40     ` Sergey Kaplun via Tarantool-patches
2023-08-17 14:31   ` Sergey Bronnikov via Tarantool-patches
2023-08-09 15:35 ` [Tarantool-patches] [PATCH luajit 08/19] Windows: Add UWP support, part 1 Sergey Kaplun via Tarantool-patches
2023-08-15 12:09   ` Maxim Kokryashkin via Tarantool-patches
2023-08-16 13:50     ` Sergey Kaplun via Tarantool-patches
2023-08-16 16:40   ` Sergey Bronnikov via Tarantool-patches
2023-08-09 15:35 ` [Tarantool-patches] [PATCH luajit 09/19] FFI: Eliminate hardcoded string hashes Sergey Kaplun via Tarantool-patches
2023-08-15 13:07   ` Maxim Kokryashkin via Tarantool-patches
2023-08-16 13:52     ` Sergey Kaplun via Tarantool-patches
2023-08-16 17:04     ` Sergey Bronnikov via Tarantool-patches
2023-08-09 15:35 ` [Tarantool-patches] [PATCH luajit 10/19] Cleanup math function compilation and fix inconsistencies Sergey Kaplun via Tarantool-patches
2023-08-11  8:06   ` Sergey Kaplun via Tarantool-patches
2023-08-15 13:10   ` Maxim Kokryashkin via Tarantool-patches
2023-08-16 17:15   ` Sergey Bronnikov via Tarantool-patches
2023-08-09 15:36 ` [Tarantool-patches] [PATCH luajit 11/19] Fix GCC 7 -Wimplicit-fallthrough warnings Sergey Kaplun via Tarantool-patches
2023-08-15 13:17   ` Maxim Kokryashkin via Tarantool-patches
2023-08-16 13:59     ` Sergey Kaplun via Tarantool-patches
2023-08-17  7:37   ` Sergey Bronnikov via Tarantool-patches
2023-08-09 15:36 ` [Tarantool-patches] [PATCH luajit 12/19] DynASM: Fix warning Sergey Kaplun via Tarantool-patches
2023-08-15 13:21   ` Maxim Kokryashkin via Tarantool-patches
2023-08-16 14:01     ` Sergey Kaplun via Tarantool-patches
2023-08-17  7:39   ` Sergey Bronnikov via Tarantool-patches
2023-08-17  7:51     ` Sergey Bronnikov via Tarantool-patches
2023-08-17  7:58       ` Sergey Kaplun via Tarantool-patches
2023-08-09 15:36 ` [Tarantool-patches] [PATCH luajit 13/19] ARM: Fix GCC 7 -Wimplicit-fallthrough warnings Sergey Kaplun via Tarantool-patches
2023-08-15 13:25   ` Maxim Kokryashkin via Tarantool-patches
2023-08-16 14:08     ` Sergey Kaplun via Tarantool-patches
2023-08-17  7:44   ` Sergey Bronnikov via Tarantool-patches
2023-08-17  8:01     ` Sergey Kaplun via Tarantool-patches
2023-08-09 15:36 ` [Tarantool-patches] [PATCH luajit 14/19] Fix debug.getinfo() argument check Sergey Kaplun via Tarantool-patches
2023-08-15 13:35   ` Maxim Kokryashkin via Tarantool-patches
2023-08-16 14:20     ` Sergey Kaplun via Tarantool-patches
2023-08-16 20:13       ` Maxim Kokryashkin via Tarantool-patches
2023-08-17  8:29   ` Sergey Bronnikov via Tarantool-patches
2023-08-09 15:36 ` [Tarantool-patches] [PATCH luajit 15/19] Fix LJ_MAX_JSLOTS assertion in rec_check_slots() Sergey Kaplun via Tarantool-patches
2023-08-15 14:07   ` Maxim Kokryashkin via Tarantool-patches
2023-08-16 14:22     ` Sergey Kaplun via Tarantool-patches
2023-08-17  8:57   ` Sergey Bronnikov via Tarantool-patches
2023-08-17  8:57     ` Sergey Kaplun via Tarantool-patches
2023-08-09 15:36 ` [Tarantool-patches] [PATCH luajit 16/19] Prevent integer overflow while parsing long strings Sergey Kaplun via Tarantool-patches
2023-08-15 14:38   ` Maxim Kokryashkin via Tarantool-patches
2023-08-16 14:52     ` Sergey Kaplun via Tarantool-patches
2023-08-17 10:53   ` Sergey Bronnikov via Tarantool-patches
2023-08-17 13:57     ` Sergey Kaplun via Tarantool-patches
2023-08-17 14:28       ` Sergey Bronnikov via Tarantool-patches
2023-08-09 15:36 ` [Tarantool-patches] [PATCH luajit 17/19] MIPS64: Fix register allocation in assembly of HREF Sergey Kaplun via Tarantool-patches
2023-08-16  9:01   ` Maxim Kokryashkin via Tarantool-patches
2023-08-16 15:17     ` Sergey Kaplun via Tarantool-patches
2023-08-16 20:14       ` Maxim Kokryashkin via Tarantool-patches
2023-08-17 11:06   ` Sergey Bronnikov via Tarantool-patches
2023-08-17 13:50     ` Sergey Kaplun via Tarantool-patches
2023-08-17 14:30       ` Sergey Bronnikov via Tarantool-patches
2023-08-09 15:36 ` [Tarantool-patches] [PATCH luajit 18/19] DynASM/MIPS: Fix shadowed variable Sergey Kaplun via Tarantool-patches
2023-08-16  9:03   ` Maxim Kokryashkin via Tarantool-patches
2023-08-16 15:22     ` Sergey Kaplun via Tarantool-patches
2023-08-17 12:01   ` Sergey Bronnikov via Tarantool-patches
2023-08-09 15:36 ` [Tarantool-patches] [PATCH luajit 19/19] MIPS: Add MIPS64 R6 port Sergey Kaplun via Tarantool-patches
2023-08-16  9:16   ` Maxim Kokryashkin via Tarantool-patches
2023-08-16 15:24     ` Sergey Kaplun via Tarantool-patches
2023-08-17 13:03   ` Sergey Bronnikov via Tarantool-patches
2023-08-17 13:59     ` Sergey Kaplun via Tarantool-patches
2023-08-16 15:35 ` [Tarantool-patches] [PATCH luajit 00/19] Prerequisites for improve assertions Sergey Kaplun via Tarantool-patches
2023-08-17 14:06   ` Maxim Kokryashkin via Tarantool-patches
2023-08-17 14:38 ` Sergey Bronnikov via Tarantool-patches
2023-08-31 15:17 ` 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:
  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=7179245cf38c56a88bb8f3aa1bbeaf15402fcd1a.1691592488.git.skaplun@tarantool.org \
    --to=tarantool-patches@dev.tarantool.org \
    --cc=imun@tarantool.org \
    --cc=sergeyb@tarantool.org \
    --cc=skaplun@tarantool.org \
    --subject='Re: [Tarantool-patches] [PATCH luajit 03/19] MIPS: Fix handling of spare long-range jump slots.' \
    /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