From 0ab070af96c99dd524d4f4a84bfa52313396a98e Mon Sep 17 00:00:00 2001 From: Wenyong Huang Date: Tue, 14 Jun 2022 07:28:53 +0800 Subject: [PATCH 1/3] Fix fast jit issues (#1223) Fix br if not clear_value issue Fix i64.DIV/REM issue: i64 result rax/rdx may be overwritten by following i32 load eax/edx --- core/iwasm/fast-jit/fe/jit_emit_control.c | 1 + core/iwasm/fast-jit/fe/jit_emit_numberic.c | 38 ++++++++++++++++------ 2 files changed, 29 insertions(+), 10 deletions(-) diff --git a/core/iwasm/fast-jit/fe/jit_emit_control.c b/core/iwasm/fast-jit/fe/jit_emit_control.c index ef699ca86..a0a3f96f4 100644 --- a/core/iwasm/fast-jit/fe/jit_emit_control.c +++ b/core/iwasm/fast-jit/fe/jit_emit_control.c @@ -899,6 +899,7 @@ jit_compile_op_br_if(JitCompContext *cc, uint32 br_depth, uint8 **p_frame_ip) SET_BUILDER_POS(if_basic_block); SET_BB_BEGIN_BCIP(if_basic_block, *p_frame_ip - 1); + clear_values(cc->jit_frame); if (!handle_op_br(cc, br_depth, p_frame_ip)) goto fail; diff --git a/core/iwasm/fast-jit/fe/jit_emit_numberic.c b/core/iwasm/fast-jit/fe/jit_emit_numberic.c index 3290fa9c7..e11a0de2a 100644 --- a/core/iwasm/fast-jit/fe/jit_emit_numberic.c +++ b/core/iwasm/fast-jit/fe/jit_emit_numberic.c @@ -512,7 +512,7 @@ compile_int_div_no_check(JitCompContext *cc, IntArithmetic arith_op, case INT_DIV_S: case INT_DIV_U: { - JitInsn *insn = NULL; + JitInsn *insn = NULL, *insn1 = NULL; if (is_i32) { GEN_INSN(MOV, eax_hreg, left); @@ -520,8 +520,6 @@ compile_int_div_no_check(JitCompContext *cc, IntArithmetic arith_op, insn = GEN_INSN(DIV_S, eax_hreg, eax_hreg, right); else insn = GEN_INSN(DIV_U, eax_hreg, eax_hreg, right); - - res = eax_hreg; } else { GEN_INSN(MOV, rax_hreg, left); @@ -529,18 +527,29 @@ compile_int_div_no_check(JitCompContext *cc, IntArithmetic arith_op, insn = GEN_INSN(DIV_S, rax_hreg, rax_hreg, right); else insn = GEN_INSN(DIV_U, rax_hreg, rax_hreg, right); - - res = rax_hreg; } jit_lock_reg_in_insn(cc, insn, eax_hreg); jit_lock_reg_in_insn(cc, insn, edx_hreg); + + if (is_i32) { + res = jit_cc_new_reg_I32(cc); + insn1 = jit_insn_new_MOV(res, eax_hreg); + } + else { + res = jit_cc_new_reg_I64(cc); + insn1 = jit_insn_new_MOV(res, rax_hreg); + } + + if (insn && insn1) { + jit_insn_insert_after(insn, insn1); + } break; } case INT_REM_S: case INT_REM_U: { - JitInsn *insn = NULL; + JitInsn *insn = NULL, *insn1 = NULL; if (is_i32) { GEN_INSN(MOV, eax_hreg, left); @@ -548,8 +557,6 @@ compile_int_div_no_check(JitCompContext *cc, IntArithmetic arith_op, insn = GEN_INSN(REM_S, edx_hreg, eax_hreg, right); else insn = GEN_INSN(REM_U, edx_hreg, eax_hreg, right); - - res = edx_hreg; } else { GEN_INSN(MOV, rax_hreg, left); @@ -557,12 +564,23 @@ compile_int_div_no_check(JitCompContext *cc, IntArithmetic arith_op, insn = GEN_INSN(REM_S, rdx_hreg, rax_hreg, right); else insn = GEN_INSN(REM_U, rdx_hreg, rax_hreg, right); - - res = rdx_hreg; } jit_lock_reg_in_insn(cc, insn, eax_hreg); jit_lock_reg_in_insn(cc, insn, edx_hreg); + + if (is_i32) { + res = jit_cc_new_reg_I32(cc); + insn1 = jit_insn_new_MOV(res, edx_hreg); + } + else { + res = jit_cc_new_reg_I64(cc); + insn1 = jit_insn_new_MOV(res, rdx_hreg); + } + + if (insn && insn1) { + jit_insn_insert_after(insn, insn1); + } break; } #else From 96fa546cc9d7429777e169c54eecf5b5b26856b2 Mon Sep 17 00:00:00 2001 From: "liang.he" Date: Tue, 14 Jun 2022 10:44:43 +0800 Subject: [PATCH 2/3] Add a sanitizer to check if there is a definition of vreg before its (#1224) --- core/iwasm/fast-jit/jit_regalloc.c | 31 ++++++++++++++++++++++++++++++ 1 file changed, 31 insertions(+) diff --git a/core/iwasm/fast-jit/jit_regalloc.c b/core/iwasm/fast-jit/jit_regalloc.c index 098d2bc99..c6644e76f 100644 --- a/core/iwasm/fast-jit/jit_regalloc.c +++ b/core/iwasm/fast-jit/jit_regalloc.c @@ -6,6 +6,10 @@ #include "jit_utils.h" #include "jit_compiler.h" +#if BH_DEBUG != 0 +#define VREG_DEF_SANITIZER +#endif + /** * A uint16 stack for storing distances of occurrences of virtual * registers. @@ -350,6 +354,29 @@ is_alloc_candidate(JitCompContext *cc, JitReg reg) && (!jit_cc_is_hreg(cc, reg) || !jit_cc_is_hreg_fixed(cc, reg))); } +#ifdef VREG_DEF_SANITIZER +static void +check_vreg_definition(RegallocContext *rc, JitInsn *insn) +{ + unsigned j; + JitRegVec regvec = jit_insn_opnd_regs(insn); + unsigned first_use = jit_insn_opnd_first_use(insn); + JitReg *regp; + + /* check if there is the definition of an vr before its references */ + JIT_REG_VEC_FOREACH_USE(regvec, j, regp, first_use) + { + VirtualReg *vr = NULL; + + if (!is_alloc_candidate(rc->cc, *regp)) + continue; + + vr = rc_get_vr(rc, *regp); + bh_assert(vr->distances); + } +} +#endif + /** * Collect distances from the beginning of basic block of all occurrences of * each virtual register. @@ -371,6 +398,10 @@ collect_distances(RegallocContext *rc, JitBasicBlock *basic_block) unsigned i; JitReg *regp; +#ifdef VREG_DEF_SANITIZER + check_vreg_definition(rc, insn); +#endif + /* NOTE: the distance may be pushed more than once if the virtual register occurs multiple times in the instruction. */ From 5340e3c3de2b015d8e55b20c60343df027124d48 Mon Sep 17 00:00:00 2001 From: "liang.he" Date: Tue, 14 Jun 2022 17:13:11 +0800 Subject: [PATCH 3/3] Fix sanitizer check issue when both def reg and ref reg are the same (#1226) Fix issue of instruction like MOV i3, i3 --- core/iwasm/fast-jit/jit_regalloc.c | 20 +++++++++++++++++--- 1 file changed, 17 insertions(+), 3 deletions(-) diff --git a/core/iwasm/fast-jit/jit_regalloc.c b/core/iwasm/fast-jit/jit_regalloc.c index c6644e76f..ba0d09452 100644 --- a/core/iwasm/fast-jit/jit_regalloc.c +++ b/core/iwasm/fast-jit/jit_regalloc.c @@ -358,19 +358,33 @@ is_alloc_candidate(JitCompContext *cc, JitReg reg) static void check_vreg_definition(RegallocContext *rc, JitInsn *insn) { - unsigned j; JitRegVec regvec = jit_insn_opnd_regs(insn); - unsigned first_use = jit_insn_opnd_first_use(insn); + unsigned i; JitReg *regp; + unsigned first_use = jit_insn_opnd_first_use(insn); + JitReg reg_defined; /* check if there is the definition of an vr before its references */ - JIT_REG_VEC_FOREACH_USE(regvec, j, regp, first_use) + JIT_REG_VEC_FOREACH(regvec, i, regp) { VirtualReg *vr = NULL; if (!is_alloc_candidate(rc->cc, *regp)) continue; + /*a strong assumption that there is only on defined reg*/ + if (i < first_use) { + reg_defined = *regp; + continue; + } + + /** + * both definition and references are in one instruction, + * like MOV i3,i3 + **/ + if (reg_defined == *regp) + continue; + vr = rc_get_vr(rc, *regp); bh_assert(vr->distances); }