diff --git a/ChangeLog b/ChangeLog index 85a685547..e91a7c647 100644 --- a/ChangeLog +++ b/ChangeLog @@ -1,3 +1,20 @@ +2013-02-05 Paulo Andrade + + *include/lightning.h, lib/lightning.c: Add the new jit_live code + to explicitly mark a register as live. It is required to avoid + assuming functions always return a value in the gpr and fpr return + register, and to avoid the need of some very specialized codes + that vary too much from backend to backend, to instruct the + optimization code the return register is live. + + * lib/jit_arm.c, lib/jit_mips.c, lib/jit_ppc.c, lib/jit_print.c, + lib/jit_x86.c: Update for the new jit_live code. + + * check/ret.ok, check/ret.tst: New files implementing a simple + test case that would previously fail at least in ix86/x86_64. + + * check/Makefile.am: Update for new "ret" test case. + 2013-02-05 Paulo Andrade * lib/jit_ppc-cpu.c, lib/jit_ppc.c: Validate and correct diff --git a/TODO b/TODO index 5fd2df361..a2d9a25b2 100644 --- a/TODO +++ b/TODO @@ -13,16 +13,6 @@ and "i" variants, and possibly unsigned version. Branches would use "bo" and "bx" prefix. - * Convert retr to an actual node, otherwise, code like: - movi %r0 1 - divr %r1 %r2 %r3 - retr %r0 - will fail in x86 because, besides "divr" telling it clobbers - %rax (r0) it ends being ignored because retr is a noop there - (removed "mov %rax,%rax" expansion) and the code checking for - live registers ends up not knowing about %rax being live after - the "divr". This affects only x86. - * Validate that divrem in jit_x86-cpu.c is not modifying the non result arguments. This is not verified by clobber.tst, as it only checks registers not involved in the operation diff --git a/check/Makefile.am b/check/Makefile.am index 34fbbcf59..220c99a37 100644 --- a/check/Makefile.am +++ b/check/Makefile.am @@ -69,6 +69,7 @@ EXTRA_DIST = \ qalu.inc \ qalu_mul.tst qalu_mul.ok \ qalu_div.tst qalu_div.ok \ + ret.tst ret.ok \ ccall.ok \ check.sh \ check.x87.sh \ @@ -92,7 +93,8 @@ base_TESTS = \ varargs stack \ clobber carry call \ float \ - qalu_mul qalu_div + qalu_mul qalu_div \ + ret $(base_TESTS): check.sh $(LN_S) $(srcdir)/check.sh $@ diff --git a/check/ret.ok b/check/ret.ok new file mode 100644 index 000000000..9766475a4 --- /dev/null +++ b/check/ret.ok @@ -0,0 +1 @@ +ok diff --git a/check/ret.tst b/check/ret.tst new file mode 100644 index 000000000..de1d82c01 --- /dev/null +++ b/check/ret.tst @@ -0,0 +1,51 @@ +.data 16 +ok: +.c "ok" + +.code + jmpi main + +/* + * very simple test on purpose because otherwise it would not trigger + * the bug where the retr %r0 or retr_d %f0 would be omitted because + * the argument was already the return register, but the register end + * clobbered by another instruction, like the div*, and the wrong + * value returned because the retr* was removed and this way, lost + * information that the register was live at function exit. + */ + +check_r0: + prolog + movi %r0 1 + movi %r2 10 + // on x86 this changes %rax on other arches could use %r0 as temporary + divi %r1 %r2 3 + // %r0 must still be 1 + retr %r0 + epilog + +check_f0: + prolog + movi_d %f0 0.5 + movi_d %f2 10 + divi_d %f1 %f2 3 + retr_d %f0 + epilog + +main: + prolog + calli check_r0 + retval %r1 + beqi r0_ok %r1 1 + calli @abort +r0_ok: + calli check_f0 + retval_d %f1 + beqi_d f0_ok %f1 0.5 + calli @abort +f0_ok: + prepare + pushargi ok + finishi @puts + ret + epilog diff --git a/include/lightning.h b/include/lightning.h index 3694616cb..057b45a12 100644 --- a/include/lightning.h +++ b/include/lightning.h @@ -101,6 +101,8 @@ typedef struct jit_state jit_state_t; typedef enum { #define jit_data(u,v,w) _jit_data(_jit,u,v,w) jit_code_data, +#define jit_live(u) jit_new_node_w(jit_code_live, u) + jit_code_live, jit_code_save, jit_code_load, #define jit_name(u) _jit_name(_jit,u) jit_code_name, diff --git a/lib/jit_arm.c b/lib/jit_arm.c index d27d79678..4c760f845 100644 --- a/lib/jit_arm.c +++ b/lib/jit_arm.c @@ -278,7 +278,10 @@ _jit_ret(jit_state_t *_jit) void _jit_retr(jit_state_t *_jit, jit_int32_t u) { - jit_movr(JIT_RET, u); + if (JIT_RET != u) + jit_movr(JIT_RET, u); + else + jit_live(JIT_RET); jit_ret(); } @@ -295,20 +298,24 @@ _jit_retr_f(jit_state_t *_jit, jit_int32_t u) if (jit_cpu.abi) { if (u != JIT_FRET) jit_movr_f(JIT_FRET, u); + else + jit_live(JIT_FRET); + } + else { + if (u != JIT_RET) + jit_movr_f_w(JIT_RET, u); + else + jit_live(JIT_RET); } - else if (u != JIT_RET) - jit_movr_f_w(JIT_RET, u); jit_ret(); } void _jit_reti_f(jit_state_t *_jit, jit_float32_t u) { - if (jit_cpu.abi) { - if (u != JIT_FRET) - jit_movi_f(JIT_FRET, u); - } - else if (u != JIT_RET) + if (jit_cpu.abi) + jit_movi_f(JIT_FRET, u); + else jit_movi_f_w(JIT_RET, u); jit_ret(); } @@ -319,20 +326,24 @@ _jit_retr_d(jit_state_t *_jit, jit_int32_t u) if (jit_cpu.abi) { if (u != JIT_FRET) jit_movr_d(JIT_FRET, u); + else + jit_live(JIT_FRET); + } + else { + if (u != JIT_RET) + jit_movr_d_ww(JIT_RET, _R1, u); + else + jit_live(JIT_RET); } - else if (u != JIT_RET) - jit_movr_d_ww(JIT_RET, _R1, u); jit_ret(); } void _jit_reti_d(jit_state_t *_jit, jit_float64_t u) { - if (jit_cpu.abi) { - if (u != JIT_FRET) - jit_movi_d(JIT_FRET, u); - } - else if (u != JIT_RET) + if (jit_cpu.abi) + jit_movi_d(JIT_FRET, u); + else jit_movi_d_ww(JIT_RET, _R1, u); jit_ret(); } @@ -1476,6 +1487,7 @@ _emit_code(jit_state_t *_jit) else vfp_movi_d(rn(node->u.w), node->w.d); break; + case jit_code_live: case jit_code_arg: case jit_code_arg_f: case jit_code_arg_d: break; diff --git a/lib/jit_mips.c b/lib/jit_mips.c index 4c606c4bc..745623ff4 100644 --- a/lib/jit_mips.c +++ b/lib/jit_mips.c @@ -187,14 +187,20 @@ _jit_retr(jit_state_t *_jit, jit_int32_t u) void _jit_reti(jit_state_t *_jit, jit_word_t u) { - jit_movi(JIT_RET, u); + if (JIT_RET != u) + jit_movi(JIT_RET, u); + else + jit_live(JIT_RET); jit_ret(); } void _jit_retr_f(jit_state_t *_jit, jit_int32_t u) { - jit_movr_f(JIT_FRET, u); + if (JIT_FRET != u) + jit_movr_f(JIT_FRET, u); + else + jit_live(JIT_FRET); jit_ret(); } @@ -208,7 +214,10 @@ _jit_reti_f(jit_state_t *_jit, jit_float32_t u) void _jit_retr_d(jit_state_t *_jit, jit_int32_t u) { - jit_movr_d(JIT_FRET, u); + if (JIT_FRET != u) + jit_movr_d(JIT_FRET, u); + else + jit_live(JIT_FRET); jit_ret(); } @@ -1209,6 +1218,7 @@ _emit_code(jit_state_t *_jit) movi_d_ww(rn(node->u.w), rn(node->v.w), (jit_float64_t *)node->w.n->u.w); break; + case jit_code_live: case jit_code_arg: case jit_code_arg_f: case jit_code_arg_d: break; diff --git a/lib/jit_ppc.c b/lib/jit_ppc.c index 301c3fb06..d63fe7902 100644 --- a/lib/jit_ppc.c +++ b/lib/jit_ppc.c @@ -186,7 +186,10 @@ _jit_ret(jit_state_t *_jit) void _jit_retr(jit_state_t *_jit, jit_int32_t u) { - jit_movr(JIT_RET, u); + if (JIT_RET != u) + jit_movr(JIT_RET, u); + else + jit_live(JIT_RET); jit_ret(); } @@ -200,7 +203,10 @@ _jit_reti(jit_state_t *_jit, jit_word_t u) void _jit_retr_f(jit_state_t *_jit, jit_int32_t u) { - jit_movr_f(JIT_FRET, u); + if (JIT_RET != u) + jit_movr_f(JIT_FRET, u); + else + jit_live(JIT_FRET); jit_ret(); } @@ -214,7 +220,10 @@ _jit_reti_f(jit_state_t *_jit, jit_float32_t u) void _jit_retr_d(jit_state_t *_jit, jit_int32_t u) { - jit_movr_d(JIT_FRET, u); + if (JIT_FRET != u) + jit_movr_d(JIT_FRET, u); + else + jit_live(JIT_FRET); jit_ret(); } @@ -1188,6 +1197,7 @@ _emit_code(jit_state_t *_jit) epilog(node); _jit->function = NULL; break; + case jit_code_live: case jit_code_arg: case jit_code_arg_f: case jit_code_arg_d: break; diff --git a/lib/jit_print.c b/lib/jit_print.c index ee7e08d23..5cec55b6a 100644 --- a/lib/jit_print.c +++ b/lib/jit_print.c @@ -36,6 +36,7 @@ */ static char *code_name[] = { "data", + "live", "save", "load", "#name", "#note", "label", diff --git a/lib/jit_x86.c b/lib/jit_x86.c index bac645964..f33448a76 100644 --- a/lib/jit_x86.c +++ b/lib/jit_x86.c @@ -356,8 +356,12 @@ _jit_ret(jit_state_t *_jit) void _jit_retr(jit_state_t *_jit, jit_int32_t u) { + /* movr(%ret, %ret) would be optimized out */ if (JIT_RET != u) jit_movr(JIT_RET, u); + /* explicitly tell it is live */ + else + jit_live(JIT_RET); jit_ret(); } @@ -373,6 +377,8 @@ _jit_retr_f(jit_state_t *_jit, jit_int32_t u) { if (JIT_FRET != u) jit_movr_f(JIT_FRET, u); + else + jit_live(JIT_FRET); jit_ret(); } @@ -388,6 +394,8 @@ _jit_retr_d(jit_state_t *_jit, jit_int32_t u) { if (JIT_FRET != u) jit_movr_d(JIT_FRET, u); + else + jit_live(JIT_FRET); jit_ret(); } @@ -1599,6 +1607,7 @@ _emit_code(jit_state_t *_jit) fstpr(rn(node->u.w) + 1); break; #endif + case jit_code_live: case jit_code_arg: case jit_code_arg_f: case jit_code_arg_d: break; diff --git a/lib/lightning.c b/lib/lightning.c index c2ee09078..2486e51cf 100644 --- a/lib/lightning.c +++ b/lib/lightning.c @@ -809,6 +809,9 @@ _jit_classify(jit_state_t *_jit, jit_code_t code) case jit_code_prolog: case jit_code_epilog: mask = 0; break; + case jit_code_live: + mask = jit_cc_a0_reg; + break; case jit_code_arg: case jit_code_arg_f: case jit_code_arg_d: mask = jit_cc_a0_int; break;