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 513CB352E91; Thu, 11 May 2023 00:34:06 +0300 (MSK) DKIM-Filter: OpenDKIM Filter v2.11.0 dev.tarantool.org 513CB352E91 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=tarantool.org; s=dev; t=1683754446; bh=upUsAU20C5GI8dFTvobNotU769FgWKRVkFbzagnl6N8=; h=To:Date:Subject:List-Id:List-Unsubscribe:List-Archive:List-Post: List-Help:List-Subscribe:From:Reply-To:Cc:From; b=uQhRWNUhLkxgMbdsITQoCqwQ0pO9cgBnWqEvimdjwo0fe8WJSg7WufoVaupKBAyeP e8+IsgUAfLJZsFjxkYj2zEREc1MW0dc+sgi3xgvsmHGtpFFAd8AAth+2RKGeNfUbT9 tM8Atti7u3oKSwajarpmwXy/RRjAsix9Rser0OYk= Received: from smtpng1.i.mail.ru (smtpng1.i.mail.ru [94.100.181.251]) (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 D01AE352E91 for ; Thu, 11 May 2023 00:34:04 +0300 (MSK) DKIM-Filter: OpenDKIM Filter v2.11.0 dev.tarantool.org D01AE352E91 Received: by smtpng1.m.smailru.net with esmtpa (envelope-from ) id 1pwrRj-0001Sy-PQ; Thu, 11 May 2023 00:34:04 +0300 To: Maxim Kokryashkin , Sergey Bronnikov Date: Thu, 11 May 2023 00:30:02 +0300 Message-Id: <20230510213002.5153-1-skaplun@tarantool.org> X-Mailer: git-send-email 2.34.1 MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Mailru-Src: smtp X-7564579A: B8F34718100C35BD X-77F55803: 4F1203BC0FB41BD9EB47717F7D29529238FE22877F995669B8AECF16E68EE75F182A05F5380850405BA336422E2F5BF60C0B2DEAAD8F775EBC20CD29808E37CF1C4FA23979AE9DB0 X-7FA49CB5: FF5795518A3D127A4AD6D5ED66289B5278DA827A17800CE7544B1CCE26E01C74EA1F7E6F0F101C67BD4B6F7A4D31EC0BCC500DACC3FED6E28638F802B75D45FF8AA50765F7900637A975286280E7A0BD8638F802B75D45FF36EB9D2243A4F8B5A6FCA7DBDB1FC311F39EFFDF887939037866D6147AF826D847058D546DCB079E793ABC3BBDA0F37E117882F4460429724CE54428C33FAD305F5C1EE8F4F765FC1FA46963A5C90001A471835C12D1D9774AD6D5ED66289B52BA9C0B312567BB23117882F4460429728776938767073520C24E1E72F37C03A0F04B652EEC242312D2E47CDBA5A96583BA9C0B312567BB231DD303D21008E29813377AFFFEAFD269A417C69337E82CC2E827F84554CEF50127C277FBC8AE2E8BA83251EDC214901ED5E8D9A59859A8B65D56369A3576CBA5089D37D7C0E48F6C5571747095F342E88FB05168BE4CE3AF X-B7AD71C0: 1B70FBA5C9BEEE72C9761FC34675ADEB871C96603B655635EE9D5CB6078CC77C7565B5A834936E98EFFFE7C7C1A70394 X-C1DE0DAB: 0D63561A33F958A57093100CDD4E837DE6C5FF6748F4169EB314F45FB7F808CDF87CCE6106E1FC07E67D4AC08A07B9B06A1CB4668A9CA5FA9C5DF10A05D560A950611B66E3DA6D700B0A020F03D25A0997E3FB2386030E77 X-C8649E89: 1C3962B70DF3F0ADBF74143AD284FC71106E36FF2641B7B8DC8270968E61249B1004E42C50DC4CA955A7F0CF078B5EC49A30900B95165D347D10A9FCB2A62DFE8E2C13F60E1FF68F8705376C7764C73BF786707AB814C781418B4222D4B472901D7E09C32AA3244C2CE8D38295CF9F58760D551F37DAA4113FD9C8CA1B0515E0927AC6DF5659F194 X-D57D3AED: 3ZO7eAau8CL7WIMRKs4sN3D3tLDjz0dLbV79QFUyzQ2Ujvy7cMT6pYYqY16iZVKkSc3dCLJ7zSJH7+u4VD18S7Vl4ZUrpaVfd2+vE6kuoey4m4VkSEu530nj6fImhcD4MUrOEAnl0W826KZ9Q+tr5ycPtXkTV4k65bRjmOUUP8cvGozZ33TWg5HZplvhhXbhDGzqmQDTd6OAevLeAnq3Ra9uf7zvY2zzsIhlcp/Y7m53TZgf2aB4JOg4gkr2bioj6KE94Dszzao7ucHxvx18qA== X-DA7885C5: E7C91AE31BC40FF6997898CDDDB5E2961BD40182AE4C7279309E519CC2D1DA03262E2D401490A4A0DB037EFA58388B346E8BC1A9835FDE71 X-Mailru-Sender: 689FA8AB762F73933AF1F914F131DBF5BCEF1B2E8D6D9761B7BE4DDAD85E0AB90FBE9A32752B8C9C2AA642CC12EC09F1FB559BB5D741EB962F61BD320559CF1EFD657A8799238ED55FEEDEB644C299C0ED14614B50AE0675 X-Mras: Ok Subject: [Tarantool-patches] [PATCH luajit] Fix TNEW load forwarding with instable types. 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 (cherry picked from commit d4c0c6e17ef7edf1f2893bc807746b80612e63e9) During loop unrolling IR instructions are coped, substituted and re-emitted for the FOLD/CSE/etc. pipeline. Assume, we have the following loop-carried dependency: we create a new table or load some stored one depends on iteration number. In case when we copy the emitted ALOAD IR instruction (from from the stored table), we copy it as is, including loading type. But the ALOAD from the new table created (on the previous iteration) should have nil type, so the assertion in `fwd_ahload()` is failed. This patch replaces the assertion to `return 0` to avoid the assertion failure and stop forwarding in such situations. But the emitted type-guarded ALOAD will lead to the persistent failure of the assertion guard on the trace. Hence, another one fix should be added. Also, TDUP IR is affected, too. See also the issue mentioned in the test. Sergey Kaplun: * added the description and the test for the problem Part of tarantool/tarantool#8516 --- Branch: https://github.com/tarantool/luajit/tree/skaplun/lj-994-instable-types-during-loop-unroll PR: https://github.com/tarantool/tarantool/pull/8642 Related issues: * https://github.com/tarantool/tarantool/issues/8516 * https://github.com/LuaJIT/LuaJIT/issues/994 I don't mention the 994 intentionally to avoid Mike bothering. Also, it isn't the origin of this commit. Quite the opposite: as a result of this backporting the following issue was created. I prefer to backport the patches (this one and the prospective for 994) separately. So, after this patch is backported, it doesn't add any critical bugs (always failing guard just creates a new side trace), but helps to avoid conflicts for future backporting (sadly remembered "Improve assertions."). src/lj_opt_mem.c | 3 +- ...instable-types-during-loop-unroll.test.lua | 53 +++++++++++++++++++ 2 files changed, 55 insertions(+), 1 deletion(-) create mode 100644 test/tarantool-tests/lj-994-instable-types-during-loop-unroll.test.lua diff --git a/src/lj_opt_mem.c b/src/lj_opt_mem.c index cc177d39..c8265b4f 100644 --- a/src/lj_opt_mem.c +++ b/src/lj_opt_mem.c @@ -180,7 +180,8 @@ static TRef fwd_ahload(jit_State *J, IRRef xref) } ref = store->prev; } - lua_assert(ir->o != IR_TNEW || irt_isnil(fins->t)); + if (ir->o == IR_TNEW && !irt_isnil(fins->t)) + return 0; /* Type instability in loop-carried dependency. */ if (irt_ispri(fins->t)) { return TREF_PRI(irt_type(fins->t)); } else if (irt_isnum(fins->t) || (LJ_DUALNUM && irt_isint(fins->t)) || diff --git a/test/tarantool-tests/lj-994-instable-types-during-loop-unroll.test.lua b/test/tarantool-tests/lj-994-instable-types-during-loop-unroll.test.lua new file mode 100644 index 00000000..435f6e0e --- /dev/null +++ b/test/tarantool-tests/lj-994-instable-types-during-loop-unroll.test.lua @@ -0,0 +1,53 @@ +local tap = require('tap') + +local test = tap.test('lj-994-instable-types-during-loop-unroll'):skipcond({ + ['Test requires JIT enabled'] = not jit.status(), +}) + +-- Test file to demonstrate LuaJIT misbehaviour during loop +-- unrolling and load forwarding for newly created tables. +-- See also https://github.com/LuaJIT/LuaJIT/issues/994. + +-- TODO: test that compiled traces don't always exit by the type +-- guard. See also the comment for the TDUP test chunk. +test:plan(1) + +-- TNEW. +local result +local stored_tab = {1} +local slot = {} +local key = 1 + +jit.opt.start('hotloop=1') +-- The trouble happens during loop unrolling when we copy +-- `>+ num ALOAD` IR in the context of the table on the previous +-- iteration instead of a new one created via TNEW containing no +-- values (so type nil should be used instead of num). +for _ = 1, 5 do + local t = slot + -- Use non-constant key to avoid LJ_TRERR_GFAIL and undoing the + -- loop. + result = t[key] + -- Swap table loaded by SLOAD to the created via TNEW. + slot = _ % 2 ~= 0 and stored_tab or {} +end +test:is(result, nil, 'TNEW load forwarding was successful') + +-- TDUP. +--[[ +-- FIXME: Disable test case for now. Enable, with another one +-- backported commit with a fix for the aforementioned issue +-- (and after patch "Improve assertions."). +-- Test taken trace exits too. +for _ = 1, 5 do + local t = slot + -- Now use constant key slot to get necessary branch. + -- LJ_TRERR_GFAIL isn't triggered here. + -- See `fwd_ahload()` in for details. + result = t[1] + slot = _ % 2 ~= 0 and stored_tab or {true} +end +test:is(result, true, 'TDUP load forwarding was successful') +]] + +os.exit(test:check() and 0 or 1) -- 2.34.1