<html><head><meta http-equiv="content-type" content="text/html; charset=utf-8"></head><body style="overflow-wrap: break-word; -webkit-nbsp-mode: space; line-break: after-white-space;"><meta http-equiv="content-type" content="text/html; charset=utf-8"><div style="overflow-wrap: break-word; -webkit-nbsp-mode: space; line-break: after-white-space;">Hi!<div><br></div><div>Thanks for the fixes, LGTM now.</div><div><br></div><div>Sergos</div><div><br><div><div><br><blockquote type="cite"><div>On 29 Sep 2022, at 12:58, Maxim Kokryashkin <m.kokryashkin@tarantool.org> wrote:</div><br class="Apple-interchange-newline"><div>
<div><div>Hi, Sergos!</div><div>Thanks for the questions!</div><div>Please consider my answers amd changes below.</div><blockquote style="border-left:1px solid #0857A6; margin:10px; padding:0 0 0 10px;"><div><br>> LuaJIT narrowing optimization during BC_UNM recording may ignore<br>> information about sign of zero for integer types of IR. So far the<br>> resulting value on a trace is not the same as for the interpreter.<br><br>I didn’t get the point - how is it detected, otherwise than tostring()?<br>If so - should we change the tostring() instead?<br>Otherwise - we need a test that exposes this difference</div></blockquote><div>I’ve changed the tests, so it’s now more clear that zero sign can affect arithmetic.</div><div>Branch is force-pushed.</div><div>Here is the diff:</div><div>===============================================</div><div><div><div>--- a/test/tarantool-tests/gh-6976-narrowing-of-unary-minus.test.lua</div><div>+++ b/test/tarantool-tests/gh-6976-narrowing-of-unary-minus.test.lua</div><div>@@ -2,7 +2,7 @@</div><div> local test = tap.test('gh-6976-narrowing-of-unary-minus')</div><div> test:plan(2)</div><div> </div><div>-jit.opt.start('hotloop=1', 'hotexit=1')</div><div>+jit.opt.start('hotloop=1')</div><div> </div><div> local function check(routine)</div><div> jit.off()</div><div>@@ -20,32 +20,29 @@</div><div> return true</div><div> end</div><div> </div><div>-test:ok(</div><div>- check(</div><div>- function()</div><div>- local res = require('table.new')(3, 0)</div><div>- for _ = 1, 3 do</div><div>- local zero = 0</div><div>- zero = -zero</div><div>- table.insert(res, tostring(zero))</div><div>- end</div><div>- return res</div><div>- end</div><div>- ),</div><div>- 'incorrect recording for zero'</div><div>-)</div><div>-</div><div>-test:ok(</div><div>- check(</div><div>- function()</div><div>- local res = require('table.new')(3, 0)</div><div>- for i = 2, 0, -1 do</div><div>- table.insert(res, tostring(-i))</div><div>- end</div><div>- return res</div><div>- end</div><div>- ),</div><div>- 'assertion guard fail'</div><div>-)</div><div>+test:ok(check(function()</div><div>+ -- We use `table.new()` here to avoid trace</div><div>+ -- exits due to table rehashing.</div><div>+ local res = require('table.new')(3, 0)</div><div>+ for _ = 1, 3 do</div><div>+ local zero = 0</div><div>+ zero = -zero</div><div>+ -- There is no difference between 0 and -0 from</div><div>+ -- arithmetic perspective, unless you try to divide</div><div>+ -- something by them.</div><div>+ -- `1 / 0 = inf` and `1 / -0 = -inf`</div><div>+ table.insert(res, 1 / zero)</div><div>+ end</div><div>+ return res</div><div>+end), 'incorrect recording for zero')</div><div>+</div><div>+test:ok(check(function()</div><div>+ -- See the comment about `table.new()` above.</div><div>+ local res = require('table.new')(3, 0)</div><div>+ for i = 2, 0, -1 do</div><div>+ table.insert(res, 1 / -i)</div><div>+ end</div><div>+ return res</div><div>+end),'assertion guard fail')</div><div> </div><div> os.exit(test:check() and 0 or 1)</div><div>===============================================</div><div><snipped></div></div></div><blockquote style="border-left:1px solid #0857A6; margin:10px; padding:0 0 0 10px;"><div><br>> This patch fixes the non-DUALNUM mode behaviour. When the zero value is<br>> identified during recording it should be cast to number so IR_CONV is<br>> emitted. Also, this patch adds assertion guard checking that value on<br>> which operation of unary minus is performed isn't zero.<br><br>Does it mean I will exit the trace every time I met a `x = 0; x = -x` in it?</div></blockquote><div>No, that assertion guard takes you back to the interpreter only if a</div><div>trace for unary minus was recorded considering `x` as a non-zero value,</div><div>and at some point in this trace `x` became zero.</div></div></div></blockquote><div><br></div>ok, it looks reasonable.</div><div><br><blockquote type="cite"><div><div><div> </div><div> </div><div>Best regards,</div><div><div>Maxim Kokryashkin</div></div><div> </div></div>
</div></blockquote></div><br></div></div></div></body></html>