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 1174C1355ED; Wed, 30 Nov 2022 13:40:31 +0300 (MSK) DKIM-Filter: OpenDKIM Filter v2.11.0 dev.tarantool.org 1174C1355ED DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=tarantool.org; s=dev; t=1669804831; bh=l8ltG3zmJIA3gWA9I+iOb9N6SsvGC3Y6o2wSGshuiJ0=; h=In-Reply-To:Date:Cc:References:To:Subject:List-Id: List-Unsubscribe:List-Archive:List-Post:List-Help:List-Subscribe: From:Reply-To:From; b=rgj8xlSINfqIDUycT26pTxSp9HodKUjE8RU19C/3guzrXSG81Ub8EZzkRK16NVW17 zNpjW3wOTT0CheA/Ii9vxsUYueHo8wQMp8e79sCznbxwQ6syUlEpFwOGXJwDqdGizD c3m4o80JV9Jf8CyJLabWnFCVLvEObz0lUhulDO8c= Received: from smtp17.i.mail.ru (smtp17.i.mail.ru [95.163.41.70]) (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 8C6F18BB7C for ; Wed, 30 Nov 2022 13:40:29 +0300 (MSK) DKIM-Filter: OpenDKIM Filter v2.11.0 dev.tarantool.org 8C6F18BB7C Received: by smtp17.i.mail.ru with esmtpa (envelope-from ) id 1p0KVw-00629J-R7; Wed, 30 Nov 2022 13:40:29 +0300 Content-Type: multipart/alternative; boundary="Apple-Mail=_18DD838B-B2EA-4F5C-8CBE-AFBBD8F7183A" Mime-Version: 1.0 (Mac OS X Mail 16.0 \(3731.200.110.1.12\)) X-Priority: 3 (Normal) In-Reply-To: <1664445487.810639758@f460.i.mail.ru> Date: Wed, 30 Nov 2022 13:40:04 +0300 Cc: tarantool-patches@dev.tarantool.org, Maksim Kokryashkin Message-Id: <35C34FB9-EBAF-4F9A-ABC1-877D8A04A684@tarantool.org> References: <20220920111649.86190-1-max.kokryashkin@gmail.com> <4DD728DB-B92B-4466-A709-E3FD08576876@tarantool.org> <1664445487.810639758@f460.i.mail.ru> To: Maxim Kokryashkin X-Mailer: Apple Mail (2.3731.200.110.1.12) X-Mailru-Src: smtp X-7564579A: B8F34718100C35BD X-77F55803: 4F1203BC0FB41BD9F70D9A593BA47E2A8FC9DFB589E7951358ACFB4B7681944A182A05F538085040DAA9B5EFB70AE9E5145B16E8DBB9948819DA7B687C2280F77931B57AB1A1684A X-C8649E89: 4E36BF7865823D7055A7F0CF078B5EC49A30900B95165D34BD150993ED26DDF49940233D9AF71896AD79245EE34A1C6EDC33E58541DB34DC467A27FAE35796B71D7E09C32AA3244C7A3B73758BB9E0F8C8D753C528503E00F94338140B71B8EEFACE5A9C96DEB163 X-D57D3AED: 3ZO7eAau8CL7WIMRKs4sN3D3tLDjz0dLbV79QFUyzQ2Ujvy7cMT6pYYqY16iZVKkSc3dCLJ7zSJH7+u4VD18S7Vl4ZUrpaVfd2+vE6kuoey4m4VkSEu530nj6fImhcD4MUrOEAnl0W826KZ9Q+tr5ycPtXkTV4k65bRjmOUUP8cvGozZ33TWg5HZplvhhXbhDGzqmQDTd6OAevLeAnq3Ra9uf7zvY2zzsIhlcp/Y7m53TZgf2aB4JOg4gkr2biojWg9qwXN3O9Xd4LYtlaPGSQ== X-Mailru-Sender: 11C2EC085EDE56FA38FD4C59F7EFE407690C885E585D1AF71C86E42099AECE804EAB46A6D34EF62619381EE24192DF5555834048F03EF5D4C9A814A92B2E3B1BA4250FC3964EA4964198E0F3ECE9B5443453F38A29522196 X-Mras: OK Subject: Re: [Tarantool-patches] [PATCH luajit v2] Fix narrowing of unary minus. 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: sergos via Tarantool-patches Reply-To: sergos Errors-To: tarantool-patches-bounces@dev.tarantool.org Sender: "Tarantool-patches" --Apple-Mail=_18DD838B-B2EA-4F5C-8CBE-AFBBD8F7183A Content-Transfer-Encoding: quoted-printable Content-Type: text/plain; charset=utf-8 Hi! Thanks for the fixes, LGTM now. Sergos > On 29 Sep 2022, at 12:58, Maxim Kokryashkin = wrote: >=20 > Hi, Sergos! > Thanks for the questions! > Please consider my answers amd changes below. >=20 > > LuaJIT narrowing optimization during BC_UNM recording may ignore > > information about sign of zero for integer types of IR. So far the > > resulting value on a trace is not the same as for the interpreter. >=20 > I didn=E2=80=99t get the point - how is it detected, otherwise than = tostring()? > If so - should we change the tostring() instead? > Otherwise - we need a test that exposes this difference > I=E2=80=99ve changed the tests, so it=E2=80=99s now more clear that = zero sign can affect arithmetic. > Branch is force-pushed. > Here is the diff: > =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D= =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D > --- a/test/tarantool-tests/gh-6976-narrowing-of-unary-minus.test.lua > +++ b/test/tarantool-tests/gh-6976-narrowing-of-unary-minus.test.lua > @@ -2,7 +2,7 @@ > local test =3D tap.test('gh-6976-narrowing-of-unary-minus') > test:plan(2) > =20 > -jit.opt.start('hotloop=3D1', 'hotexit=3D1') > +jit.opt.start('hotloop=3D1') > =20 > local function check(routine) > jit.off() > @@ -20,32 +20,29 @@ > return true > end > =20 > -test:ok( > - check( > - function() > - local res =3D require('table.new')(3, 0) > - for _ =3D 1, 3 do > - local zero =3D 0 > - zero =3D -zero > - table.insert(res, tostring(zero)) > - end > - return res > - end > - ), > - 'incorrect recording for zero' > -) > - > -test:ok( > - check( > - function() > - local res =3D require('table.new')(3, 0) > - for i =3D 2, 0, -1 do > - table.insert(res, tostring(-i)) > - end > - return res > - end > - ), > - 'assertion guard fail' > -) > +test:ok(check(function() > + -- We use `table.new()` here to avoid trace > + -- exits due to table rehashing. > + local res =3D require('table.new')(3, 0) > + for _ =3D 1, 3 do > + local zero =3D 0 > + zero =3D -zero > + -- There is no difference between 0 and -0 from > + -- arithmetic perspective, unless you try to divide > + -- something by them. > + -- `1 / 0 =3D inf` and `1 / -0 =3D -inf` > + table.insert(res, 1 / zero) > + end > + return res > +end), 'incorrect recording for zero') > + > +test:ok(check(function() > + -- See the comment about `table.new()` above. > + local res =3D require('table.new')(3, 0) > + for i =3D 2, 0, -1 do > + table.insert(res, 1 / -i) > + end > + return res > +end),'assertion guard fail') > =20 > os.exit(test:check() and 0 or 1) > =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D= =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D > >=20 > > This patch fixes the non-DUALNUM mode behaviour. When the zero value = is > > identified during recording it should be cast to number so IR_CONV = is > > emitted. Also, this patch adds assertion guard checking that value = on > > which operation of unary minus is performed isn't zero. >=20 > Does it mean I will exit the trace every time I met a `x =3D 0; x =3D = -x` in it? > No, that assertion guard takes you back to the interpreter only if a > trace for unary minus was recorded considering `x` as a non-zero = value, > and at some point in this trace `x` became zero. ok, it looks reasonable. > =20 > =20 > Best regards, > Maxim Kokryashkin > =20 --Apple-Mail=_18DD838B-B2EA-4F5C-8CBE-AFBBD8F7183A Content-Transfer-Encoding: quoted-printable Content-Type: text/html; charset=utf-8
Hi!

Thanks for the = fixes, LGTM = now.

Sergos


On 29 Sep 2022, at 12:58, Maxim Kokryashkin = <m.kokryashkin@tarantool.org> wrote:

Hi, Sergos!
Thanks for the = questions!
Please consider my answers amd changes = below.

> LuaJIT narrowing = optimization during BC_UNM recording may ignore
> information = about sign of zero for integer types of IR. So far the
> resulting = value on a trace is not the same as for the interpreter.

I = didn=E2=80=99t get the point - how is it detected, otherwise than = tostring()?
If so - should we change the tostring() = instead?
Otherwise - we need a test that exposes this = difference
I=E2=80=99ve changed the tests, so = it=E2=80=99s now more clear that zero sign can affect = arithmetic.
Branch is force-pushed.
Here is the = diff:
=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D= =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D= =3D=3D
--- = a/test/tarantool-tests/gh-6976-narrowing-of-unary-minus.test.lua
+++ = b/test/tarantool-tests/gh-6976-narrowing-of-unary-minus.test.lua
@@ -2,7 +2,7 @@
 local test =3D = tap.test('gh-6976-narrowing-of-unary-minus')
 test:plan(2)<= /div>
 
-jit.opt.start('hotloop=3D1', = 'hotexit=3D1')
+jit.opt.start('hotloop=3D1')
 
 local function check(routine)
  =  jit.off()
@@ -20,32 +20,29 @@
  =  return = true
 end
 
-test:ok(
- =  check(
-    function()
-   =    local res =3D require('table.new')(3, 0)
-   =    for _ =3D 1, 3 do
-       =  local zero =3D 0
-        zero =3D = -zero
-        table.insert(res, = tostring(zero))
-      end
-   =    return res
-    end
- =  ),
-  'incorrect recording for = zero'
-)
-
-test:ok(
- =  check(
-    function()
-   =    local res =3D require('table.new')(3, 0)
-   =    for i =3D 2, 0, -1 do
-       =  table.insert(res, tostring(-i))
-     =  end
-      return res
-   =  end
-  ),
-  'assertion guard = fail'
-)
+test:ok(check(function()
+ =  -- We use `table.new()` here to avoid trace
+  -- = exits due to table rehashing.
+  local res =3D = require('table.new')(3, 0)
+  for _ =3D 1, 3 = do
+    local zero =3D 0
+   =  zero =3D -zero
+    -- There is no difference = between 0 and -0 from
+    -- arithmetic = perspective, unless you try to divide
+    -- = something by them.
+    -- `1 / 0 =3D inf` and `1 / = -0 =3D -inf`
+    table.insert(res, 1 / = zero)
+  end
+  return = res
+end), 'incorrect recording for = zero')
+
+test:ok(check(function()
+ =  -- See the comment about `table.new()` above.
+ =  local res =3D require('table.new')(3, 0)
+  for i =3D= 2, 0, -1 do
+    table.insert(res, 1 / = -i)
+  end
+  return = res
+end),'assertion guard = fail')
 
 os.exit(test:check() and 0 or = 1)
=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D= =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D= =3D
<snipped>

> This patch fixes the non-DUALNUM mode behaviour. = When the zero value is
> identified during recording it should be = cast to number so IR_CONV is
> emitted. Also, this patch adds = assertion guard checking that value on
> which operation of unary = minus is performed isn't zero.

Does it mean I will exit the trace = every time I met a `x =3D 0; x =3D -x` in it?
No, = that assertion guard takes you back to the interpreter only if = a
trace for unary minus was recorded considering `x` as a = non-zero value,
and at some point in this trace `x` became = zero.

ok, it looks = reasonable.

 
 
Best = regards,
Maxim = Kokryashkin
 

= --Apple-Mail=_18DD838B-B2EA-4F5C-8CBE-AFBBD8F7183A--