Re: [Tarantool-patches] [PATCH luajit 1/2] Prevent recording of loops with -0 step or NaN values.

[Sergey Kaplun via Tarantool-patches <tarantool-patches@dev.tarantool.org>]

Sergey,

Updated the comment as we discussed offline.
Branch is force-pushed.

On 13.03.26, Sergey Kaplun via Tarantool-patches wrote:
> Hi, Sergey!
> See my answer below.
> 
> On 13.03.26, Sergey Bronnikov wrote:
> > Hi, Sergey,
> > 
> > thanks for the fixes!
> > 
> > Sergey
> > 
> > On 3/13/26 13:07, Sergey Kaplun wrote:
> > 
> > 
> > <snipped>
> > 
> > >>> +local function test_trace_recorded(test_payload)
> > >>> +  jit.flush()
> > >>> +  -- Reset hotcounters.
> > >> nit: comment can be omitted
> > > I prefer not to. There may be the question: why we don't declare this
> > > parameters once? The reason is that the hotcounters may cause collisions
> > > and lead to the false-positive tests failures. Should I make the comment
> > > more verbose?
> > 
> > We reset hotcounters in tests about 470 times (grep -R -B 1 "hotloop=1" 
> > test | wc -l) and only
> > 
> > 15 times we add a comment like "Reset hotcounters.". You add a comment 
> > here but missed it in the patch
> > 
> > "MIPS64: Avoid unaligned load in lj_vm_exit_interp.". Why we should 
> > leave comment here and
> > 
> > omit it the aforementioned patch? I'll not insist removing it, just 
> > interesting, it is not an issue for blocking merge.
> 
> The main idea is to prevent the hotcount collisions between any other
> functions that may possibly get hot. Unaligned load isn't a problem then
> since we have no check for JIT semantics (no calls to `jit.util.traceinfo()`).
> The same approach is vital for all checks that assume the specific trace
> recording (or abortion). Hence, this comment is added in the first
> place to attract the attention of the reader to these "standard lines",
> which are not standard at all (since it is not done in the main chunk only
> once).
> 
> Than the question is: should I make the comment more verbose and
> specific?

Made the comment more verbose:

===================================================================
diff --git a/test/tarantool-tests/lj-1432-minus-zero-step.test.lua b/test/tarantool-tests/lj-1432-minus-zero-step.test.lua
index 112153dc..e3b97dde 100644
--- a/test/tarantool-tests/lj-1432-minus-zero-step.test.lua
+++ b/test/tarantool-tests/lj-1432-minus-zero-step.test.lua
@@ -42,7 +42,7 @@ end
 
 local function test_trace_recorded(test_payload)
   jit.flush()
-  -- Reset hotcounters.
+  -- XXX: Reset hotcounters to avoid false-positive collisions.
   jit.opt.start('hotloop=1', 'hotexit=1')
   test_payload()
   return traceinfo(1)
diff --git a/test/tarantool-tests/lj-1433-nan-for-loop-control-var.test.lua b/test/tarantool-tests/lj-1433-nan-for-loop-control-var.test.lua
index b9e5ad92..f7566d6a 100644
--- a/test/tarantool-tests/lj-1433-nan-for-loop-control-var.test.lua
+++ b/test/tarantool-tests/lj-1433-nan-for-loop-control-var.test.lua
@@ -62,7 +62,7 @@ end
 
 local function test_trace_recorded(test_payload)
   jit.flush()
-  -- Reset hotcounters.
+  -- XXX: Reset hotcounters to avoid false-positive collisions.
   jit.opt.start('hotloop=1', 'hotexit=1')
   test_payload()
   return traceinfo(1)
===================================================================

> 
> > 
> > >
> > <snipped>
> 
> -- 
> Best regards,
> Sergey Kaplun

-- 
Best regards,
Sergey Kaplun