[Guile-commits] 184/437: Implement the "live" code to explicitly tell a register is live.

[Andy Wingo]

wingo pushed a commit to branch lightning
in repository guile.

commit 60c1c545fcbb74f789a77d6bdd43d1e3e3c45a9a
Author: pcpa <address@hidden>
Date:   Tue Feb 5 14:14:25 2013 -0200

    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(-)

diff --git a/ChangeLog b/ChangeLog
index 85a6855..e91a7c6 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,5 +1,22 @@
 2013-02-05 Paulo Andrade <address@hidden>
 
+       *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 <address@hidden>
+
        * lib/jit_ppc-cpu.c, lib/jit_ppc.c: Validate and correct
        problems in the qmul and qdiv ppc implementation.
 
diff --git a/TODO b/TODO
index 5fd2df3..a2d9a25 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 34fbbcf..220c99a 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 0000000..9766475
--- /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 0000000..de1d82c
--- /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 3694616..057b45a 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 d27d796..4c760f8 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 4c606c4..745623f 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 301c3fb..d63fe79 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 ee7e08d..5cec55b 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 bac6459..f33448a 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 c2ee090..2486e51 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;