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--