<!DOCTYPE html>
<html data-lt-installed="true">
<head>
<meta http-equiv="Content-Type" content="text/html; charset=UTF-8">
</head>
<body style="padding-bottom: 1px;">
<p>Hello, Sergey,</p>
<p>thanks for the patch! LGTM with a minor comment.<br>
</p>
<div class="moz-cite-prefix">On 11.12.2024 16:21, Sergey Kaplun
wrote:<br>
</div>
<blockquote type="cite"
cite="mid:c6f0e186834eef000569d3911c5d8d1aedb2cebb.1733909411.git.skaplun@tarantool.org">
<pre class="moz-quote-pre" wrap="">From: Mike Pall <mike>
(cherry picked from commit de6b1a11dd1a3349179084578c5d533be1c30234)
Before this patch, Valgrind produces tons of warnings during the VMevent
calls since the `IR_NOP` instruction isn't fully initialized. Hence, any
parsing operations for it in handlers during `jit.dump()` leads to the
"uninitialised value" error. This patch fixes the issue by the proper
init of such IRs.</pre>
</blockquote>
<p>Please add to a comment that with these tests the problem could</p>
<p> be reproduced:</p>
<p>test/tarantool-tests/lj-1128-table-new-opt-tnew.test.lua</p>
<p>test/tarantool-tests/fix-jit-dump-ir-conv.test.lua</p>
<p>test/tarantool-tests/unit-jit-parse.test.lua<br>
</p>
<blockquote type="cite"
cite="mid:c6f0e186834eef000569d3911c5d8d1aedb2cebb.1733909411.git.skaplun@tarantool.org">
<pre class="moz-quote-pre" wrap="">
Sergey Kaplun:
* added the description for the problem
Needed for tarantool/tarantool#3705
Part of tarantool/tarantool#10709
---
src/lj_asm.c | 2 +-
src/lj_ir.h | 8 ++++++++
src/lj_opt_dce.c | 5 +----
src/lj_opt_mem.c | 25 +++++--------------------
4 files changed, 15 insertions(+), 25 deletions(-)
diff --git a/src/lj_asm.c b/src/lj_asm.c
index 5137d05c..f7540165 100644
--- a/src/lj_asm.c
+++ b/src/lj_asm.c
@@ -2377,7 +2377,7 @@ void lj_asm_trace(jit_State *J, GCtrace *T)
/* Ensure an initialized instruction beyond the last one for HIOP checks. */
/* This also allows one RENAME to be added without reallocating curfinal. */
as->orignins = lj_ir_nextins(J);
- J->cur.ir[as->orignins].o = IR_NOP;
+ lj_ir_nop(&J->cur.ir[as->orignins]);
/* Setup initial state. Copy some fields to reduce indirections. */
as->J = J;
diff --git a/src/lj_ir.h b/src/lj_ir.h
index 27c66f63..46e7413e 100644
--- a/src/lj_ir.h
+++ b/src/lj_ir.h
@@ -587,4 +587,12 @@ static LJ_AINLINE int ir_sideeff(IRIns *ir)
LJ_STATIC_ASSERT((int)IRT_GUARD == (int)IRM_W);
+/* Replace IR instruction with NOP. */
+static LJ_AINLINE void lj_ir_nop(IRIns *ir)
+{
+ ir->ot = IRT(IR_NOP, IRT_NIL);
+ ir->op1 = ir->op2 = 0;
+ ir->prev = 0;
+}
+
#endif
diff --git a/src/lj_opt_dce.c b/src/lj_opt_dce.c
index 6948179c..41e36d99 100644
--- a/src/lj_opt_dce.c
+++ b/src/lj_opt_dce.c
@@ -46,10 +46,7 @@ static void dce_propagate(jit_State *J)
irt_clearmark(ir->t);
} else if (!ir_sideeff(ir)) {
*pchain[ir->o] = ir->prev; /* Reroute original instruction chain. */
- ir->t.irt = IRT_NIL;
- ir->o = IR_NOP; /* Replace instruction with NOP. */
- ir->op1 = ir->op2 = 0;
- ir->prev = 0;
+ lj_ir_nop(ir);
continue;
}
pchain[ir->o] = &ir->prev;
diff --git a/src/lj_opt_mem.c b/src/lj_opt_mem.c
index c9f1216c..a6e9a072 100644
--- a/src/lj_opt_mem.c
+++ b/src/lj_opt_mem.c
@@ -375,10 +375,7 @@ TRef LJ_FASTCALL lj_opt_dse_ahstore(jit_State *J)
goto doemit; /* No elimination possible. */
/* Remove redundant store from chain and replace with NOP. */
*refp = store->prev;
- store->o = IR_NOP;
- store->t.irt = IRT_NIL;
- store->op1 = store->op2 = 0;
- store->prev = 0;
+ lj_ir_nop(store);
/* Now emit the new store instead. */
}
goto doemit;
@@ -479,10 +476,7 @@ TRef LJ_FASTCALL lj_opt_dse_ustore(jit_State *J)
goto doemit; /* No elimination possible. */
/* Remove redundant store from chain and replace with NOP. */
*refp = store->prev;
- store->o = IR_NOP;
- store->t.irt = IRT_NIL;
- store->op1 = store->op2 = 0;
- store->prev = 0;
+ lj_ir_nop(store);
if (ref+1 < J->cur.nins &&
store[1].o == IR_OBAR && store[1].op1 == xref) {
IRRef1 *bp = &J->chain[IR_OBAR];
@@ -491,10 +485,7 @@ TRef LJ_FASTCALL lj_opt_dse_ustore(jit_State *J)
bp = &obar->prev;
/* Remove OBAR, too. */
*bp = obar->prev;
- obar->o = IR_NOP;
- obar->t.irt = IRT_NIL;
- obar->op1 = obar->op2 = 0;
- obar->prev = 0;
+ lj_ir_nop(obar);
}
/* Now emit the new store instead. */
}
@@ -585,10 +576,7 @@ TRef LJ_FASTCALL lj_opt_dse_fstore(jit_State *J)
goto doemit; /* No elimination possible. */
/* Remove redundant store from chain and replace with NOP. */
*refp = store->prev;
- store->o = IR_NOP;
- store->t.irt = IRT_NIL;
- store->op1 = store->op2 = 0;
- store->prev = 0;
+ lj_ir_nop(store);
/* Now emit the new store instead. */
}
goto doemit;
@@ -839,10 +827,7 @@ TRef LJ_FASTCALL lj_opt_dse_xstore(jit_State *J)
goto doemit; /* No elimination possible. */
/* Remove redundant store from chain and replace with NOP. */
*refp = store->prev;
- store->o = IR_NOP;
- store->t.irt = IRT_NIL;
- store->op1 = store->op2 = 0;
- store->prev = 0;
+ lj_ir_nop(store);
/* Now emit the new store instead. */
}
goto doemit;
</pre>
</blockquote>
</body>
<lt-container></lt-container>
</html>