From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from [87.239.111.99] (localhost [127.0.0.1]) by dev.tarantool.org (Postfix) with ESMTP id 80B0D5764B9; Wed, 9 Aug 2023 18:49:29 +0300 (MSK) DKIM-Filter: OpenDKIM Filter v2.11.0 dev.tarantool.org 80B0D5764B9 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=tarantool.org; s=dev; t=1691596169; bh=3VoHnxFyuzTxicToDtQE+ryEEtMs8UnB9gS1odiyG6M=; h=To:Date:In-Reply-To:References:Subject:List-Id:List-Unsubscribe: List-Archive:List-Post:List-Help:List-Subscribe:From:Reply-To:Cc: From; b=k2jIA3R8EB9d2EYXii2LU8uHBYsO+rSXNDlixrY07VAIhov2KvGABpcD+/4QuKrie vlMq7M7TqGcXEh5xop2Pgj40H9SVi0nHBru14YxsRE6ocMfwhMjwEbogfeOdbG9Bfi Ki5e4EczFN0gUEyUyPcfUNIY1avTXcExW9vjh0xs= Received: from smtp32.i.mail.ru (smtp32.i.mail.ru [95.163.41.73]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (2048 bits) server-digest SHA256) (No client certificate requested) by dev.tarantool.org (Postfix) with ESMTPS id 9E8AC57649E for ; Wed, 9 Aug 2023 18:41:22 +0300 (MSK) DKIM-Filter: OpenDKIM Filter v2.11.0 dev.tarantool.org 9E8AC57649E Received: by smtp32.i.mail.ru with esmtpa (envelope-from ) id 1qTlJJ-003Nbf-2v; Wed, 09 Aug 2023 18:41:22 +0300 To: Igor Munkin , Sergey Bronnikov Date: Wed, 9 Aug 2023 18:36:06 +0300 Message-ID: <37c2435a3529beb36c2e428f9c8e8b5c007c68e7.1691592488.git.skaplun@tarantool.org> X-Mailer: git-send-email 2.41.0 In-Reply-To: References: MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Mailru-Src: smtp X-4EC0790: 10 X-7564579A: 646B95376F6C166E X-77F55803: 4F1203BC0FB41BD969E04B5EED670DC8E1D56388F8ECDB9EA9DEC181E9DCB203182A05F538085040F561013564904DC26A27CB2E765B1616679B704043DD92DD24C452058460A5BD X-7FA49CB5: FF5795518A3D127A4AD6D5ED66289B5278DA827A17800CE75C0AD7D016C066E3C2099A533E45F2D0395957E7521B51C2CFCAF695D4D8E9FCEA1F7E6F0F101C6778DA827A17800CE7A1DB0B089319D380EA1F7E6F0F101C6723150C8DA25C47586E58E00D9D99D84E1BDDB23E98D2D38BE5CCB53A13BC8DBAF2053ABF30327077E720199B0D2A56ECCC7F00164DA146DAFE8445B8C89999728AA50765F79006371DD7586035AB81AE389733CBF5DBD5E9C8A9BA7A39EFB766F5D81C698A659EA7CC7F00164DA146DA9985D098DBDEAEC8C6602A96AF88C695F6B57BC7E6449061A352F6E88A58FB86F5D81C698A659EA73AA81AA40904B5D9A18204E546F3947C6089696B24BB1D1903F1AB874ED890284AD6D5ED66289B523666184CF4C3C14F6136E347CC761E07725E5C173C3A84C3F5D93BA78F69AFFFBA3038C0950A5D36B5C8C57E37DE458B330BD67F2E7D9AF16D1867E19FE14079C09775C1D3CA48CF3D321E7403792E342EB15956EA79C166A417C69337E82CC275ECD9A6C639B01B78DA827A17800CE7E1BCFB2C0BE3F189731C566533BA786AA5CC5B56E945C8DA X-C1DE0DAB: 0D63561A33F958A55BF98F274F552E732F6BEBBAB7CE8134EF8BD1B3D25FCDA4F87CCE6106E1FC07E67D4AC08A07B9B0AD74539164518AE59C5DF10A05D560A950611B66E3DA6D700B0A020F03D25A0997E3FB2386030E77 X-C8649E89: 1C3962B70DF3F0ADE00A9FD3E00BEEDF3FED46C3ACD6F73ED3581295AF09D3DF87807E0823442EA2ED31085941D9CD0AF7F820E7B07EA4CF601681869337218A229D202C0E25226F1413A0C5A8B2E09F728C201A5A2BAFE3673DC415E80A8BD91529AA6C8ABD04CA52E4189AE1FFA7A913866192BC339929A74DFFEFA5DC0E7F02C26D483E81D6BE5EF9655DD6DEA7D65774BB76CC95456EEC5B5AD62611EEC62B5AFB4261A09AF0 X-D57D3AED: 3ZO7eAau8CL7WIMRKs4sN3D3tLDjz0dLbV79QFUyzQ2Ujvy7cMT6pYYqY16iZVKkSc3dCLJ7zSJH7+u4VD18S7Vl4ZUrpaVfd2+vE6kuoey4m4VkSEu530nj6fImhcD4MUrOEAnl0W826KZ9Q+tr5ycPtXkTV4k65bRjmOUUP8cvGozZ33TWg5HZplvhhXbhDGzqmQDTd6OAevLeAnq3Ra9uf7zvY2zzsIhlcp/Y7m53TZgf2aB4JOg4gkr2biojUzxoxvtYX2o6jcHMiTkGFg== X-Mailru-Sender: 11C2EC085EDE56FAC07928AF2646A769C9B971B94C277D226A27CB2E765B1616CA71892AC981DD5FDEDBA653FF35249392D99EB8CC7091A70E183A470755BFD208F19895AA18418972D6B4FCE48DF648AE208404248635DF X-Mras: Ok Subject: [Tarantool-patches] [PATCH luajit 17/19] MIPS64: Fix register allocation in assembly of HREF. X-BeenThere: tarantool-patches@dev.tarantool.org X-Mailman-Version: 2.1.34 Precedence: list List-Id: Tarantool development patches List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , From: Sergey Kaplun via Tarantool-patches Reply-To: Sergey Kaplun Cc: tarantool-patches@dev.tarantool.org Errors-To: tarantool-patches-bounces@dev.tarantool.org Sender: "Tarantool-patches" From: Mike Pall Contributed by James Cowgill. (cherry-picked from commit 99cdfbf6a1e8856f64908072ef10443a7eab14f2) The issue is observed for the following merged IRs: | p64 HREF 0001 "a" ; or other keys | > p64 EQ 0002 [0x4002d0c528] ; nilnode Sometimes, when we need to rematerialize a constant during evicting of the register. So, the instruction related to constant rematerialization is placed in the delay branch slot, which suppose to contain the loads of trace exit number to the `$ra` register. The resulting assembly is the following (for example): | beq ra, r1, 0x400abee9b0 ->exit | lui r1, 65531 ; delay slot without setting of the `ra` This leading to the assertion failure during trace exit in `lj_trace_exit()`, since a trace number is incorrect. This patch moves the constant register allocations above the main instruction emitting code in `asm_href()`. Sergey Kaplun: * added the description and the test for the problem Part of tarantool/tarantool#8825 --- src/lj_asm_mips.h | 42 +++++--- ...-mips64-href-delay-slot-side-exit.test.lua | 101 ++++++++++++++++++ 2 files changed, 126 insertions(+), 17 deletions(-) create mode 100644 test/tarantool-tests/lj-362-mips64-href-delay-slot-side-exit.test.lua diff --git a/src/lj_asm_mips.h b/src/lj_asm_mips.h index c27d8413..23ffc3aa 100644 --- a/src/lj_asm_mips.h +++ b/src/lj_asm_mips.h @@ -859,6 +859,9 @@ static void asm_href(ASMState *as, IRIns *ir, IROp merge) Reg dest = ra_dest(as, ir, allow); Reg tab = ra_alloc1(as, ir->op1, rset_clear(allow, dest)); Reg key = RID_NONE, type = RID_NONE, tmpnum = RID_NONE, tmp1 = RID_TMP, tmp2; +#if LJ_64 + Reg cmp64 = RID_NONE; +#endif IRRef refkey = ir->op2; IRIns *irkey = IR(refkey); int isk = irref_isk(refkey); @@ -901,6 +904,26 @@ static void asm_href(ASMState *as, IRIns *ir, IROp merge) #endif tmp2 = ra_scratch(as, allow); rset_clear(allow, tmp2); +#if LJ_64 + if (LJ_SOFTFP || !irt_isnum(kt)) { + /* Allocate cmp64 register used for 64-bit comparisons */ + if (LJ_SOFTFP && irt_isnum(kt)) { + cmp64 = key; + } else if (!isk && irt_isaddr(kt)) { + cmp64 = tmp2; + } else { + int64_t k; + if (isk && irt_isaddr(kt)) { + k = ((int64_t)irt_toitype(irkey->t) << 47) | irkey[1].tv.u64; + } else { + lua_assert(irt_ispri(kt) && !irt_isnil(kt)); + k = ~((int64_t)~irt_toitype(ir->t) << 47); + } + cmp64 = ra_allock(as, k, allow); + rset_clear(allow, cmp64); + } + } +#endif /* Key not found in chain: jump to exit (if merged) or load niltv. */ l_end = emit_label(as); @@ -943,24 +966,9 @@ static void asm_href(ASMState *as, IRIns *ir, IROp merge) emit_dta(as, MIPSI_DSRA32, tmp1, tmp1, 15); emit_tg(as, MIPSI_DMTC1, tmp1, tmpnum); emit_tsi(as, MIPSI_LD, tmp1, dest, (int32_t)offsetof(Node, key.u64)); - } else if (LJ_SOFTFP && irt_isnum(kt)) { - emit_branch(as, MIPSI_BEQ, tmp1, key, l_end); - emit_tsi(as, MIPSI_LD, tmp1, dest, (int32_t)offsetof(Node, key.u64)); - } else if (irt_isaddr(kt)) { - Reg refk = tmp2; - if (isk) { - int64_t k = ((int64_t)irt_toitype(irkey->t) << 47) | irkey[1].tv.u64; - refk = ra_allock(as, k, allow); - rset_clear(allow, refk); - } - emit_branch(as, MIPSI_BEQ, tmp1, refk, l_end); - emit_tsi(as, MIPSI_LD, tmp1, dest, offsetof(Node, key)); } else { - Reg pri = ra_allock(as, ~((int64_t)~irt_toitype(ir->t) << 47), allow); - rset_clear(allow, pri); - lua_assert(irt_ispri(kt) && !irt_isnil(kt)); - emit_branch(as, MIPSI_BEQ, tmp1, pri, l_end); - emit_tsi(as, MIPSI_LD, tmp1, dest, offsetof(Node, key)); + emit_branch(as, MIPSI_BEQ, tmp1, cmp64, l_end); + emit_tsi(as, MIPSI_LD, tmp1, dest, (int32_t)offsetof(Node, key.u64)); } *l_loop = MIPSI_BNE | MIPSF_S(tmp1) | ((as->mcp-l_loop-1) & 0xffffu); if (!isk && irt_isaddr(kt)) { diff --git a/test/tarantool-tests/lj-362-mips64-href-delay-slot-side-exit.test.lua b/test/tarantool-tests/lj-362-mips64-href-delay-slot-side-exit.test.lua new file mode 100644 index 00000000..8c75e69c --- /dev/null +++ b/test/tarantool-tests/lj-362-mips64-href-delay-slot-side-exit.test.lua @@ -0,0 +1,101 @@ +local tap = require('tap') +-- Test file to demonstrate the incorrect JIT behaviour for HREF +-- IR compilation on mips64. +-- See also https://github.com/LuaJIT/LuaJIT/pull/362. +local test = tap.test('lj-362-mips64-href-delay-slot-side-exit'):skipcond({ + ['Test requires JIT enabled'] = not jit.status(), +}) + +test:plan(1) + +-- To reproduce the issue we need to compile a trace with +-- `IR_HREF`, with a lookup of constant hash key GC value. To +-- prevent an `IR_HREFK` to be emitted instead, we need a table +-- with a huge hash part. Delta of address between the start of +-- the hash part of the table and the current node to lookup must +-- be more than `(1024 * 64 - 1) * sizeof(Node)`. +-- See , for details. +-- XXX: This constant is well suited to prevent test to be flaky, +-- because the aforementioned delta is always large enough. +-- Also, this constant avoids table rehashing, when inserting new +-- keys. +local N_HASH_FIELDS = 2 ^ 16 + 2 ^ 15 + +-- XXX: don't set `hotexit` to prevent compilation of trace after +-- exiting the main test cycle. +jit.opt.start('hotloop=1') + +-- Don't use `table.new()`, here by intence -- this leads to the +-- allocation failure for the mcode memory, so traces are not +-- compiled. +local filled_tab = {} +-- Filling-up the table with GC values to minimize the amount of +-- hash collisions and increase delta between the start of the +-- hash part of the table and currently stored node. +for _ = 1, N_HASH_FIELDS do + filled_tab[1LL] = 1 +end + +-- luacheck: no unused +local tab_value_a +local tab_value_b +local tab_value_c +local tab_value_d +local tab_value_e +local tab_value_f +local tab_value_g +local tab_value_h +local tab_value_i + +-- The function for this trace has a bunch of the following IRs: +-- p64 HREF 0001 "a" ; or other keys +-- > p64 EQ 0002 [0x4002d0c528] ; nilnode +-- Sometimes, when we need to rematerialize a constant during +-- evicting of the register. So, the instruction related to +-- constant rematerialization is placed in the delay branch slot, +-- which suppose to contain the loads of trace exit number to the +-- `$ra` register. This leading to the assertion failure during +-- trace exit in `lj_trace_exit()`, since a trace number is +-- incorrect. The amount of the side exit to check is empirical +-- (even a little bit more, than necessary just in case). +local function href_const(tab) + tab_value_a = tab.a + tab_value_b = tab.b + tab_value_c = tab.c + tab_value_d = tab.d + tab_value_e = tab.e + tab_value_f = tab.f + tab_value_g = tab.g + tab_value_h = tab.h + tab_value_i = tab.i +end + +-- Compile main trace first. +href_const(filled_tab) +href_const(filled_tab) + +-- Now brute-force side exits to check that they are compiled +-- correct. Take side exits in the reverse order to take a new +-- side exit each time. +filled_tab.i = 'i' +href_const(filled_tab) +filled_tab.h = 'h' +href_const(filled_tab) +filled_tab.g = 'g' +href_const(filled_tab) +filled_tab.f = 'f' +href_const(filled_tab) +filled_tab.e = 'e' +href_const(filled_tab) +filled_tab.d = 'd' +href_const(filled_tab) +filled_tab.c = 'c' +href_const(filled_tab) +filled_tab.b = 'b' +href_const(filled_tab) +filled_tab.a = 'a' +href_const(filled_tab) + +test:ok(true, 'no assertion failures during trace exits') + +test:done(true) -- 2.41.0