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 55524D69355; Mon, 17 Jun 2024 11:44:33 +0300 (MSK) DKIM-Filter: OpenDKIM Filter v2.11.0 dev.tarantool.org 55524D69355 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=tarantool.org; s=dev; t=1718613873; bh=kw2QKL+SDsyp3t8BkNeqwZ3tZgA9OhO8qQI8r/Gc5c0=; 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=nN9r1D1jQ7m6boj29HJQdLfrn7vPHkErxJSqkLQgOztWph7gRJos2Z8khH16DT4Sd pNhfwLiGfrrcR9+CfhBw4z+R0syqr4yCdPgfKS8FBvVYcnK1bygqP8T93wgj77rWmC /YEynYPS2/rdNiz+W8P2RP/RvWk7NCI8xM2U0oLY= Received: from smtp30.i.mail.ru (smtp30.i.mail.ru [95.163.41.71]) (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 318ED464584 for ; Mon, 17 Jun 2024 11:44:31 +0300 (MSK) DKIM-Filter: OpenDKIM Filter v2.11.0 dev.tarantool.org 318ED464584 Received: by exim-smtp-dbbd7b44b-7mms2 with esmtpa (envelope-from ) id 1sJ7yX-000000001HD-3fHP; Mon, 17 Jun 2024 11:44:30 +0300 Content-Type: multipart/alternative; boundary="------------Pbs4AtMdUNcg9y8pB1YFTxxg" Message-ID: <3556c257-14f0-416d-b8bb-22ad760fc51e@tarantool.org> Date: Mon, 17 Jun 2024 11:44:29 +0300 MIME-Version: 1.0 User-Agent: Mozilla Thunderbird Content-Language: en-US To: Sergey Kaplun References: <20240517132918.25926-1-skaplun@tarantool.org> In-Reply-To: X-Mailru-Src: smtp X-4EC0790: 10 X-7564579A: 646B95376F6C166E X-77F55803: 4F1203BC0FB41BD9AC8CA0B4439200FA3396D26EA105C0471DE01117BA8131F700894C459B0CD1B9B501EFC1B056BCEC9487ABAC94A94B5454A31BA7E96DA7C5E411E8B5AC5011D8D6C07CA76FD93A9B X-7FA49CB5: FF5795518A3D127A4AD6D5ED66289B5278DA827A17800CE72847AA60176ABEF3EA1F7E6F0F101C67BD4B6F7A4D31EC0BCC500DACC3FED6E28638F802B75D45FF8AA50765F7900637B84F9009663064BD8638F802B75D45FF36EB9D2243A4F8B5A6FCA7DBDB1FC311F39EFFDF887939037866D6147AF826D8934844DB488E93C513DC3FAC65C7C1148ED7BF2F47B09F43CC7F00164DA146DAFE8445B8C89999728AA50765F7900637F3E38EE449E3E2AE389733CBF5DBD5E9C8A9BA7A39EFB766F5D81C698A659EA7CC7F00164DA146DA9985D098DBDEAEC8A9FF340AA05FB58CF6B57BC7E6449061A352F6E88A58FB86F5D81C698A659EA73AA81AA40904B5D9A18204E546F3947C2D01283D1ACF37BA03F1AB874ED890284AD6D5ED66289B523666184CF4C3C14F6136E347CC761E07725E5C173C3A84C3C4D4DFE5459F50CDBA3038C0950A5D36B5C8C57E37DE458B330BD67F2E7D9AF16D1867E19FE14079C09775C1D3CA48CF17B107DEF921CE791DD303D21008E298D5E8D9A59859A8B64854413538E1713F75ECD9A6C639B01B78DA827A17800CE758ECEFFC28DC0EE1731C566533BA786AA5CC5B56E945C8DA X-C1DE0DAB: 0D63561A33F958A5E571C94CBFF698485002B1117B3ED696F8FBB558FA8C236222DFD5397F446790823CB91A9FED034534781492E4B8EEAD220496FFA5CD4785BDAD6C7F3747799A X-C8649E89: 1C3962B70DF3F0ADBF74143AD284FC7177DD89D51EBB7742424CF958EAFF5D571004E42C50DC4CA955A7F0CF078B5EC49A30900B95165D34DFB7A809FB53708746AAC16579D113B6CE9F1061584AF57DE007C26B5E601595B8A648887B10C6BA1D7E09C32AA3244C7303A945187365C877DD89D51EBB7742EB79F72DF6E2FF3AEA455F16B58544A2557BDE0DD54B3590A5AE236DF995FB59978A700BF655EAEEED6A17656DB59BCAD427812AF56FC65B X-D57D3AED: 3ZO7eAau8CL7WIMRKs4sN3D3tLDjz0dLbV79QFUyzQ2Ujvy7cMT6pYYqY16iZVKkSc3dCLJ7zSJH7+u4VD18S7Vl4ZUrpaVfd2+vE6kuoey4m4VkSEu530nj6fImhcD4MUrOEAnl0W826KZ9Q+tr5ycPtXkTV4k65bRjmOUUP8cvGozZ33TWg5HZplvhhXbhDGzqmQDTd6OAevLeAnq3Ra9uf7zvY2zzsIhlcp/Y7m53TZgf2aB4JOg4gkr2biojSQgXCM0oo7X8W9xnAAMWNQ== X-Mailru-Sender: 520A125C2F17F0B1E52FEF5D219D6140B501EFC1B056BCEC9487ABAC94A94B54D5B93C6456960BC80152A3D17938EB451EB5A0BCEC6A560B3DDE9B364B0DF289BE2DA36745F2EEB5CEBA01FB949A1F1EEAB4BC95F72C04283CDA0F3B3F5B9367 X-Mras: Ok Subject: Re: [Tarantool-patches] [PATCH luajit] Maintain chain invariant in DCE. 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 Cc: tarantool-patches@dev.tarantool.org Errors-To: tarantool-patches-bounces@dev.tarantool.org Sender: "Tarantool-patches" This is a multi-part message in MIME format. --------------Pbs4AtMdUNcg9y8pB1YFTxxg Content-Type: text/plain; charset=UTF-8; format=flowed Content-Transfer-Encoding: 7bit Hi, Sergey thanks for update, LGTM. On 16.06.2024 22:35, Sergey Kaplun wrote: > Hi, Sergey! > Thanks for the review! > I've added comments and force-pushed the branch. > > On 14.06.24, Sergey Bronnikov wrote: >> Hi, Sergey >> >> thanks for the patch! see my comment below >> >> On 17.05.2024 16:29, Sergey Kaplun wrote: >>> From: Mike Pall >>> >>> Thanks to Peter Cawley. >>> >>> (cherry picked from commit f72c19e482b6f918b7cf42b0436e2b117d160a29) >>> >>> Instructions with strong guards that are sometimes emitted with a guard >>> and sometimes emitted without a guard (like HREFK, CONV, or SLOAD) may >>> be eliminated from the IR chain and replaced with the NOP IR. If the >>> next IR of the same kind on the trace is not eliminated, it may >>> reference the IR NOP instead of an instruction of the same type. This >>> may lead to the corresponding assertion failure in the `rec_check_ir()`. >>> >>> This patch unconditionally links the IRs during chain maintenance in >>> DCE. >>> >>> Sergey Kaplun: >>> * added the description and the test for the problem >>> >>> Part of tarantool/tarantool#9924 >>> --- >>> > > >>> +local counter = 0 >>> +-- luacheck: no unused >>> +local tab = {} >>> +while true do >>> + -- The loop is still not recorded because the guard always >>> + -- fails. >>> + -- So, just try to compile it and check that there is no >>> + -- assertion failure. >>> + if counter > 2 then break end >>> + counter = counter + 1 >>> + -- The pattern `-#{}` allows us to get CONV IRs. The first >>> + -- appearance of this IR (in the `(-#{}).key`) is considered >>> + -- unused by the compiler due to the corresponding "noop" >>> + -- `__newindex` function. >>> + -- The second usage of conversion (`tab[-#{}]`) is guarded (to >>> + -- the int type), so it can't be eliminated. >>> + -- As a result, the 0048 CONV references the 0039 NOP IR after >>> + -- DCE in the IR chain. >> I suppose an IR output would be helpful here. What do you think? > I've added the corresponding output: > > | -- The IR itself looks like the following > | -- by the `jit.dump` (NOPs are not printed): > | -- 0036 num CONV 0035 num.int > | -- 0037 num NEG 0036 0007 > | -- 0042 > tab TDUP {0x40154030} > | -- 0043 int FLOAD 0014 tab.hmask > | -- 0044 > int EQ 0043 +1 > | -- 0045 p32 FLOAD 0014 tab.node > | -- 0046 > p32 HREFK 0045 "__newindex" @0 > | -- 0047 > fun HLOAD 0046 > | -- 0048 > fun EQ 0047 "lj-1094-ir-chain-dce.test.lua":20 > | -- 0049 > int CONV 0037 int.num index > > For some reason I reproduced this bug only for GC64 mode, so I've > adjusted a IRs numbers and added the corresponding comment. > > >>> + -- XXX: TDUP prevents the corresponding second usage from being >>> + -- eliminated since the table insert semantics may change. >>> + -- XXX: Use some numbers to simplify reading the `jit.dump` >>> + -- output. >>> + tab, tab[-#{}], (-#{}).key = {tdup = 'tdup'}, 1, 2 >>> +end >>> + >>> +test:ok(true, 'no assertion failure') >>> + >>> +test:done(true) --------------Pbs4AtMdUNcg9y8pB1YFTxxg Content-Type: text/html; charset=UTF-8 Content-Transfer-Encoding: 7bit

Hi, Sergey


thanks for update, LGTM.

On 16.06.2024 22:35, Sergey Kaplun wrote:
Hi, Sergey!
Thanks for the review!
I've added comments and force-pushed the branch.

On 14.06.24, Sergey Bronnikov wrote:

Hi, Sergey

thanks for the patch! see my comment below

On 17.05.2024 16:29, Sergey Kaplun wrote:

From: Mike Pall <mike>

Thanks to Peter Cawley.

(cherry picked from commit f72c19e482b6f918b7cf42b0436e2b117d160a29)

Instructions with strong guards that are sometimes emitted with a guard
and sometimes emitted without a guard (like HREFK, CONV, or SLOAD) may
be eliminated from the IR chain and replaced with the NOP IR. If the
next IR of the same kind on the trace is not eliminated, it may
reference the IR NOP instead of an instruction of the same type. This
may lead to the corresponding assertion failure in the `rec_check_ir()`.

This patch unconditionally links the IRs during chain maintenance in
DCE.

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

Part of tarantool/tarantool#9924
---


<snipped>


+local counter = 0
+-- luacheck: no unused
+local tab = {}
+while true do
+  -- The loop is still not recorded because the guard always
+  -- fails.
+  -- So, just try to compile it and check that there is no
+  -- assertion failure.
+  if counter > 2 then break end
+  counter = counter + 1
+  -- The pattern `-#{}` allows us to get CONV IRs. The first
+  -- appearance of this IR (in the `(-#{}).key`) is considered
+  -- unused by the compiler due to the corresponding "noop"
+  -- `__newindex` function.
+  -- The second usage of conversion (`tab[-#{}]`) is guarded (to
+  -- the int type), so it can't be eliminated.
+  -- As a result, the 0048 CONV references the 0039 NOP IR after
+  -- DCE in the IR chain.

I suppose an IR output would be helpful here. What do you think?

I've added the corresponding output:

| -- The IR itself looks like the following
| -- by the `jit.dump` (NOPs are not printed):
| -- 0036    num CONV   0035  num.int
| -- 0037    num NEG    0036  0007
| -- 0042 >  tab TDUP   {0x40154030}
| -- 0043    int FLOAD  0014  tab.hmask
| -- 0044 >  int EQ     0043  +1
| -- 0045    p32 FLOAD  0014  tab.node
| -- 0046 >  p32 HREFK  0045  "__newindex" @0
| -- 0047 >  fun HLOAD  0046
| -- 0048 >  fun EQ     0047  "lj-1094-ir-chain-dce.test.lua":20
| -- 0049 >  int CONV   0037  int.num index

For some reason I reproduced this bug only for GC64 mode, so I've
adjusted a IRs numbers and added the corresponding comment.



+  -- XXX: TDUP prevents the corresponding second usage from being
+  -- eliminated since the table insert semantics may change.
+  -- XXX: Use some numbers to simplify reading the `jit.dump`
+  -- output.
+  tab, tab[-#{}], (-#{}).key = {tdup = 'tdup'}, 1, 2
+end
+
+test:ok(true, 'no assertion failure')
+
+test:done(true)


    

  
  


--------------Pbs4AtMdUNcg9y8pB1YFTxxg--