DUALNUM: IRSLOAD_TYPECHECK elision in rec_for_loop needs to be slightly more conservative

[corsix]

In DUALNUM, the behaviour of BC_FORI is subtly different between the interpreter and the JIT: if the loop control variables all fit as integers but stop+step overflows, then the interpreter will choose to have all the control variables as integers but the JIT will choose to have all the control variables as numbers. This is mostly fine, but there's one place it can cause problems.

In DUALNUM, JIT-compilation of BC_FORL will elide type checks for the loop control variables if they're all initialised by constants outside the loop and the runtime type of those variables at recording-time matches the type chosen by the JIT for those variables. Unfortunately, the runtime type of those variables can differ if stop+step overflows (because of the BC_FORI difference).

The following example shows this:

local function f(case)
  if case == 0 then end
  local n = 0
  for i = 2147483580, 2147483647 do
    if case == 1 then break end
    n = n + 1
  end
  return n
end
-- Compile the head of f, up to and including the FORI:
for i = 1, 50 do f(1) end
for i = 1, 50 do f(1) end
for i = 1, 50 do f(1) end
-- Initialise f's loop with a compiled FORI, then compile the FORL:
local f2 = f(2)
-- Initialise f's loop with an interpreted FORI, then hit the compiled FORL:
local f0 = f(0)
-- This can fail on DUALNUM:
assert(f2 == f0)

One possible solution might be:

diff --git a/src/lj_record.c b/src/lj_record.c
index b7af589697..1efa73bbcc 100644
--- a/src/lj_record.c
+++ b/src/lj_record.c
@@ -455,6 +455,14 @@ static void rec_for_check(jit_State *J, IRType t, int dir,
   }
 }
 
+#if LJ_DUALNUM
+static LJ_AINLINE int knumisint(jit_State *J, IRRef ref) {
+  IRIns *ir = IR(ref);
+  lua_Number n = ir_knum(ir)->n;
+  return (n == (lua_Number)lj_num2int(n));
+}
+#endif
+
 /* Record a FORL instruction. */
 static void rec_for_loop(jit_State *J, const BCIns *fori, ScEvEntry *scev,
 			 int init)
@@ -477,10 +485,15 @@ static void rec_for_loop(jit_State *J, const BCIns *fori, ScEvEntry *scev,
   scev->step = tref_ref(step);
   rec_for_check(J, t, dir, stop, step, init);
   scev->start = tref_ref(find_kinit(J, fori, ra+FORL_IDX, IRT_INT));
-  tc = (LJ_DUALNUM &&
-	!(scev->start && irref_isk(scev->stop) && irref_isk(scev->step) &&
-	  tvisint(&tv[FORL_IDX]) == (t == IRT_INT))) ?
-	IRSLOAD_TYPECHECK : 0;
+#if LJ_DUALNUM
+  tc = IRSLOAD_TYPECHECK;
+  if (scev->start && irref_isk(scev->stop) && irref_isk(scev->step)) {
+    if (t != IRT_NUM || !knumisint(J, scev->stop) || !knumisint(J, scev->step))
+      tc = 0;
+  }
+#else
+  tc = 0;
+#endif
   if (tc) {
     J->base[ra+FORL_STOP] = stop;
     J->base[ra+FORL_STEP] = step;

[corsix]

Variation that elides in more IRT_NUM cases:

diff --git a/src/lj_record.c b/src/lj_record.c
index b7af589697..b16a96dbd5 100644
--- a/src/lj_record.c
+++ b/src/lj_record.c
@@ -455,6 +455,18 @@ static void rec_for_check(jit_State *J, IRType t, int dir,
   }
 }
 
+#if LJ_DUALNUM
+static LJ_AINLINE int knumnotint(jit_State *J, IRRef ref) {
+  if (irref_isk(ref)) {
+    IRIns *ir = IR(ref);
+    lua_Number n = ir_knum(ir)->n;
+    if (n != (lua_Number)lj_num2int(n))
+      return 1; /* Is a KNUM, and not representable as an int. */
+  }
+  return 0;
+}
+#endif
+
 /* Record a FORL instruction. */
 static void rec_for_loop(jit_State *J, const BCIns *fori, ScEvEntry *scev,
 			 int init)
@@ -477,10 +489,18 @@ static void rec_for_loop(jit_State *J, const BCIns *fori, ScEvEntry *scev,
   scev->step = tref_ref(step);
   rec_for_check(J, t, dir, stop, step, init);
   scev->start = tref_ref(find_kinit(J, fori, ra+FORL_IDX, IRT_INT));
-  tc = (LJ_DUALNUM &&
-	!(scev->start && irref_isk(scev->stop) && irref_isk(scev->step) &&
-	  tvisint(&tv[FORL_IDX]) == (t == IRT_INT))) ?
-	IRSLOAD_TYPECHECK : 0;
+#if LJ_DUALNUM
+  tc = IRSLOAD_TYPECHECK;
+  if (t == IRT_INT) {
+    if (scev->start && irref_isk(scev->stop) && irref_isk(scev->step))
+      tc = 0;
+  } else {
+    if (knumnotint(J, scev->stop) || knumnotint(J, scev->step))
+      tc = 0;
+  }
+#else
+  tc = 0;
+#endif
   if (tc) {
     J->base[ra+FORL_STOP] = stop;
     J->base[ra+FORL_STEP] = step;