[Tarantool-patches] [PATCH luajit] Fix TNEW load forwarding with instable types.

[Sergey Kaplun]

From: Mike Pall <mike>

(cherry picked from commit d4c0c6e17ef7edf1f2893bc807746b80612e63e9)

During loop unrolling IR instructions are coped, substituted and
re-emitted for the FOLD/CSE/etc. pipeline. Assume, we have the following
loop-carried dependency: we create a new table or load some stored one
depends on iteration number. In case when we copy the emitted ALOAD IR
instruction (from from the stored table), we copy it as is, including
loading type. But the ALOAD from the new table created (on the previous
iteration) should have nil type, so the assertion in `fwd_ahload()` is
failed.

This patch replaces the assertion to `return 0` to avoid the assertion
failure and stop forwarding in such situations.

But the emitted type-guarded ALOAD will lead to the persistent failure
of the assertion guard on the trace. Hence, another one fix should be
added. Also, TDUP IR is affected, too.
See also the issue mentioned in the test.

Sergey Kaplun:
* added the description and the test for the problem

Part of tarantool/tarantool#8516
---

Branch: https://github.com/tarantool/luajit/tree/skaplun/lj-994-instable-types-during-loop-unroll
PR: https://github.com/tarantool/tarantool/pull/8642
Related issues:
* https://github.com/tarantool/tarantool/issues/8516
* https://github.com/LuaJIT/LuaJIT/issues/994

I don't mention the 994 intentionally to avoid Mike bothering. Also, it
isn't the origin of this commit. Quite the opposite: as a result of this
backporting the following issue was created.

I prefer to backport the patches (this one and the prospective for 994)
separately. So, after this patch is backported, it doesn't add any
critical bugs (always failing guard just creates a new side trace), but
helps to avoid conflicts for future backporting (sadly remembered
"Improve assertions.").

 src/lj_opt_mem.c                              |  3 +-
 ...instable-types-during-loop-unroll.test.lua | 53 +++++++++++++++++++
 2 files changed, 55 insertions(+), 1 deletion(-)
 create mode 100644 test/tarantool-tests/lj-994-instable-types-during-loop-unroll.test.lua

diff --git a/src/lj_opt_mem.c b/src/lj_opt_mem.c
index cc177d39..c8265b4f 100644
--- a/src/lj_opt_mem.c
+++ b/src/lj_opt_mem.c
@@ -180,7 +180,8 @@ static TRef fwd_ahload(jit_State *J, IRRef xref)
 	}
 	ref = store->prev;
       }
-      lua_assert(ir->o != IR_TNEW || irt_isnil(fins->t));
+      if (ir->o == IR_TNEW && !irt_isnil(fins->t))
+	return 0;  /* Type instability in loop-carried dependency. */
       if (irt_ispri(fins->t)) {
 	return TREF_PRI(irt_type(fins->t));
       } else if (irt_isnum(fins->t) || (LJ_DUALNUM && irt_isint(fins->t)) ||
diff --git a/test/tarantool-tests/lj-994-instable-types-during-loop-unroll.test.lua b/test/tarantool-tests/lj-994-instable-types-during-loop-unroll.test.lua
new file mode 100644
index 00000000..435f6e0e
--- /dev/null
+++ b/test/tarantool-tests/lj-994-instable-types-during-loop-unroll.test.lua
@@ -0,0 +1,53 @@
+local tap = require('tap')
+
+local test = tap.test('lj-994-instable-types-during-loop-unroll'):skipcond({
+  ['Test requires JIT enabled'] = not jit.status(),
+})
+
+-- Test file to demonstrate LuaJIT misbehaviour during loop
+-- unrolling and load forwarding for newly created tables.
+-- See also https://github.com/LuaJIT/LuaJIT/issues/994.
+
+-- TODO: test that compiled traces don't always exit by the type
+-- guard. See also the comment for the TDUP test chunk.
+test:plan(1)
+
+-- TNEW.
+local result
+local stored_tab = {1}
+local slot = {}
+local key = 1
+
+jit.opt.start('hotloop=1')
+-- The trouble happens during loop unrolling when we copy
+-- `>+ num ALOAD` IR in the context of the table on the previous
+-- iteration instead of a new one created via TNEW containing no
+-- values (so type nil should be used instead of num).
+for _ = 1, 5 do
+  local t = slot
+  -- Use non-constant key to avoid LJ_TRERR_GFAIL and undoing the
+  -- loop.
+  result = t[key]
+  -- Swap table loaded by SLOAD to the created via TNEW.
+  slot = _ % 2 ~= 0 and stored_tab or {}
+end
+test:is(result, nil, 'TNEW load forwarding was successful')
+
+-- TDUP.
+--[[
+-- FIXME: Disable test case for now. Enable, with another one
+-- backported commit with a fix for the aforementioned issue
+-- (and after patch "Improve assertions.").
+-- Test taken trace exits too.
+for _ = 1, 5 do
+  local t = slot
+  -- Now use constant key slot to get necessary branch.
+  -- LJ_TRERR_GFAIL isn't triggered here.
+  -- See `fwd_ahload()` in <src/lj_opt_mem.c> for details.
+  result = t[1]
+  slot = _ % 2 ~= 0 and stored_tab or {true}
+end
+test:is(result, true, 'TDUP load forwarding was successful')
+]]
+
+os.exit(test:check() and 0 or 1)
-- 
2.34.1