From 60c1c545fcbb74f789a77d6bdd43d1e3e3c45a9a Mon Sep 17 00:00:00 2001 From: pcpa Date: Tue, 5 Feb 2013 14:14:25 -0200 Subject: [PATCH] Implement the "live" code to explicitly tell a register is live. *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. --- ChangeLog | 17 +++++++++++++++ TODO | 10 --------- check/Makefile.am | 4 +++- check/ret.ok | 1 + check/ret.tst | 51 +++++++++++++++++++++++++++++++++++++++++++++ include/lightning.h | 2 ++ lib/jit_arm.c | 42 ++++++++++++++++++++++++------------- lib/jit_mips.c | 16 +++++++++++--- lib/jit_ppc.c | 16 +++++++++++--- lib/jit_print.c | 1 + lib/jit_x86.c | 9 ++++++++ lib/lightning.c | 3 +++ 12 files changed, 140 insertions(+), 32 deletions(-) create mode 100644 check/ret.ok create mode 100644 check/ret.tst 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;