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’t 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’ve changed the tests, so it’s now more clear that zero sign can affect arithmetic.

Branch is force-pushed.

Here is the diff:

===============================================

--- 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 = tap.test('gh-6976-narrowing-of-unary-minus')

 test:plan(2)

 

-jit.opt.start('hotloop=1', 'hotexit=1')

+jit.opt.start('hotloop=1')

 

 local function check(routine)

   jit.off()

@@ -20,32 +20,29 @@

   return true

 end

 

-test:ok(

-  check(

-    function()

-      local res = require('table.new')(3, 0)

-      for _ = 1, 3 do

-        local zero = 0

-        zero = -zero

-        table.insert(res, tostring(zero))

-      end

-      return res

-    end

-  ),

-  'incorrect recording for zero'

-)

-

-test:ok(

-  check(

-    function()

-      local res = require('table.new')(3, 0)

-      for i = 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 = require('table.new')(3, 0)

+  for _ = 1, 3 do

+    local zero = 0

+    zero = -zero

+    -- There is no difference between 0 and -0 from

+    -- arithmetic perspective, unless you try to divide

+    -- something by them.

+    -- `1 / 0 = inf` and `1 / -0 = -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 = require('table.new')(3, 0)

+  for i = 2, 0, -1 do

+    table.insert(res, 1 / -i)

+  end

+  return res

+end),'assertion guard fail')

 

 os.exit(test:check() and 0 or 1)

===============================================

<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 = 0; x = -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.

 

 

Best regards,

Maxim Kokryashkin