Fix some more bugs with linear_scan

We should not account uses for non-register references. I think there is still a bug with register releasing. Because the MVM_JIT_ARCH macro trickery relies on the symbol naem of the macro, and because we also need to distinguish between possible values, it's necessary to #define MVM_JIT_ARCH_X64 etc. and later #undef them. I'm looking into a more regular way to solve this.
MoarVM · Dec 30, 2016 · 11ead5e · vendethiel · Dec 30, 2016 · 11ead5e
1 parent 842d5a7
commit 11ead5e
Show file tree

Hide file tree

Showing 3 changed files with 48 additions and 22 deletions.
diff --git a/src/jit/internal.h b/src/jit/internal.h
@@ -57,6 +57,23 @@ void MVM_jit_emit_copy(MVMThreadContext *tc, MVMJitCompiler *compiler, MVMint32
 void MVM_jit_emit_stack_arg(MVMThreadContext *tc, MVMJitCompiler *compiler, MVMint32 stack_pos,
                             MVMint32 reg_cls, MVMint8 reg_num, MVMint32 size);
 void MVM_jit_emit_marker(MVMThreadContext *tc, MVMJitCompiler *compiler, MVMint32 num);
+
+/* Although we use these only symbolically, we need to assign a temporary value
+ * in order to to distinguish between these */
+#define MVM_JIT_ARCH_X64 1
+#define MVM_JIT_PLATFORM_POSIX 1
+#define MVM_JIT_PLATFORM_WIN32 2
+
 #if MVM_JIT_ARCH == MVM_JIT_ARCH_X64
-#include "jit/x64/arch.h"
+#define MVM_JIT_ARCH_H "jit/x64/arch.h"
 #endif
+
+#undef MVM_JIT_ARCH_X64
+/* Depends on values of MVM_JIT_PLATFORM, so need to be defined, but uses the
+ * MVM_JIT_ARCH names literally, so these need to be undefined. */
+#ifdef MVM_JIT_ARCH_H
+#include MVM_JIT_ARCH_H
+#endif
+
+#undef MVM_JIT_PLATFORM_POSIX
+#undef MVM_JIT_PLATFORM_WIN32
diff --git a/src/jit/linear_scan.c b/src/jit/linear_scan.c
@@ -222,10 +222,14 @@ static void determine_live_ranges(MVMThreadContext *tc, RegisterAllocator *alc,
             num_def++;
             num_live_range++;
         }
-        num_use += tile->num_refs;
+
         for (j = 0; j < tile->num_refs; j++) {
             /* account its use */
-            value_set_find(alc->sets, tile->refs[j])->num_uses++;
+            MVMint8 register_spec = MVM_JIT_REGISTER_FETCH(tile->register_spec, j+1);
+            if (MVM_JIT_REGISTER_IS_USED(register_spec)) {
+                value_set_find(alc->sets, tile->refs[j])->num_uses++;
+                num_use++;
+            }
         }
 
         /* I don't think we have inserted things before that actually refer to
@@ -280,8 +284,10 @@ static void determine_live_ranges(MVMThreadContext *tc, RegisterAllocator *alc,
             UnionFind *use_set   = value_set_find(alc->sets, tile->refs[j]);
             LiveRange *use_range = &alc->values[use_set->live_range_idx];
             MVMint8 register_spec = MVM_JIT_REGISTER_FETCH(tile->register_spec, j+1);
-            ValueRef use = { i, j + 1 };
-            use_range->uses[use_range->num_uses++] = use;
+            if (MVM_JIT_REGISTER_IS_USED(register_spec)) {
+                ValueRef use = { i, j + 1 };
+                use_range->uses[use_range->num_uses++] = use;
+            }
             if (MVM_JIT_REGISTER_HAS_REQUIREMENT(register_spec)) {
                 /* TODO - this may require resolving conflicting register
                  * specifications */
@@ -457,14 +463,19 @@ static void linear_scan(MVMThreadContext *tc, RegisterAllocator *alc, MVMJitTile
 
 void MVM_jit_linear_scan_allocate(MVMThreadContext *tc, MVMJitCompiler *compiler, MVMJitTileList *list) {
     RegisterAllocator alc;
-    MVMint32 i, j;
+    MVMint32 i;
     /* initialize allocator */
-
+    DO_FOO();
     alc.is_nvr = 0;
     for (i = 0; i < sizeof(non_volatile_registers); i++) {
         alc.is_nvr |= (1 << non_volatile_registers[i]);
     }
 
+    alc.active_top = 0;
+    memset(alc.active, -1, sizeof(alc.active));
+
+    alc.spill_top = 0;
+
     alc.reg_give = alc.reg_take = 0;
     memcpy(alc.reg_ring, available_registers,
            sizeof(available_registers));

diff --git a/src/jit/x64/arch.h b/src/jit/x64/arch.h
@@ -27,7 +27,7 @@
     _(XMM6), \
     _(XMM7)
 
-
+/* declare enums */
 
 enum {
 X64_GPR(MVM_JIT_REG)
@@ -53,27 +53,16 @@ X64_SSE(MVM_JIT_REG)
     _(R11)
 
 /* define set of non-volatile regsiters */
-#if MVM_JIT_PLATFORM == MVM_JIT_PLATFORM_WIN32
+
+
 #define X64_NVR(_) \
     _(RBX), \
-    _(RBP), \
     _(RSP), \
-    _(RSI), \
-    _(RDI), \
-    _(R12), \
-    _(R13), \
-    _(R14), \
-    _(R15)
-#else
-#define X64_NVR(_) \
-    _(RBX), \
     _(RBP), \
-    _(RSP), \
     _(R12), \
     _(R13), \
     _(R14), \
     _(R15)
-#endif
 
 /* GPR used for arguments */
 #define X64_ARG_GPR(_) \
@@ -89,7 +78,6 @@ X64_SSE(MVM_JIT_REG)
 #define X64_ARG_SSE(_) \
     X64_SSE(_)
 
-
 #else
 
 /* Microsoft why you give us so few registers :-( */
@@ -101,6 +89,16 @@ X64_SSE(MVM_JIT_REG)
     _(R9), \
     _(R10), \
     _(R11)
+#define X64_NVR(_) \
+    _(RBX), \
+    _(RSP), \
+    _(RBP), \
+    _(RSI), \
+    _(RDI), \
+    _(R12), \
+    _(R13), \
+    _(R14), \
+    _(R15)
 #define X64_ARG_GPR(_) \
     _(RCX), \
     _(RDX), \