[Guile-commits] 305/437: Consider all callee_save_p regs as live on non trackable jumps

[Andy Wingo]

wingo pushed a commit to branch lightning
in repository guile.

commit c8bb0dad56df3eba77389faca561b91bbdecdfc4
Author: pcpa <address@hidden>
Date:   Sun Sep 7 15:32:32 2014 -0300

    Consider all callee_save_p regs as live on non trackable jumps
---
 ChangeLog         | 17 ++++++++++++++
 check/Makefile.am | 13 ++++++-----
 check/jmpr.ok     |  1 +
 check/jmpr.tst    | 66 +++++++++++++++++++++++++++++++++++++++++++++++++++++++
 lib/lightning.c   | 16 +++-----------
 5 files changed, 94 insertions(+), 19 deletions(-)

diff --git a/ChangeLog b/ChangeLog
index 97c8f47..ebff1f8 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,3 +1,20 @@
+2014-09-07 Paulo Andrade <address@hidden>
+
+       * lib/lightning.c: Mark all registers advertised as live, as
+       per jit_callee_save_p as live whenever reaching a jump that
+       cannot be tracked. This is a rethink of the previous commit,
+       and is a better approach, otherwise there would not be much
+       sense on relying on jit_callee_save_p if it could not be
+       trusted.
+
+       * check/jmpr.tst, check/jmpr.ok: New files implementing a very
+       simple test case, that would actually cause an assertion on
+       code before the change to only mark as live when reaching a
+       jump that could not tracked, the actually advertised as callee
+       save registers.
+
+       check/Makefile.am: Update for new jmpr test case.
+
 2014-09-01 Paulo Andrade <address@hidden>
 
        * lib/lightning.c: Do not mark all registers in unknown state
diff --git a/check/Makefile.am b/check/Makefile.am
index c900fbd..9c6c54a 100644
--- a/check/Makefile.am
+++ b/check/Makefile.am
@@ -77,6 +77,7 @@ EXTRA_DIST =                          \
        carry.tst       carry.ok        \
        call.tst        call.ok         \
        float.tst       float.ok        \
+       jmpr.tst        jmpr.ok         \
        qalu.inc                        \
        qalu_mul.tst    qalu_mul.ok     \
        qalu_div.tst    qalu_div.ok     \
@@ -104,7 +105,7 @@ base_TESTS =                                \
        fop_abs fop_sqrt                \
        varargs stack                   \
        clobber carry call              \
-       float                           \
+       float jmpr                      \
        qalu_mul qalu_div               \
        ret
 
@@ -131,7 +132,7 @@ x87_TESTS =                                 \
        fop_abs.x87 fop_sqrt.x87                \
        varargs.x87 stack.x87                   \
        clobber.x87 carry.x87 call.x87          \
-       float.x87
+       float.x87 jmpr.x87
 $(x87_TESTS):  check.x87.sh
        $(LN_S) $(srcdir)/check.x87.sh $@
 TESTS += $(x87_TESTS)
@@ -153,7 +154,7 @@ x87_nodata_TESTS =                                          
        \
        fop_abs.x87.nodata fop_sqrt.x87.nodata                          \
        varargs.x87.nodata stack.x87.nodata                             \
        clobber.x87.nodata carry.x87.nodata call.x87.nodata             \
-       float.x87.nodata
+       float.x87.nodata jmpr.x87.nodata
 $(x87_nodata_TESTS):   check.x87.nodata.sh
        $(LN_S) $(srcdir)/check.x87.nodata.sh $@
 TESTS += $(x87_nodata_TESTS)
@@ -177,7 +178,7 @@ arm_TESTS =                                 \
        fop_abs.arm fop_sqrt.arm                \
        varargs.arm stack.arm                   \
        clobber.arm carry.arm call.arm          \
-       float.arm
+       float.arm jmpr.arm
 $(arm_TESTS):  check.arm.sh
        $(LN_S) $(srcdir)/check.arm.sh $@
 TESTS += $(arm_TESTS)
@@ -201,7 +202,7 @@ swf_TESTS =                                 \
        fop_abs.swf fop_sqrt.swf                \
        varargs.swf stack.swf                   \
        clobber.swf carry.swf call.swf          \
-       float.swf
+       float.swf jmpr.arm
 $(swf_TESTS):  check.swf.sh
        $(LN_S) $(srcdir)/check.swf.sh $@
 TESTS += $(swf_TESTS)
@@ -225,7 +226,7 @@ nodata_TESTS =                                              
\
        fop_abs.nodata fop_sqrt.nodata                  \
        varargs.nodata stack.nodata                     \
        clobber.nodata carry.nodata call.nodata         \
-       float.nodata
+       float.nodata jmpr.nodata
 $(nodata_TESTS):       check.nodata.sh
        $(LN_S) $(srcdir)/check.nodata.sh $@
 TESTS += $(nodata_TESTS)
diff --git a/check/jmpr.ok b/check/jmpr.ok
new file mode 100644
index 0000000..9766475
--- /dev/null
+++ b/check/jmpr.ok
@@ -0,0 +1 @@
+ok
diff --git a/check/jmpr.tst b/check/jmpr.tst
new file mode 100644
index 0000000..669f54e
--- /dev/null
+++ b/check/jmpr.tst
@@ -0,0 +1,66 @@
+/*
+This is a very simple check to a condition that on lightning 2.0.5
+could cause an assertion on some backends, due to correcting a problem
+with temporaries that could not be saved/reloaded due to being used only
+in the hardware instruction, or being considered live for too long on the
+lightning instruction, and that could not be reloaded after the jump target
+(or after false/true target on conditional branches).
+
+If this code in lib/lightning.c:_jit_update():
+
+                   for (regno = 0; regno < _jitc->reglen; regno++) {
+                       spec = jit_class(_rvs[regno].spec);
+                       if (jit_regset_tstbit(mask, regno) &&
+                           (spec & (jit_class_gpr|jit_class_fpr)) &&
+                           !(spec & jit_class_sav))
+                           jit_regset_clrbit(mask, regno);
+                   }
+
+were removed, this test case, on x86_64 would fail like this:
+
+lt-lightning: lightning.c:305: _jit_get_reg: Assertion `regspec & 0x02000000' 
failed.
+Aborted (core dumped)
+ */
+
+.data  32
+ret:
+#if __WORDSIZE == 32
+.i     0
+#else
+.l     0
+#endif
+ok:
+.c     "ok"
+
+.code
+       prolog
+       jmpi start
+
+add_v1_v2:
+       addr %v1 %v1 %v2
+       ldi %r0 ret
+       jmpr %r0
+
+start:
+       movi %v1 1
+       movi %v2 2
+       movi %r0 ret_add_v1_v2
+       sti ret %r0
+       movi %v0 add_v1_v2
+       jmpr %v0
+       movi_d %f0 3
+       beqi_d pass_movi_f0 %f0 3
+       calli @abort
+pass_movi_f0:
+       beqi pass_check_v2 %v2 2
+       calli @abort
+pass_check_v2:
+ret_add_v1_v2:
+       beqi pass_add_v1_v2 %v1 3
+       calli @abort
+pass_add_v1_v2:
+       prepare
+               pushargi ok
+       finishi @puts
+       ret
+       epilog
diff --git a/lib/lightning.c b/lib/lightning.c
index a8a9463..1516bd4 100644
--- a/lib/lightning.c
+++ b/lib/lightning.c
@@ -2172,19 +2172,9 @@ _jit_update(jit_state_t *_jit, jit_node_t *node,
                     * no options for "nospill" temporaries (other temporaries
                     * also benefit from not needing to spill/reload), so, the
                     * user must ensure to either spill/reload, or only leave
-                    * live registers on JIT_Vn if using a jump that cannot
-                    * be tracked around the generated jit code */
-                   for (spec = JIT_R_NUM - 1; spec >= 0; spec--) {
-                       regno = jit_r(spec);
-                       if (jit_regset_tstbit(mask, regno))
-                           jit_regset_clrbit(mask, regno);
-                   }
-                   for (spec = JIT_F_NUM - 1; spec >= 0; spec--) {
-                       regno = jit_f(spec);
-                       if (jit_regset_tstbit(mask, regno))
-                           jit_regset_clrbit(mask, regno);
-                   }
-                   /* In some backends JIT_Rn and JIT_Fn are callee save */
+                    * live values on registers that are advertised as
+                    * callee save (as per jit_callee_save_p); on most targets
+                    * these are the JIT_Vn registers. */
                    for (regno = 0; regno < _jitc->reglen; regno++) {
                        spec = jit_class(_rvs[regno].spec);
                        if (jit_regset_tstbit(mask, regno) &&