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 2E106287E12; Thu, 26 Jan 2023 10:09:48 +0300 (MSK) DKIM-Filter: OpenDKIM Filter v2.11.0 dev.tarantool.org 2E106287E12 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=tarantool.org; s=dev; t=1674716988; bh=nYPTox46CIKQNrIgEiPO6zUzgqFwpLA1HPzkaJY63to=; h=Date:To:References:In-Reply-To:Subject:List-Id:List-Unsubscribe: List-Archive:List-Post:List-Help:List-Subscribe:From:Reply-To:Cc: From; b=poYfHhVGNQHhMM04s0+8my6u8R+cIBbux9UfMCfoBKKWmdHJiVVVsVr+PgleXahUV mF4lgXWXL//uyVvgOO8TRFxMlkDQ5WVCIwe8eo1C/zNquIz5lJW46Qk7mKnwr2ZPrK fAhMqkRiZsNpFiIztwhVzRbvu9f4WWCLo8eu4Rh0= Received: from smtp33.i.mail.ru (smtp33.i.mail.ru [95.163.41.74]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by dev.tarantool.org (Postfix) with ESMTPS id 9A46670370 for ; Thu, 26 Jan 2023 10:09:46 +0300 (MSK) DKIM-Filter: OpenDKIM Filter v2.11.0 dev.tarantool.org 9A46670370 Received: by smtp33.i.mail.ru with esmtpa (envelope-from ) id 1pKwOH-0042ko-Nz; Thu, 26 Jan 2023 10:09:46 +0300 Date: Thu, 26 Jan 2023 10:06:20 +0300 To: Maxim Kokryashkin Message-ID: References: <673614b0d270047c096b156f5a886211770c4ac1.1674068996.git.skaplun@tarantool.org> <1674573205.475122003@f164.i.mail.ru> MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Disposition: inline Content-Transfer-Encoding: 8bit In-Reply-To: <1674573205.475122003@f164.i.mail.ru> X-Mailru-Src: smtp X-4EC0790: 10 X-7564579A: 646B95376F6C166E X-77F55803: 4F1203BC0FB41BD939F4CB9F411D0C04A7D1C053D74D162CA31642CA885570F9182A05F53808504070A1E0E13E90CD5A7F1E8FE963A265CA102FE04801FCF9EF62B5F94DA003AC84 X-7FA49CB5: FF5795518A3D127A4AD6D5ED66289B5278DA827A17800CE76D34FAA3D8B31588C2099A533E45F2D0395957E7521B51C2CFCAF695D4D8E9FCEA1F7E6F0F101C6778DA827A17800CE7A8883CB087864CAC8F08D7030A58E5AD1A62830130A00468AEEEE3FBA3A834EE7353EFBB553375668694185D6E7180A2C36E77BFD3FCFC72442613FF14CA257DA471835C12D1D9774AD6D5ED66289B5278DA827A17800CE77A825AB47F0FC8649FA2833FD35BB23D2EF20D2F80756B5F868A13BD56FB6657A471835C12D1D977725E5C173C3A84C35B606B10FC07407C117882F4460429728AD0CFFFB425014E868A13BD56FB6657D81D268191BDAD3DC09775C1D3CA48CF3F23D99D50CED82F76E601842F6C81A12EF20D2F80756B5F7E9C4E3C761E06A776E601842F6C81A127C277FBC8AE2E8BF5CE31487DA1EA653AA81AA40904B5D99C9F4D5AE37F343AD1F44FA8B9022EA23BBE47FD9DD3FB595F5C1EE8F4F765FC72CEEB2601E22B093A03B725D353964B0B7D0EA88DDEDAC722CA9DD8327EE4930A3850AC1BE2E7356C9A9530EBF72002C4224003CC83647689D4C264860C145E X-C8649E89: 4E36BF7865823D7055A7F0CF078B5EC49A30900B95165D340A58BF0C7EAC340F53D3D88A97319F10EAAC17BFC8ADDA7F4BB1065A714377D404A87207F9CF98501D7E09C32AA3244C23AC12E1A2F9818973E6C54ED4137BDD8580396430872480FACE5A9C96DEB163 X-D57D3AED: 3ZO7eAau8CL7WIMRKs4sN3D3tLDjz0dLbV79QFUyzQ2Ujvy7cMT6pYYqY16iZVKkSc3dCLJ7zSJH7+u4VD18S7Vl4ZUrpaVfd2+vE6kuoey4m4VkSEu530nj6fImhcD4MUrOEAnl0W826KZ9Q+tr5ycPtXkTV4k65bRjmOUUP8cvGozZ33TWg5HZplvhhXbhDGzqmQDTd6OAevLeAnq3Ra9uf7zvY2zzsIhlcp/Y7m53TZgf2aB4JOg4gkr2biojSgrCHE6ee1jR6YqjJFyIEw== X-Mailru-Sender: F16D9CAFEEA6770E7B6EAD4ADB3BCAF04C231AD4D0E0C665367052BB0C649A177392DF524794EFEEF2400F607609286E924004A7DEC283833C7120B22964430C52B393F8C72A41A84198E0F3ECE9B5443453F38A29522196 X-Mras: Ok Subject: Re: [Tarantool-patches] [PATCH luajit 3/3] x86/x64: Fix loop realignment. 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" Hi, Maxim! Thanks for the review! On 24.01.23, Maxim Kokryashkin wrote: > > Hi, Sergey! > Thanks for the patch! > Please consider my comments below. >   > >  > >>From: Mike Pall > >> > >>(cherry picked from commit 522d2073da4be2af79db4728cbb375db0fbdfc48) > >> > >>`asm_intarith()` function may try to drop `test r, r` instruction before > >Please note that "r" is an allocated register for the instruction. > >>the Jcc instruction. However, in case when Jcc instruction is "Jump > >Typo: s/in case when/in cases where/ Fixed. > >>short if ..." instruction (i.e. has no 0F opcode prefix like "Jump near > >>if ..."), the `test` instruction is dropped when shouldn't be, due to > >Typo: s/when/when it/ > >>memory miss. As the result, the loop can't be realigned later in > >Typo: s/memory/a memory/ > >Also, that part about the memory miss is unclear, it would be better if you > >could clarify it a bit. > >>`asm_loop_fixup` due to target to jump isn't aligned and the assertion > >Typo: s/isn’t aligned/being misaligned/ Fixed. > >>fails. > >> > >>This patch adds the additional check for 0F opcode in `asm_intarith()`. > >Typo: s/for 0F/for the 0F/ Fixed, thanks! > >> > >>Sergey Kaplun: > >>* added the description and the test for the problem > >> > >>Part of tarantool/tarantool#8069 The new commit message is the following: | x86/x64: Fix loop realignment. | | (cherry picked from commit 522d2073da4be2af79db4728cbb375db0fbdfc48) | | `asm_intarith()` function may try to drop `test r, r` (where `r` is an | allocated register) instruction before the Jcc instruction. However, in | cases when Jcc instruction is "Jump short if ..." instruction (i.e. has | no 0F opcode prefix like "Jump near if ..."), the `test` instruction is | dropped when it shouldn't be, due to usage for the comparison the next | byte after instruction itself. As the result, the loop can't be | realigned later in `asm_loop_fixup` due to target to jump being | misaligned and the assertion fails. | | This patch adds the additional check for the 0F opcode in | `asm_intarith()`. | | Sergey Kaplun: | * added the description and the test for the problem | | Part of tarantool/tarantool#8069 Branch is force pushed. > >>--- > >> src/lj_asm_x86.h | 5 +++-- > >> .../lj-556-fix-loop-realignment.test.lua | 18 ++++++++++++++++++ > >> 2 files changed, 21 insertions(+), 2 deletions(-) > >> create mode 100644 test/tarantool-tests/lj-556-fix-loop-realignment.test.lua > >> > >>diff --git a/src/lj_asm_x86.h b/src/lj_asm_x86.h > >>index 8efda8e5..e6c42c6d 100644 > >>--- a/src/lj_asm_x86.h > >>+++ b/src/lj_asm_x86.h > >>@@ -2068,8 +2068,9 @@ static void asm_intarith(ASMState *as, IRIns *ir, x86Arith xa) > >>   int32_t k = 0; > >>   if (as->flagmcp == as->mcp) { /* Drop test r,r instruction. */ > >>     MCode *p = as->mcp + ((LJ_64 && *as->mcp < XI_TESTb) ? 3 : 2); > >>- if ((p[1] & 15) < 14) { > >>- if ((p[1] & 15) >= 12) p[1] -= 4; /* L <->S, NL <-> NS */ > >>+ MCode *q = p[0] == 0x0f ? p+1 : p; > >>+ if ((*q & 15) < 14) { > >>+ if ((*q & 15) >= 12) *q -= 4; /* L <->S, NL <-> NS */ > >>       as->flagmcp = NULL; > >>       as->mcp = p; > >>     } /* else: cannot transform LE/NLE to cc without use of OF. */ > >>diff --git a/test/tarantool-tests/lj-556-fix-loop-realignment.test.lua b/test/tarantool-tests/lj-556-fix-loop-realignment.test.lua > >>new file mode 100644 > >>index 00000000..9a8e6098 > >>--- /dev/null > >>+++ b/test/tarantool-tests/lj-556-fix-loop-realignment.test.lua > >>@@ -0,0 +1,18 @@ > >>+local tap = require('tap') > >>+ > >>+local test = tap.test('lj-505-fold-icorrect-behavior') > >>+test:plan(1) > >>+ > >>+-- Test file to demonstrate JIT misbehaviour for loop realignment > >>+-- in LUAJIT_NUMMODE=2. See also > >>+-- https://github.com/LuaJIT/LuaJIT/issues/556 . > >>+ > >>+jit.opt.start('hotloop=1') > >>+ > >>+local s = 4 > >>+while s > 0 do > >>+ s = s - 1 > >>+end > >>+ > >>+test:ok(true, 'loop is compiled and ran successfully') > >>+os.exit(test:check() and 0 or 1) > >>-- > >The test works just fine with HEAD on  > >f7d61d96  ci: introduce workflow for exotic builds. > >  > >Tested configurations:  > >LJ_64: True, LJ_GC64: True, LJ_DUALNUM: True > >LJ_64: True, LJ_GC64: False, LJ_DUALNUM: True It's strange... I use the following build command: | $ cmake . -DCMAKE_BUILD_TYPE=Debug -DLUA_USE_APICHECK=ON -DLUA_USE_ASSERT=ON -DLUAJIT_ENABLE_GC64=OFF -DLUAJIT_NUMMODE=2 && make -j and get the following assertion: | asm_loop_fixup: Assertion `((intptr_t)target & 15) == 0' failed. What command do you use to build LuaJIT? > >-- > >Best regards, > >Maxim Kokryashkin > >  -- Best regards, Sergey Kaplun