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 476FE69EE83; Fri, 13 Oct 2023 13:11:33 +0300 (MSK) DKIM-Filter: OpenDKIM Filter v2.11.0 dev.tarantool.org 476FE69EE83 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=tarantool.org; s=dev; t=1697191893; bh=ZxGubwxARpvIGcMsdGfC2CCVy8JdtpCBL5/82kEDJDY=; 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=NnqVWkpB4QPHArALXDxuxjLyyGE7JQMHQaQupd7UrwC35N+MVjojyH5bSf5V9lAUJ SBqkxxPvGEKQWaLDr61Q9k6UzWGgnrtJI4gNpR1lODZJUAPWpdctiVvoA5DbEhaq4A TIrhXW98pNp2dbvPFWnapzzJ6YvAq2I2kSgD7NyU= Received: from smtp60.i.mail.ru (smtp60.i.mail.ru [95.163.41.98]) (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 016AC5F2B01 for ; Fri, 13 Oct 2023 13:11:32 +0300 (MSK) DKIM-Filter: OpenDKIM Filter v2.11.0 dev.tarantool.org 016AC5F2B01 Received: by smtp60.i.mail.ru with esmtpa (envelope-from ) id 1qrF8k-00DCqd-1i; Fri, 13 Oct 2023 13:11:30 +0300 Date: Fri, 13 Oct 2023 13:11:30 +0300 To: Sergey Kaplun Message-ID: References: <20231003133705.5700-1-max.kokryashkin@gmail.com> MIME-Version: 1.0 Content-Type: text/plain; charset=iso-8859-1 Content-Disposition: inline Content-Transfer-Encoding: 8bit In-Reply-To: X-Mailru-Src: smtp X-7564579A: EEAE043A70213CC8 X-77F55803: 4F1203BC0FB41BD91A160B2A88227A7277AF5FF924596F5D01639E1FD6F960421313CFAB8367EF908E2BE116634AD74D9F100D044D2A60EB5B1C28CE3F3611956DC744274B5CC956E0BD2A1C2FE4B953 X-C1DE0DAB: 0D63561A33F958A5E54F9BE4E47FF770EF2B2EAABA875655768D43959CACA240F87CCE6106E1FC07E67D4AC08A07B9B0DB8A315C1FF4794DBDAD6C7F3747799A X-C8649E89: 1C3962B70DF3F0ADE00A9FD3E00BEEDF77DD89D51EBB7742DC8270968E61249B1004E42C50DC4CA955A7F0CF078B5EC49A30900B95165D34A9873270276D4334003672021C54BE6F5ABBBD85FCAF125D85C43CD384905B91B246C4F6CF7E0A051D7E09C32AA3244C0377E16809DBD3794DCE07327BF6C4B239C99C45E8D137E9BAD658CF5C8AB4025DA084F8E80FEBD396F07DFE06A4A8314E894E437E78228B66933FA05BD8EF0CAD958392AE682691 X-D57D3AED: 3ZO7eAau8CL7WIMRKs4sN3D3tLDjz0dLbV79QFUyzQ2Ujvy7cMT6pYYqY16iZVKkSc3dCLJ7zSJH7+u4VD18S7Vl4ZUrpaVfd2+vE6kuoey4m4VkSEu530nj6fImhcD4MUrOEAnl0W826KZ9Q+tr5ycPtXkTV4k65bRjmOUUP8cvGozZ33TWg5HZplvhhXbhDGzqmQDTd6OAevLeAnq3Ra9uf7zvY2zzsIhlcp/Y7m53TZgf2aB4JOg4gkr2biojBYtV3BjG6I4EG8RXtLpxkA== X-Mailru-Sender: 11C2EC085EDE56FA38FD4C59F7EFE4077A07C2D53C515A7C868201199E5B365876748896E63B1A09D51284F0FE6F529ABC7555A253F5B200DF104D74F62EE79D27EC13EC74F6107F4198E0F3ECE9B5443453F38A29522196 X-Mras: OK Subject: Re: [Tarantool-patches] [PATCH luajit v2] Mark CONV as non-weak, to prevent elimination of its side-effect. 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: Maxim Kokryashkin via Tarantool-patches Reply-To: Maxim Kokryashkin Cc: Maksim Kokryashkin , tarantool-patches@dev.tarantool.org Errors-To: tarantool-patches-bounces@dev.tarantool.org Sender: "Tarantool-patches" Hi, Sergey! Thanks for the review! See my replies below. On Mon, Oct 09, 2023 at 12:27:35PM +0300, Sergey Kaplun wrote: > Hi, Maxim! > Thanks for the patch! > LGTM, just a minor nit below. > > On 06.10.23, Maxim Kokryashkin wrote: > > > > > > > +-- Before the patch, the `0022 > int CONV 0017 int.num` > > > I see that IR "0022 > int CONV ..." is present in both IR traces... > > Yep, they are omitted due to DCE and it happens on the trace assembly > > stage. Dropped a comment. > > > > +-- instruction is omitted due to DCE, which results in the > > > > +-- third side exit being taken, instead of the second, > > > > +-- and, hence, incorrect summation. After the patch, `CONV` > > > > +-- is left intact and is not omitted; it remains as a guarded > > > > +-- instruction, so the second side exit is taken and sum is > > > > +-- performed correctly. > > > > + > > > > +for _, val in ipairs(data) do > > > > + if val == val then > > > > + sum = sum + val > > > > + end > > > > +end > > Here is the diff with changes. Branch is force-pushed: > > === > > diff --git a/test/tarantool-tests/mark-conv-non-weak.test.lua b/test/tarantool-tests/mark-conv-non-weak.test.lua > > index f54f30ba..b71be4da 100644 > > --- a/test/tarantool-tests/mark-conv-non-weak.test.lua > > +++ b/test/tarantool-tests/mark-conv-non-weak.test.lua > > @@ -4,11 +4,13 @@ local test = tap.test('mark-conv-non-weak'):skipcond({ > > }) > > > > test:plan(1) > > +-- XXX: These values were chosen to create type instability > > +-- in the loop-carried dependency, so the checked `CONV int.num` > > +-- instruction is emitted. See `loop_unrool` in `lj_opt_loop.c`. > > +local data = {0, 0.1, 0, 0 / 0} > > +local sum = 0.1 > > Do we really need to start the sum from 0.1, why just not 0? Yep, we really do. Dropped a comment for that, branch is force-pushed. Here is the diff: === diff --git a/test/tarantool-tests/mark-conv-non-weak.test.lua b/test/tarantool-tests/mark-conv-non-weak.test.lua index b71be4da..331ca25e 100644 --- a/test/tarantool-tests/mark-conv-non-weak.test.lua +++ b/test/tarantool-tests/mark-conv-non-weak.test.lua @@ -8,6 +8,11 @@ test:plan(1) -- in the loop-carried dependency, so the checked `CONV int.num` -- instruction is emitted. See `loop_unrool` in `lj_opt_loop.c`. local data = {0, 0.1, 0, 0 / 0} + +--- XXX: The sum is required to be initialized with a non-zero +-- floating point value; otherwise, `0023  + num ADD    0017  0007` +-- instruction in the IR below becomes `ADDOV` and the `CONV int.num` +-- conversion is used by it. local sum = 0.1 jit.opt.start('hotloop=1') ===