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 6BE66E42CE4; Tue, 14 Jan 2025 17:10:04 +0300 (MSK) DKIM-Filter: OpenDKIM Filter v2.11.0 dev.tarantool.org 6BE66E42CE4 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=tarantool.org; s=dev; t=1736863804; bh=L1Q4yZUq+4l9NUyx5XHesAwjeX7gvIpSrp1sAZqi79c=; h=Date:To:Cc:References:In-Reply-To:Subject:List-Id: List-Unsubscribe:List-Archive:List-Post:List-Help:List-Subscribe: From:Reply-To:From; b=IsJHoXJX2P4RlTMJNG3V9nYLOy/0RfFk6/3yIedxY9xIQtAHCvgYKsJect2yZ7hI/ EV994Ydnu4SV+1m8tz7pDPYNiajUKC3bhaU3ZSny0LZmzPRXh7+ljmEiiiLQ7mAM9f jvCHmgr4T6oABLBMUNiYywvjstNThOf+66CM9TmM= Received: from send194.i.mail.ru (send194.i.mail.ru [95.163.59.33]) (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 BFE594A0A19 for ; Tue, 14 Jan 2025 17:10:03 +0300 (MSK) DKIM-Filter: OpenDKIM Filter v2.11.0 dev.tarantool.org BFE594A0A19 Received: by exim-smtp-6758d5575c-628dv with esmtpa (envelope-from ) id 1tXhcI-00000000PcP-3piI; Tue, 14 Jan 2025 17:10:03 +0300 Content-Type: multipart/alternative; boundary="------------HjjnRQKw53lpieJlliKaowLw" Message-ID: <281bb3fb-904f-4903-8604-849595e795ab@tarantool.org> Date: Tue, 14 Jan 2025 17:10:02 +0300 MIME-Version: 1.0 User-Agent: Mozilla Thunderbird Content-Language: en-US To: Sergey Kaplun Cc: tarantool-patches@dev.tarantool.org References: <9ac2f3e63dc202d948d8b7f38ac3540682fc398b.1736509260.git.skaplun@tarantool.org> In-Reply-To: <9ac2f3e63dc202d948d8b7f38ac3540682fc398b.1736509260.git.skaplun@tarantool.org> X-Mailru-Src: smtp X-4EC0790: 10 X-7564579A: 646B95376F6C166E X-77F55803: 4F1203BC0FB41BD97BF177E4F5EB85B8E1230AFA6136DD93CCECC82C094B2A1C00894C459B0CD1B9FFC554CDB2E8421206104296DCCA83D230965EB18BE23A987DA77B8BADC6E7D9160237749B4E196D X-7FA49CB5: FF5795518A3D127A4AD6D5ED66289B5278DA827A17800CE7FCFCB92DA8654BB0EA1F7E6F0F101C67BD4B6F7A4D31EC0BCC500DACC3FED6E28638F802B75D45FF8AA50765F79006378F6D32451C4A3CAA8638F802B75D45FF36EB9D2243A4F8B5A6FCA7DBDB1FC311F39EFFDF887939037866D6147AF826D84B55214DF01176A4B6A883A0018345B973953FDC723617CACC7F00164DA146DAFE8445B8C89999728AA50765F7900637028599BB38096F4F389733CBF5DBD5E9C8A9BA7A39EFB766F5D81C698A659EA7CC7F00164DA146DA9985D098DBDEAEC8989FD0BDF65E50FBF6B57BC7E6449061A352F6E88A58FB86F5D81C698A659EA73AA81AA40904B5D9A18204E546F3947C4E7D9683544204AF6E0066C2D8992A164AD6D5ED66289B523666184CF4C3C14F6136E347CC761E07725E5C173C3A84C3B74263D4D5690889BA3038C0950A5D36B5C8C57E37DE458B330BD67F2E7D9AF16D1867E19FE14079C09775C1D3CA48CF3D321E7403792E342EB15956EA79C166A417C69337E82CC275ECD9A6C639B01B78DA827A17800CE7CD707F342D9BDC98731C566533BA786AA5CC5B56E945C8DA X-C1DE0DAB: 0D63561A33F958A532BA7C8F5302053A5002B1117B3ED69646CACF8505A31BFDA13BD6A4B0E00B96823CB91A9FED034534781492E4B8EEADA91A6E18C88C5E2F X-C8649E89: 1C3962B70DF3F0ADE00A9FD3E00BEEDF3FED46C3ACD6F73ED3581295AF09D3DF87807E0823442EA2ED31085941D9CD0AF7F820E7B07EA4CF848829F56C7472CF589288CAFBB79F2719695685A8607C29C10FE892E734D91F760962882471415B69A713A3E5A0090D1D7BC4D659908B2284C6B78237173532638F76363170E076111DC66A97D0BFE2913E6812662D5F2AB9AF64DB4688768036DF5FE9C0001AF333F2C28C22F508233FCF178C6DD14203 X-D57D3AED: 3ZO7eAau8CL7WIMRKs4sN3D3tLDjz0dLbV79QFUyzQ2Ujvy7cMT6pYYqY16iZVKkSc3dCLJ7zSJH7+u4VD18S7Vl4ZUrpaVfd2+vE6kuoey4m4VkSEu530nj6fImhcD4MUrOEAnl0W826KZ9Q+tr5ycPtXkTV4k65bRjmOUUP8cvGozZ33TWg5HZplvhhXbhDGzqmQDTd6OAevLeAnq3Ra9uf7zvY2zzsIhlcp/Y7m53TZgf2aB4JOg4gkr2bioj7YQzuqfQVvy1A2wjSmCjjA== X-Mailru-Sender: 520A125C2F17F0B1E52FEF5D219D6140FFC554CDB2E8421206104296DCCA83D28CEA859820A70F080152A3D17938EB451EB5A0BCEC6A560B3DDE9B364B0DF289BE2DA36745F2EEB5CEBA01FB949A1F1EEAB4BC95F72C04283CDA0F3B3F5B9367 X-Mras: Ok Subject: Re: [Tarantool-patches] [PATCH luajit 1/4] x86/x64: Don't fuse loads across table.clear. 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 Bronnikov via Tarantool-patches Reply-To: Sergey Bronnikov Errors-To: tarantool-patches-bounces@dev.tarantool.org Sender: "Tarantool-patches" This is a multi-part message in MIME format. --------------HjjnRQKw53lpieJlliKaowLw Content-Type: text/plain; charset=UTF-8; format=flowed Content-Transfer-Encoding: 7bit Hi, Sergey! thanks for the patch! LGTM On 10.01.2025 16:07, Sergey Kaplun wrote: > From: Mike Pall > > Reported by Peter Cawley. > > (cherry picked from commit 45c88b7963de2969a9a656c03ba06ad995d7fd5f) > > Load fusing optimization takes into account only the presence of the > corresponding stores, but not any calls that may affect the table > content. This may lead to the incorrect stores if the fusing > optimization occurs across the `table.clear()` call, leading to > inconsistent behaviour between the JIT and the VM. > > This patch adds the corresponding check. > > Sergey Kaplun: > * added the description and the test for the problem > > Part of tarantool/tarantool#10709 > --- > src/lj_asm_x86.h | 1 + > .../lj-1117-fuse-across-table-clear.test.lua | 36 +++++++++++++++++++ > 2 files changed, 37 insertions(+) > create mode 100644 test/tarantool-tests/lj-1117-fuse-across-table-clear.test.lua > > diff --git a/src/lj_asm_x86.h b/src/lj_asm_x86.h > index 86ce3937..f47c460a 100644 > --- a/src/lj_asm_x86.h > +++ b/src/lj_asm_x86.h > @@ -465,6 +465,7 @@ static Reg asm_fuseload(ASMState *as, IRRef ref, RegSet allow) > } > } else if (ir->o == IR_ALOAD || ir->o == IR_HLOAD || ir->o == IR_ULOAD) { > if (noconflict(as, ref, ir->o + IRDELTA_L2S, 0) && > + noconflict(as, ref, IR_CALLS, 0) && /* Don't cross table.clear. */ > !(LJ_GC64 && irt_isaddr(ir->t))) { > asm_fuseahuref(as, ir->op1, xallow); > return RID_MRM; > diff --git a/test/tarantool-tests/lj-1117-fuse-across-table-clear.test.lua b/test/tarantool-tests/lj-1117-fuse-across-table-clear.test.lua > new file mode 100644 > index 00000000..2f7c91d1 > --- /dev/null > +++ b/test/tarantool-tests/lj-1117-fuse-across-table-clear.test.lua > @@ -0,0 +1,36 @@ > +local tap = require('tap') > +-- Test file to demonstrate LuaJIT's incorrect fusion across > +-- `table.clear()`. > +-- See also:https://github.com/LuaJIT/LuaJIT/issues/1117. > +local test = tap.test('lj-1117-fuse-across-table-clear'):skipcond({ > + ['Test requires JIT enabled'] = not jit.status(), > +}) > + > +local ffi = require('ffi') > +local table_clear = require('table.clear') > + > +test:plan(1) > + > +local tab = {0} > +local alias_tab = tab > +local result_tab = {} > + > +jit.opt.start('hotloop=1') > + > +for i = 1, 4 do > + -- Load to be fused. > + local value = tab[1] > + -- Clear the alias table to trick the code flow analysis. > + table_clear(alias_tab) > + -- Need this cast to trigger load fusion. See `asm_comp()` for > + -- the details. Before the patch, this fusion takes the > + -- incorrect address of the already cleared table, which leads > + -- to the failure of the check below. > + result_tab[i] = ffi.cast('int64_t', value) > + -- Revive the value. > + tab[1] = 0 > +end > + > +test:samevalues(result_tab, 'no fusion across table.clear') > + > +test:done(true) --------------HjjnRQKw53lpieJlliKaowLw Content-Type: text/html; charset=UTF-8 Content-Transfer-Encoding: 7bit

Hi, Sergey!

thanks for the patch! LGTM

On 10.01.2025 16:07, Sergey Kaplun wrote:
From: Mike Pall <mike>

Reported by Peter Cawley.

(cherry picked from commit 45c88b7963de2969a9a656c03ba06ad995d7fd5f)

Load fusing optimization takes into account only the presence of the
corresponding stores, but not any calls that may affect the table
content. This may lead to the incorrect stores if the fusing
optimization occurs across the `table.clear()` call, leading to
inconsistent behaviour between the JIT and the VM.

This patch adds the corresponding check.

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

Part of tarantool/tarantool#10709
---
 src/lj_asm_x86.h                              |  1 +
 .../lj-1117-fuse-across-table-clear.test.lua  | 36 +++++++++++++++++++
 2 files changed, 37 insertions(+)
 create mode 100644 test/tarantool-tests/lj-1117-fuse-across-table-clear.test.lua

diff --git a/src/lj_asm_x86.h b/src/lj_asm_x86.h
index 86ce3937..f47c460a 100644
--- a/src/lj_asm_x86.h
+++ b/src/lj_asm_x86.h
@@ -465,6 +465,7 @@ static Reg asm_fuseload(ASMState *as, IRRef ref, RegSet allow)
       }
     } else if (ir->o == IR_ALOAD || ir->o == IR_HLOAD || ir->o == IR_ULOAD) {
       if (noconflict(as, ref, ir->o + IRDELTA_L2S, 0) &&
+	  noconflict(as, ref, IR_CALLS, 0) &&  /* Don't cross table.clear. */
 	  !(LJ_GC64 && irt_isaddr(ir->t))) {
 	asm_fuseahuref(as, ir->op1, xallow);
 	return RID_MRM;
diff --git a/test/tarantool-tests/lj-1117-fuse-across-table-clear.test.lua b/test/tarantool-tests/lj-1117-fuse-across-table-clear.test.lua
new file mode 100644
index 00000000..2f7c91d1
--- /dev/null
+++ b/test/tarantool-tests/lj-1117-fuse-across-table-clear.test.lua
@@ -0,0 +1,36 @@
+local tap = require('tap')
+-- Test file to demonstrate LuaJIT's incorrect fusion across
+-- `table.clear()`.
+-- See also: https://github.com/LuaJIT/LuaJIT/issues/1117.
+local test = tap.test('lj-1117-fuse-across-table-clear'):skipcond({
+  ['Test requires JIT enabled'] = not jit.status(),
+})
+
+local ffi = require('ffi')
+local table_clear = require('table.clear')
+
+test:plan(1)
+
+local tab = {0}
+local alias_tab = tab
+local result_tab = {}
+
+jit.opt.start('hotloop=1')
+
+for i = 1, 4 do
+  -- Load to be fused.
+  local value = tab[1]
+  -- Clear the alias table to trick the code flow analysis.
+  table_clear(alias_tab)
+  -- Need this cast to trigger load fusion. See `asm_comp()` for
+  -- the details. Before the patch, this fusion takes the
+  -- incorrect address of the already cleared table, which leads
+  -- to the failure of the check below.
+  result_tab[i] = ffi.cast('int64_t', value)
+  -- Revive the value.
+  tab[1] = 0
+end
+
+test:samevalues(result_tab, 'no fusion across table.clear')
+
+test:done(true)
--------------HjjnRQKw53lpieJlliKaowLw--