[Tarantool-patches] [PATCH luajit] Simplify handling of instable types in TNEW/TDUP load forwarding.

[Tarantool-patches] [PATCH luajit] Simplify handling of instable types in TNEW/TDUP load forwarding.

[Sergey Kaplun via Tarantool-patches]

From: Mike Pall <mike>

Thanks to Peter Cawley.

(cherry picked from commit c42c62e71a45a677b8b1cbf749bd33cf4d5918ff)

This patch is a follow-up for the commit
b89186cb03e79359847cdbeb000a830cc464db35 ("Fix handling of instable
types in TNEW/TDUP load forwarding."). The problem still occurs when
TDUP load forwarding is performed for the boolean value. The
`fwd_ahload()` always takes the type of IR without checking the type of
the TValue for this iteration.

This patch solves the issue by returning the expected type of the TValue
from the constant table (or `nil` for TNEW IR) instead. In the case of
type instability, the error in `loop_unroll()` will be raised, and the
loop will be unrolled for one more iteration to prevent inconsistencies.

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

Part of tarantool/tarantool#9595
---

Branch: https://github.com/tarantool/luajit/tree/skaplun/lj-994-instable-pri-types
Tarantool PR: https://github.com/tarantool/tarantool/pull/9608
Related issues:
* https://github.com/tarantool/tarantool/issues/9595
* https://github.com/LuaJIT/LuaJIT/issues/994

 src/lj_opt_mem.c                              | 20 +++++++++----------
 ...instable-types-during-loop-unroll.test.lua | 15 +++++++++++++-
 2 files changed, 23 insertions(+), 12 deletions(-)

diff --git a/src/lj_opt_mem.c b/src/lj_opt_mem.c
index 24a490d5..9044f09a 100644
--- a/src/lj_opt_mem.c
+++ b/src/lj_opt_mem.c
@@ -217,25 +217,23 @@ static TRef fwd_ahload(jit_State *J, IRRef xref)
 	}
 	ref = store->prev;
       }
-      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)) ||
-		 irt_isstr(fins->t)) {
+      /* Simplified here: let loop_unroll() figure out any type instability. */
+      if (ir->o == IR_TNEW) {
+	return TREF_NIL;
+      } else {
 	TValue keyv;
 	cTValue *tv;
 	IRIns *key = IR(xr->op2);
 	if (key->o == IR_KSLOT) key = IR(key->op1);
 	lj_ir_kvalue(J->L, &keyv, key);
 	tv = lj_tab_get(J->L, ir_ktab(IR(ir->op1)), &keyv);
-	if (itype2irt(tv) != irt_type(fins->t))
-	  return 0;  /* Type instability in loop-carried dependency. */
-	if (irt_isnum(fins->t))
+	if (tvispri(tv))
+	  return TREF_PRI(itype2irt(tv));
+	else if (tvisnum(tv))
 	  return lj_ir_knum_u64(J, tv->u64);
-	else if (LJ_DUALNUM && irt_isint(fins->t))
+	else if (tvisint(tv))
 	  return lj_ir_kint(J, intV(tv));
-	else
+	else if (tvisgcv(tv))
 	  return lj_ir_kstr(J, strV(tv));
       }
       /* Othwerwise: don't intern as a constant. */
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
index f240bdd2..a0cc8487 100644
--- 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
@@ -10,7 +10,7 @@ local test = tap.test('lj-994-instable-types-during-loop-unroll'):skipcond({
 
 -- TODO: test that compiled traces don't always exit by the type
 -- guard. See also the comment for the TDUP test chunk.
-test:plan(2)
+test:plan(3)
 
 -- TNEW.
 local result
@@ -46,4 +46,17 @@ for _ = 1, 5 do
 end
 test:is(result, true, 'TDUP load forwarding was successful')
 
+-- TDUP, primitive types.
+for i = 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]
+  -- The constant tables should contain different booleans
+  -- (primitive types).
+  slot = i % 2 ~= 0 and {false} or {true}
+end
+test:is(result, true, 'TDUP load forwarding (primitive types) was successful')
+
 test:done(true)
-- 
2.43.0

Re: [Tarantool-patches] [PATCH luajit] Simplify handling of instable types in TNEW/TDUP load forwarding.

[Sergey Bronnikov via Tarantool-patches]

Hi, Sergey!

thanks for the patch!

Everything is fine, but a have a question -

test reproduces a problem with GC32 only, I couldn't reproduce it with GC64.

Is it expected? As I got it right, the problem is not GC-specific.


Used CMake options:


CMAKE_BUILD_TYPE="Debug"
CMAKE_C_COMPILER="clang"
CMAKE_EXPORT_COMPILE_COMMANDS:BOOL="TRUE"
LUAJIT_ENABLE_COVERAGE:BOOL="TRUE"
LUAJIT_ENABLE_GC64:BOOL="TRUE"
LUAJIT_USE_ASAN:BOOL="TRUE"
LUAJIT_USE_SYSMALLOC:BOOL="TRUE"
LUA_USE_APICHECK:BOOL="TRUE"
   LUA_USE_ASSERT:BOOL="TRUE"


Sergey

On 1/22/24 14:03, Sergey Kaplun wrote:
> From: Mike Pall <mike>
>
> Thanks to Peter Cawley.
>
> (cherry picked from commit c42c62e71a45a677b8b1cbf749bd33cf4d5918ff)
>
> This patch is a follow-up for the commit
> b89186cb03e79359847cdbeb000a830cc464db35 ("Fix handling of instable
> types in TNEW/TDUP load forwarding."). The problem still occurs when
> TDUP load forwarding is performed for the boolean value. The
> `fwd_ahload()` always takes the type of IR without checking the type of
> the TValue for this iteration.
>
> This patch solves the issue by returning the expected type of the TValue
> from the constant table (or `nil` for TNEW IR) instead. In the case of
> type instability, the error in `loop_unroll()` will be raised, and the
> loop will be unrolled for one more iteration to prevent inconsistencies.
>
> Sergey Kaplun:
> * added the description and the test for the problem
>
> Part of tarantool/tarantool#9595
> ---
>
> Branch: https://github.com/tarantool/luajit/tree/skaplun/lj-994-instable-pri-types
> Tarantool PR: https://github.com/tarantool/tarantool/pull/9608
> Related issues:
> * https://github.com/tarantool/tarantool/issues/9595
> * https://github.com/LuaJIT/LuaJIT/issues/994
>
>   src/lj_opt_mem.c                              | 20 +++++++++----------
>   ...instable-types-during-loop-unroll.test.lua | 15 +++++++++++++-
>   2 files changed, 23 insertions(+), 12 deletions(-)
>
> diff --git a/src/lj_opt_mem.c b/src/lj_opt_mem.c
> index 24a490d5..9044f09a 100644
> --- a/src/lj_opt_mem.c
> +++ b/src/lj_opt_mem.c
> @@ -217,25 +217,23 @@ static TRef fwd_ahload(jit_State *J, IRRef xref)
>   	}
>   	ref = store->prev;
>         }
> -      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)) ||
> -		 irt_isstr(fins->t)) {
> +      /* Simplified here: let loop_unroll() figure out any type instability. */
> +      if (ir->o == IR_TNEW) {
> +	return TREF_NIL;
> +      } else {
>   	TValue keyv;
>   	cTValue *tv;
>   	IRIns *key = IR(xr->op2);
>   	if (key->o == IR_KSLOT) key = IR(key->op1);
>   	lj_ir_kvalue(J->L, &keyv, key);
>   	tv = lj_tab_get(J->L, ir_ktab(IR(ir->op1)), &keyv);
> -	if (itype2irt(tv) != irt_type(fins->t))
> -	  return 0;  /* Type instability in loop-carried dependency. */
> -	if (irt_isnum(fins->t))
> +	if (tvispri(tv))
> +	  return TREF_PRI(itype2irt(tv));
> +	else if (tvisnum(tv))
>   	  return lj_ir_knum_u64(J, tv->u64);
> -	else if (LJ_DUALNUM && irt_isint(fins->t))
> +	else if (tvisint(tv))
>   	  return lj_ir_kint(J, intV(tv));
> -	else
> +	else if (tvisgcv(tv))
>   	  return lj_ir_kstr(J, strV(tv));
>         }
>         /* Othwerwise: don't intern as a constant. */
> 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
> index f240bdd2..a0cc8487 100644
> --- 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
> @@ -10,7 +10,7 @@ local test = tap.test('lj-994-instable-types-during-loop-unroll'):skipcond({
>   
>   -- TODO: test that compiled traces don't always exit by the type
>   -- guard. See also the comment for the TDUP test chunk.
> -test:plan(2)
> +test:plan(3)
>   
>   -- TNEW.
>   local result
> @@ -46,4 +46,17 @@ for _ = 1, 5 do
>   end
>   test:is(result, true, 'TDUP load forwarding was successful')
>   
> +-- TDUP, primitive types.
> +for i = 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]
> +  -- The constant tables should contain different booleans
> +  -- (primitive types).
> +  slot = i % 2 ~= 0 and {false} or {true}
> +end
> +test:is(result, true, 'TDUP load forwarding (primitive types) was successful')
> +
>   test:done(true)

Re: [Tarantool-patches] [PATCH luajit] Simplify handling of instable types in TNEW/TDUP load forwarding.

[Sergey Kaplun via Tarantool-patches]

Hi, Sergey!
Thanks for the review!

On 23.01.24, Sergey Bronnikov wrote:
> Hi, Sergey!
> 
> thanks for the patch!
> 
> Everything is fine, but a have a question -
> 
> test reproduces a problem with GC32 only, I couldn't reproduce it with GC64.
> 
> Is it expected? As I got it right, the problem is not GC-specific.

Just checked on the current master without ASAN, SYSMALLOC, COVERAGE and
got failed test:
| /home/burii/builds_workspace/luajit/master/test/tarantool-tests/lj-994-instable-types-during-loop-unroll.test.lua ........ Dubious, test returned 1 (wstat 256,
|  0x100)
| Failed 1/3 subtests
| ...
| /home/burii/builds_workspace/luajit/master/src/luajit -e 'print(require"ffi".abi"gc64")'
| true

Maybe it's affected by Coverage|ASAN flags, I don't know.

But still valid test case for GC64 build.

> 
> 
> Used CMake options:
> 
> 
> CMAKE_BUILD_TYPE="Debug"
> CMAKE_C_COMPILER="clang"
> CMAKE_EXPORT_COMPILE_COMMANDS:BOOL="TRUE"
> LUAJIT_ENABLE_COVERAGE:BOOL="TRUE"
> LUAJIT_ENABLE_GC64:BOOL="TRUE"
> LUAJIT_USE_ASAN:BOOL="TRUE"
> LUAJIT_USE_SYSMALLOC:BOOL="TRUE"
> LUA_USE_APICHECK:BOOL="TRUE"
>    LUA_USE_ASSERT:BOOL="TRUE"
> 
> 
> Sergey
> 

-- 
Best regards,
Sergey Kaplun

Re: [Tarantool-patches] [PATCH luajit] Simplify handling of instable types in TNEW/TDUP load forwarding.

[Sergey Bronnikov via Tarantool-patches]

Hi,

On 1/23/24 16:22, Sergey Kaplun wrote:
> Hi, Sergey!
> Thanks for the review!
>
> On 23.01.24, Sergey Bronnikov wrote:
>> Hi, Sergey!
>>
>> thanks for the patch!
>>
>> Everything is fine, but a have a question -
>>
>> test reproduces a problem with GC32 only, I couldn't reproduce it with GC64.
>>
>> Is it expected? As I got it right, the problem is not GC-specific.
> Just checked on the current master without ASAN, SYSMALLOC, COVERAGE and
> got failed test:
> | /home/burii/builds_workspace/luajit/master/test/tarantool-tests/lj-994-instable-types-during-loop-unroll.test.lua ........ Dubious, test returned 1 (wstat 256,
> |  0x100)
> | Failed 1/3 subtests
> | ...
> | /home/burii/builds_workspace/luajit/master/src/luajit -e 'print(require"ffi".abi"gc64")'
> | true
>
> Maybe it's affected by Coverage|ASAN flags, I don't know.

The problem was due to enabled ASAN.


> But still valid test case for GC64 build.
>
>>
>> Used CMake options:
>>
>>
>> CMAKE_BUILD_TYPE="Debug"
>> CMAKE_C_COMPILER="clang"
>> CMAKE_EXPORT_COMPILE_COMMANDS:BOOL="TRUE"
>> LUAJIT_ENABLE_COVERAGE:BOOL="TRUE"
>> LUAJIT_ENABLE_GC64:BOOL="TRUE"
>> LUAJIT_USE_ASAN:BOOL="TRUE"
>> LUAJIT_USE_SYSMALLOC:BOOL="TRUE"
>> LUA_USE_APICHECK:BOOL="TRUE"
>>     LUA_USE_ASSERT:BOOL="TRUE"
>>
>>
>> Sergey
>>

Re: [Tarantool-patches] [PATCH luajit] Simplify handling of instable types in TNEW/TDUP load forwarding.

[Sergey Bronnikov via Tarantool-patches]

Hi, Sergey

thanks for the patch! LGTM

On 1/22/24 14:03, Sergey Kaplun wrote:
> From: Mike Pall <mike>
>
> Thanks to Peter Cawley.
>
> (cherry picked from commit c42c62e71a45a677b8b1cbf749bd33cf4d5918ff)
>
> This patch is a follow-up for the commit
> b89186cb03e79359847cdbeb000a830cc464db35 ("Fix handling of instable
> types in TNEW/TDUP load forwarding."). The problem still occurs when
> TDUP load forwarding is performed for the boolean value. The
> `fwd_ahload()` always takes the type of IR without checking the type of
> the TValue for this iteration.
>
> This patch solves the issue by returning the expected type of the TValue
> from the constant table (or `nil` for TNEW IR) instead. In the case of
> type instability, the error in `loop_unroll()` will be raised, and the
> loop will be unrolled for one more iteration to prevent inconsistencies.
>
> Sergey Kaplun:
> * added the description and the test for the problem
>
> Part of tarantool/tarantool#9595
> ---
>
> Branch: https://github.com/tarantool/luajit/tree/skaplun/lj-994-instable-pri-types
> Tarantool PR: https://github.com/tarantool/tarantool/pull/9608
> Related issues:
> * https://github.com/tarantool/tarantool/issues/9595
> * https://github.com/LuaJIT/LuaJIT/issues/994
>
>   src/lj_opt_mem.c                              | 20 +++++++++----------
>   ...instable-types-during-loop-unroll.test.lua | 15 +++++++++++++-
>   2 files changed, 23 insertions(+), 12 deletions(-)
>
> diff --git a/src/lj_opt_mem.c b/src/lj_opt_mem.c
> index 24a490d5..9044f09a 100644
> --- a/src/lj_opt_mem.c
> +++ b/src/lj_opt_mem.c
> @@ -217,25 +217,23 @@ static TRef fwd_ahload(jit_State *J, IRRef xref)
>   	}
>   	ref = store->prev;
>         }
> -      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)) ||
> -		 irt_isstr(fins->t)) {
> +      /* Simplified here: let loop_unroll() figure out any type instability. */
> +      if (ir->o == IR_TNEW) {
> +	return TREF_NIL;
> +      } else {
>   	TValue keyv;
>   	cTValue *tv;
>   	IRIns *key = IR(xr->op2);
>   	if (key->o == IR_KSLOT) key = IR(key->op1);
>   	lj_ir_kvalue(J->L, &keyv, key);
>   	tv = lj_tab_get(J->L, ir_ktab(IR(ir->op1)), &keyv);
> -	if (itype2irt(tv) != irt_type(fins->t))
> -	  return 0;  /* Type instability in loop-carried dependency. */
> -	if (irt_isnum(fins->t))
> +	if (tvispri(tv))
> +	  return TREF_PRI(itype2irt(tv));
> +	else if (tvisnum(tv))
>   	  return lj_ir_knum_u64(J, tv->u64);
> -	else if (LJ_DUALNUM && irt_isint(fins->t))
> +	else if (tvisint(tv))
>   	  return lj_ir_kint(J, intV(tv));
> -	else
> +	else if (tvisgcv(tv))
>   	  return lj_ir_kstr(J, strV(tv));
>         }
>         /* Othwerwise: don't intern as a constant. */
> 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
> index f240bdd2..a0cc8487 100644
> --- 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
> @@ -10,7 +10,7 @@ local test = tap.test('lj-994-instable-types-during-loop-unroll'):skipcond({
>   
>   -- TODO: test that compiled traces don't always exit by the type
>   -- guard. See also the comment for the TDUP test chunk.
> -test:plan(2)
> +test:plan(3)
>   
>   -- TNEW.
>   local result
> @@ -46,4 +46,17 @@ for _ = 1, 5 do
>   end
>   test:is(result, true, 'TDUP load forwarding was successful')
>   
> +-- TDUP, primitive types.
> +for i = 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]
> +  -- The constant tables should contain different booleans
> +  -- (primitive types).
> +  slot = i % 2 ~= 0 and {false} or {true}
> +end
> +test:is(result, true, 'TDUP load forwarding (primitive types) was successful')
> +
>   test:done(true)

Re: [Tarantool-patches] [PATCH luajit] Simplify handling of instable types in TNEW/TDUP load forwarding.

[Maxim Kokryashkin via Tarantool-patches]

[-- Attachment #1: Type: text/plain, Size: 4128 bytes --]


Hi, Sergey!
Thanks for the patch!
LGTM
--
Best regards,
Maxim Kokryashkin
 
  
>Понедельник, 22 января 2024, 14:08 +03:00 от Sergey Kaplun <skaplun@tarantool.org>:
> 
>From: Mike Pall <mike>
>
>Thanks to Peter Cawley.
>
>(cherry picked from commit c42c62e71a45a677b8b1cbf749bd33cf4d5918ff)
>
>This patch is a follow-up for the commit
>b89186cb03e79359847cdbeb000a830cc464db35 ("Fix handling of instable
>types in TNEW/TDUP load forwarding."). The problem still occurs when
>TDUP load forwarding is performed for the boolean value. The
>`fwd_ahload()` always takes the type of IR without checking the type of
>the TValue for this iteration.
>
>This patch solves the issue by returning the expected type of the TValue
>from the constant table (or `nil` for TNEW IR) instead. In the case of
>type instability, the error in `loop_unroll()` will be raised, and the
>loop will be unrolled for one more iteration to prevent inconsistencies.
>
>Sergey Kaplun:
>* added the description and the test for the problem
>
>Part of tarantool/tarantool#9595
>---
>
>Branch:  https://github.com/tarantool/luajit/tree/skaplun/lj-994-instable-pri-types
>Tarantool PR:  https://github.com/tarantool/tarantool/pull/9608
>Related issues:
>*  https://github.com/tarantool/tarantool/issues/9595
>*  https://github.com/LuaJIT/LuaJIT/issues/994
>
> src/lj_opt_mem.c | 20 +++++++++----------
> ...instable-types-during-loop-unroll.test.lua | 15 +++++++++++++-
> 2 files changed, 23 insertions(+), 12 deletions(-)
>
>diff --git a/src/lj_opt_mem.c b/src/lj_opt_mem.c
>index 24a490d5..9044f09a 100644
>--- a/src/lj_opt_mem.c
>+++ b/src/lj_opt_mem.c
>@@ -217,25 +217,23 @@ static TRef fwd_ahload(jit_State *J, IRRef xref)
>  }
>  ref = store->prev;
>       }
>- 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)) ||
>- irt_isstr(fins->t)) {
>+ /* Simplified here: let loop_unroll() figure out any type instability. */
>+ if (ir->o == IR_TNEW) {
>+ return TREF_NIL;
>+ } else {
>  TValue keyv;
>  cTValue *tv;
>  IRIns *key = IR(xr->op2);
>  if (key->o == IR_KSLOT) key = IR(key->op1);
>  lj_ir_kvalue(J->L, &keyv, key);
>  tv = lj_tab_get(J->L, ir_ktab(IR(ir->op1)), &keyv);
>- if (itype2irt(tv) != irt_type(fins->t))
>- return 0; /* Type instability in loop-carried dependency. */
>- if (irt_isnum(fins->t))
>+ if (tvispri(tv))
>+ return TREF_PRI(itype2irt(tv));
>+ else if (tvisnum(tv))
>  return lj_ir_knum_u64(J, tv->u64);
>- else if (LJ_DUALNUM && irt_isint(fins->t))
>+ else if (tvisint(tv))
>  return lj_ir_kint(J, intV(tv));
>- else
>+ else if (tvisgcv(tv))
>  return lj_ir_kstr(J, strV(tv));
>       }
>       /* Othwerwise: don't intern as a constant. */
>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
>index f240bdd2..a0cc8487 100644
>--- 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
>@@ -10,7 +10,7 @@ local test = tap.test('lj-994-instable-types-during-loop-unroll'):skipcond({
> 
> -- TODO: test that compiled traces don't always exit by the type
> -- guard. See also the comment for the TDUP test chunk.
>-test:plan(2)
>+test:plan(3)
> 
> -- TNEW.
> local result
>@@ -46,4 +46,17 @@ for _ = 1, 5 do
> end
> test:is(result, true, 'TDUP load forwarding was successful')
> 
>+-- TDUP, primitive types.
>+for i = 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]
>+ -- The constant tables should contain different booleans
>+ -- (primitive types).
>+ slot = i % 2 ~= 0 and {false} or {true}
>+end
>+test:is(result, true, 'TDUP load forwarding (primitive types) was successful')
>+
> test:done(true)
>--
>2.43.0
 

[-- Attachment #2: Type: text/html, Size: 5389 bytes --]

Re: [Tarantool-patches] [PATCH luajit] Simplify handling of instable types in TNEW/TDUP load forwarding.

[Igor Munkin via Tarantool-patches]

Sergey,

I've checked the patchset into all long-term branches in
tarantool/luajit and bumped a new version in master, release/3.0 and
release/2.11.

On 22.01.24, Sergey Kaplun via Tarantool-patches wrote:
> From: Mike Pall <mike>
> 
> Thanks to Peter Cawley.
> 
> (cherry picked from commit c42c62e71a45a677b8b1cbf749bd33cf4d5918ff)
> 
> This patch is a follow-up for the commit
> b89186cb03e79359847cdbeb000a830cc464db35 ("Fix handling of instable
> types in TNEW/TDUP load forwarding."). The problem still occurs when
> TDUP load forwarding is performed for the boolean value. The
> `fwd_ahload()` always takes the type of IR without checking the type of
> the TValue for this iteration.
> 
> This patch solves the issue by returning the expected type of the TValue
> from the constant table (or `nil` for TNEW IR) instead. In the case of
> type instability, the error in `loop_unroll()` will be raised, and the
> loop will be unrolled for one more iteration to prevent inconsistencies.
> 
> Sergey Kaplun:
> * added the description and the test for the problem
> 
> Part of tarantool/tarantool#9595
> ---
> 
> Branch: https://github.com/tarantool/luajit/tree/skaplun/lj-994-instable-pri-types
> Tarantool PR: https://github.com/tarantool/tarantool/pull/9608
> Related issues:
> * https://github.com/tarantool/tarantool/issues/9595
> * https://github.com/LuaJIT/LuaJIT/issues/994
> 
>  src/lj_opt_mem.c                              | 20 +++++++++----------
>  ...instable-types-during-loop-unroll.test.lua | 15 +++++++++++++-
>  2 files changed, 23 insertions(+), 12 deletions(-)
> 

<snipped>

> -- 
> 2.43.0
> 

-- 
Best regards,
IM