From f31fb0044d6a7275c9a4bf3300b111a902f787f7 Mon Sep 17 00:00:00 2001 From: Helmut Eller Date: Wed, 8 Jun 2022 16:20:42 +0200 Subject: [PATCH] Fix some problems with callr and calli. The problem with callr is that the register that contains the function to be called, can be overwritten by the logic that moves the values into argument registers. To fix this, I added a get_callr_temp function that should return a platform specific register that is not used to pass arguments. For Aarch64/Arm the link registers seems to work; for Amd64/i686 the RAX register. The function/tmp pair becomes an additional argument to the parallel assigment; this way the original function register is not accidentally overwritten. The problem with calli is that it may not have enough temp registers to move arguments. The windmill paper says that at most one temporary register is needed for the parallel assignment. However, we also need a temp register for mem-to-mem moves. So it seems that we need a second temporary. For Amd64/i686 we have only one temporary GPR and one temporary FPR. To fix this, I modified the algorithm from the paper a bit: we perform the mem-to-mem moves before the other moves. Later when we need the temp to break cycles, there shouldn't be any mem-to-mem moves left. So we should never need two temps at the same time. * lightening/lightening.c: (get_callr_temp): New function; need for each platform. (prepare_call_args): Include the function/callr_temp pair in the arguments for the parallel assignment. * lightening/x86.c, lightening/arm.c, lightening/aarch64.c (get_callr_temp): Implementation for each platform. * lightening/arm.c (next_abi_arg): Fix the stack size for doubles. * tests/call_10_2.c, tests/callr_10.c: New tests. * tests/regarrays.inc: New file. Common code between the above two tests that would be tedious to duplicate. --- lightening/aarch64.c | 6 ++ lightening/arm.c | 8 +- lightening/lightening.c | 35 +++++-- lightening/x86.c | 6 ++ tests/call_10_2.c | 165 ++++++++++++++++++++++++++++++++ tests/callr_10.c | 66 +++++++++++++ tests/regarrays.inc | 206 ++++++++++++++++++++++++++++++++++++++++ 7 files changed, 485 insertions(+), 7 deletions(-) create mode 100644 tests/call_10_2.c create mode 100644 tests/callr_10.c create mode 100644 tests/regarrays.inc diff --git a/lightening/aarch64.c b/lightening/aarch64.c index e67365f23..fb14f3d8f 100644 --- a/lightening/aarch64.c +++ b/lightening/aarch64.c @@ -232,3 +232,9 @@ bless_function_pointer(void *ptr) { return ptr; } + +static jit_gpr_t +get_callr_temp (jit_state_t * _jit) +{ + return _LR; +} diff --git a/lightening/arm.c b/lightening/arm.c index d587e7158..11deedd89 100644 --- a/lightening/arm.c +++ b/lightening/arm.c @@ -109,7 +109,7 @@ next_abi_arg(struct abi_arg_iterator *iter, jit_operand_t *arg) } } *arg = jit_operand_mem (abi, JIT_SP, iter->stack_size); - iter->stack_size += 4; + iter->stack_size += 4 + (abi == JIT_OPERAND_ABI_DOUBLE ? 4 : 0); } static void @@ -137,3 +137,9 @@ bless_function_pointer(void *ptr) // Set low bit to mark as thumb mode. return (void*) (((uintptr_t)ptr) | 1); } + +static jit_gpr_t +get_callr_temp (jit_state_t * _jit) +{ + return _LR; +} diff --git a/lightening/lightening.c b/lightening/lightening.c index 1254514ae..afc6fd493 100644 --- a/lightening/lightening.c +++ b/lightening/lightening.c @@ -124,6 +124,8 @@ static void reset_abi_arg_iterator(struct abi_arg_iterator *iter, size_t argc, static void next_abi_arg(struct abi_arg_iterator *iter, jit_operand_t *arg); +static jit_gpr_t get_callr_temp (jit_state_t * _jit); + jit_bool_t init_jit(void) { @@ -1096,6 +1098,15 @@ jit_move_operands(jit_state_t *_jit, jit_operand_t *dst, jit_operand_t *src, enum move_status status[argc]; for (size_t i = 0; i < argc; i++) status[i] = TO_MOVE; + + // Mem-to-mem moves require a temp register but don't overwrite + // other argument registers. Perform them first to free up the tmp + // for other uses. + for (size_t i = 0; i < argc; i++) + if ((status[i] == TO_MOVE) + && (MOVE_KIND (src[i].kind, dst[i].kind) == MOVE_MEM_TO_MEM)) + move_one(_jit, dst, src, argc, status, i); + for (size_t i = 0; i < argc; i++) if (status[i] == TO_MOVE) move_one(_jit, dst, src, argc, status, i); @@ -1236,11 +1247,23 @@ jit_leave_jit_abi(jit_state_t *_jit, size_t v, size_t vf, size_t frame_size) // Precondition: stack is already aligned. static size_t -prepare_call_args(jit_state_t *_jit, size_t argc, jit_operand_t args[]) +prepare_call_args(jit_state_t *_jit, size_t argc, jit_operand_t args[], + jit_gpr_t *fun) { - jit_operand_t dst[argc]; + size_t count = argc + (fun == NULL ? 0 : 1); + jit_operand_t src[count]; + jit_operand_t dst[count]; + + memcpy (src, args, sizeof (jit_operand_t) * argc); + if (fun != NULL) { + jit_gpr_t fun_tmp = argc == 0 ? *fun : get_callr_temp (_jit); + src[argc] = jit_operand_gpr (JIT_OPERAND_ABI_POINTER, *fun); + dst[argc] = jit_operand_gpr (JIT_OPERAND_ABI_POINTER, fun_tmp); + *fun = fun_tmp; + } + struct abi_arg_iterator iter; - + // Compute shuffle destinations and space for spilled arguments. reset_abi_arg_iterator(&iter, argc, args); for (size_t i = 0; i < argc; i++) @@ -1265,7 +1288,7 @@ prepare_call_args(jit_state_t *_jit, size_t argc, jit_operand_t args[]) } } - jit_move_operands(_jit, dst, args, argc); + jit_move_operands(_jit, dst, src, count); return stack_size; } @@ -1273,7 +1296,7 @@ prepare_call_args(jit_state_t *_jit, size_t argc, jit_operand_t args[]) void jit_calli(jit_state_t *_jit, jit_pointer_t f, size_t argc, jit_operand_t args[]) { - size_t stack_bytes = prepare_call_args(_jit, argc, args); + size_t stack_bytes = prepare_call_args(_jit, argc, args, NULL); calli(_jit, (jit_word_t)f); @@ -1283,7 +1306,7 @@ jit_calli(jit_state_t *_jit, jit_pointer_t f, size_t argc, jit_operand_t args[]) void jit_callr(jit_state_t *_jit, jit_gpr_t f, size_t argc, jit_operand_t args[]) { - size_t stack_bytes = prepare_call_args(_jit, argc, args); + size_t stack_bytes = prepare_call_args(_jit, argc, args, &f); callr(_jit, jit_gpr_regno(f)); diff --git a/lightening/x86.c b/lightening/x86.c index f8ac4b0b8..873cb27a4 100644 --- a/lightening/x86.c +++ b/lightening/x86.c @@ -405,3 +405,9 @@ bless_function_pointer(void *ptr) { return ptr; } + +static jit_gpr_t +get_callr_temp (jit_state_t * _jit) +{ + return _RAX; +} diff --git a/tests/call_10_2.c b/tests/call_10_2.c new file mode 100644 index 000000000..189757876 --- /dev/null +++ b/tests/call_10_2.c @@ -0,0 +1,165 @@ +#include "test.h" +#include "regarrays.inc" + +#define DEFINE_TEST_INT(ABI_TYPE, TYPE, LIT, NEGATE) \ +static TYPE \ +check_##TYPE (TYPE a, TYPE b, TYPE c, TYPE d, TYPE e, \ + TYPE f, TYPE g, TYPE h, TYPE i, TYPE j) \ +{ \ + ASSERT(a == LIT(0)); \ + ASSERT(b == NEGATE(1)); \ + ASSERT(c == LIT(2)); \ + ASSERT(d == NEGATE(3)); \ + ASSERT(e == LIT(4)); \ + ASSERT(f == NEGATE(5)); \ + ASSERT(g == LIT(6)); \ + ASSERT(h == NEGATE(7)); \ + ASSERT(i == LIT(8)); \ + ASSERT(j == NEGATE(9)); \ + return LIT(42); \ +} \ + \ +static void \ +run_test_##TYPE (jit_state_t *j, uint8_t *arena_base, size_t arena_size, \ + jit_gpr_t base) \ +{ \ + jit_begin(j, arena_base, arena_size); \ + size_t align = jit_enter_jit_abi(j, v_count, 0, 0); \ + jit_load_args_1(j, jit_operand_gpr (JIT_OPERAND_ABI_POINTER, base)); \ + \ + jit_operand_t args[10] = { \ + jit_operand_mem(ABI_TYPE, base, 0 * sizeof(TYPE)), \ + jit_operand_mem(ABI_TYPE, base, 1 * sizeof(TYPE)), \ + jit_operand_mem(ABI_TYPE, base, 2 * sizeof(TYPE)), \ + jit_operand_mem(ABI_TYPE, base, 3 * sizeof(TYPE)), \ + jit_operand_mem(ABI_TYPE, base, 4 * sizeof(TYPE)), \ + jit_operand_mem(ABI_TYPE, base, 5 * sizeof(TYPE)), \ + jit_operand_mem(ABI_TYPE, base, 6 * sizeof(TYPE)), \ + jit_operand_mem(ABI_TYPE, base, 7 * sizeof(TYPE)), \ + jit_operand_mem(ABI_TYPE, base, 8 * sizeof(TYPE)), \ + jit_operand_mem(ABI_TYPE, base, 9 * sizeof(TYPE)), \ + }; \ + jit_calli(j, check_##TYPE, 10, args); \ + jit_leave_jit_abi(j, v_count, 0, align); \ + jit_ret(j); \ + \ + size_t size = 0; \ + void* ret = jit_end(j, &size); \ + \ + TYPE (*f)(TYPE*) = ret; \ + \ + TYPE iargs[10] = { LIT(0), NEGATE(1), LIT(2), NEGATE(3), LIT(4), \ + NEGATE(5), LIT(6), NEGATE(7), LIT(8), NEGATE(9) }; \ + ASSERT(f(iargs) == LIT(42)); \ +} + +#define LIT(X) (X) +#define NEGATE(X) (-X) +DEFINE_TEST_INT(JIT_OPERAND_ABI_INT32, int32_t, LIT, NEGATE); +#if (UINTPTR_MAX == UINT64_MAX) +DEFINE_TEST_INT(JIT_OPERAND_ABI_INT64, int64_t, LIT, NEGATE); +#endif +#undef NEGATE + +#define NEGATE(X) (~X) +DEFINE_TEST_INT(JIT_OPERAND_ABI_UINT32, uint32_t, LIT, NEGATE); +#if (UINTPTR_MAX == UINT64_MAX) +DEFINE_TEST_INT(JIT_OPERAND_ABI_UINT64, uint64_t, LIT, NEGATE); +#endif +#undef NEGATE +#undef LIT + +typedef uint8_t* ptr_t; +#define LIT(X) ((ptr_t)(uintptr_t)(X)) +#define NEGATE(X) ((ptr_t)(~(uintptr_t)(X))) +DEFINE_TEST_INT(JIT_OPERAND_ABI_POINTER, ptr_t, LIT, NEGATE); + +static double +check_double (double a, double b, double c, double d, double e, + double f, double g, double h, double i, double j) +{ + ASSERT(a == 0.0); + ASSERT(b == -1.0); + ASSERT(c == -0xfffffffffffffp+100l); + ASSERT(d == +0xfffffffffffffp-100l); + ASSERT(e == -0xfffffffffffffp+101l); + ASSERT(f == +0xfffffffffffffp-102l); + ASSERT(g == -0xfffffffffffffp+102l); + ASSERT(h == +0xfffffffffffffp-103l); + ASSERT(i == -0xfffffffffffffp+103l); + ASSERT(j == +0xfffffffffffffp-104l); + return 42; +} + +static void +run_test_double (jit_state_t *j, uint8_t *arena_base, size_t arena_size, + jit_gpr_t base) +{ + double dargs[10] = { + 0.0, + -1.0, + -0xfffffffffffffp+100l, + +0xfffffffffffffp-100l, + -0xfffffffffffffp+101l, + +0xfffffffffffffp-102l, + -0xfffffffffffffp+102l, + +0xfffffffffffffp-103l, + -0xfffffffffffffp+103l, + +0xfffffffffffffp-104l, + }; + jit_begin(j, arena_base, arena_size); + size_t align = jit_enter_jit_abi(j, v_count, 0, 0); + jit_load_args_1(j, jit_operand_gpr (JIT_OPERAND_ABI_POINTER, base)); + enum jit_operand_abi abi = JIT_OPERAND_ABI_DOUBLE; + jit_movi_d(j, JIT_F0, dargs[0]); + jit_movi_d(j, JIT_F1, dargs[1]); + jit_movi_d(j, JIT_F2, dargs[2]); + jit_movi_d(j, JIT_F3, dargs[3]); + jit_movi_d(j, JIT_F4, dargs[4]); + jit_movi_d(j, JIT_F5, dargs[5]); + jit_movi_d(j, JIT_F6, dargs[6]); + jit_operand_t args[10] = { + jit_operand_fpr(abi, JIT_F0), + jit_operand_fpr(abi, JIT_F1), + jit_operand_fpr(abi, JIT_F2), + jit_operand_fpr(abi, JIT_F3), + jit_operand_fpr(abi, JIT_F4), + jit_operand_fpr(abi, JIT_F5), + jit_operand_fpr(abi, JIT_F6), + jit_operand_mem(abi, base, 7 * sizeof(double)), + jit_operand_mem(abi, base, 8 * sizeof(double)), + jit_operand_mem(abi, base, 9 * sizeof(double)), + }; + jit_calli(j, check_double, 10, args); + jit_leave_jit_abi(j, v_count, 0, align); + jit_ret(j); + + size_t size = 0; + void* ret = jit_end(j, &size); + + double (*f)(double*) = ret; + + ASSERT(f(dargs) == 42); +} + +static void +run_test (jit_state_t * j, uint8_t * arena_base, size_t arena_size) +{ + for (unsigned i = 0; i < gpr_count; i++) + { + run_test_int32_t (j, arena_base, arena_size, gpr_ref (i)); + run_test_uint32_t (j, arena_base, arena_size, gpr_ref (i)); +#if (UINTPTR_MAX == UINT64_MAX) + run_test_int64_t (j, arena_base, arena_size, gpr_ref (i)); + run_test_uint64_t (j, arena_base, arena_size, gpr_ref (i)); +#endif + run_test_ptr_t (j, arena_base, arena_size, gpr_ref (i)); + run_test_double (j, arena_base, arena_size, gpr_ref (i)); + } +} + +int +main (int argc, char *argv[]) +{ + return main_helper(argc, argv, run_test); +} diff --git a/tests/callr_10.c b/tests/callr_10.c new file mode 100644 index 000000000..bca488c75 --- /dev/null +++ b/tests/callr_10.c @@ -0,0 +1,66 @@ +#include "test.h" +#include "regarrays.inc" + +static int32_t f(int32_t a, int32_t b, int32_t c, int32_t d, int32_t e, + int32_t f, int32_t g, int32_t h, int32_t i, int32_t j) { + ASSERT(a == 0); + ASSERT(b == 1); + ASSERT(c == 2); + ASSERT(d == 3); + ASSERT(e == 4); + ASSERT(f == 5); + ASSERT(g == 6); + ASSERT(h == 7); + ASSERT(i == 8); + ASSERT(j == 9); + return 42; +} + +static void +run_test_2 (jit_state_t *j, uint8_t *arena_base, size_t arena_size, + jit_gpr_t base, jit_gpr_t fun) +{ + jit_begin(j, arena_base, arena_size); + size_t align = jit_enter_jit_abi(j, v_count, 0, 0); + jit_load_args_1(j, jit_operand_gpr (JIT_OPERAND_ABI_POINTER, base)); + + jit_operand_t args[10] = { + jit_operand_mem(JIT_OPERAND_ABI_INT32, base, 0 * sizeof(int32_t)), + jit_operand_mem(JIT_OPERAND_ABI_INT32, base, 1 * sizeof(int32_t)), + jit_operand_mem(JIT_OPERAND_ABI_INT32, base, 2 * sizeof(int32_t)), + jit_operand_mem(JIT_OPERAND_ABI_INT32, base, 3 * sizeof(int32_t)), + jit_operand_mem(JIT_OPERAND_ABI_INT32, base, 4 * sizeof(int32_t)), + jit_operand_mem(JIT_OPERAND_ABI_INT32, base, 5 * sizeof(int32_t)), + jit_operand_mem(JIT_OPERAND_ABI_INT32, base, 6 * sizeof(int32_t)), + jit_operand_mem(JIT_OPERAND_ABI_INT32, base, 7 * sizeof(int32_t)), + jit_operand_mem(JIT_OPERAND_ABI_INT32, base, 8 * sizeof(int32_t)), + jit_operand_mem(JIT_OPERAND_ABI_INT32, base, 9 * sizeof(int32_t)) + }; + jit_movi(j, fun, (uintptr_t)f); + jit_callr(j, fun, 10, args); + jit_leave_jit_abi(j, v_count, 0, align); + jit_ret(j); + + size_t size = 0; + void* ret = jit_end(j, &size); + + int32_t (*f)(int32_t*) = ret; + + int32_t iargs[10] = { 0, 1, 2, 3, 4, 5, 6, 7, 8, 9 }; + ASSERT(f(iargs) == 42); +} + +static void +run_test (jit_state_t *jit, uint8_t *arena_base, size_t arena_size) +{ + for (unsigned i = 0; i < gpr_count; i++) + for (unsigned j = 0; j < gpr_count; j++) + if (i != j) + run_test_2 (jit, arena_base, arena_size, gpr_ref(i), gpr_ref(j)); +} + +int +main (int argc, char *argv[]) +{ + return main_helper(argc, argv, run_test); +} diff --git a/tests/regarrays.inc b/tests/regarrays.inc new file mode 100644 index 000000000..de56c905c --- /dev/null +++ b/tests/regarrays.inc @@ -0,0 +1,206 @@ +/* Arrays describing the available user registers. -*- mode: c -*- */ + +// #ifdef orgy factored out to common include file + +static const jit_gpr_t rregs[] = { + JIT_R0, + JIT_R1, + JIT_R2, +#ifdef JIT_R3 + JIT_R3, +#endif +#ifdef JIT_R4 + JIT_R4, +#endif +#ifdef JIT_R5 + JIT_R5, +#endif +#ifdef JIT_R6 + JIT_R6, +#endif +#ifdef JIT_R7 + JIT_R7, +#endif +#ifdef JIT_R8 + JIT_R8, +#endif +#ifdef JIT_R9 + JIT_R9, +#endif +#ifdef JIT_R10 + JIT_R10, +#endif +#ifdef JIT_R11 + JIT_R11, +#endif +#ifdef JIT_R12 + JIT_R12, +#endif +#ifdef JIT_R13 + JIT_R13, +#endif +#ifdef JIT_R14 + JIT_R14, +#endif +#ifdef JIT_R15 + JIT_R15, +#endif +#ifdef JIT_R16 + JIT_R16, +#endif +}; + +static const jit_gpr_t vregs[] = { + JIT_V0, JIT_V1, JIT_V2, +#ifdef JIT_V3 + JIT_V3, +#endif +#ifdef JIT_V4 + JIT_V4, +#endif +#ifdef JIT_V5 + JIT_V5, +#endif +#ifdef JIT_V6 + JIT_V6, +#endif +#ifdef JIT_V7 + JIT_V7, +#endif +#ifdef JIT_V8 + JIT_V8, +#endif +#ifdef JIT_V9 + JIT_V9, +#endif +#ifdef JIT_V10 + JIT_V10, +#endif +#ifdef JIT_V11 + JIT_V11, +#endif +#ifdef JIT_V12 + JIT_V12, +#endif +#ifdef JIT_V13 + JIT_V13, +#endif +#ifdef JIT_V14 + JIT_V14, +#endif +#ifdef JIT_V15 + JIT_V15, +#endif +#ifdef JIT_V16 + JIT_V16, +#endif +}; + +static const jit_fpr_t fregs[] = { + JIT_F0, JIT_F1, JIT_F2, + JIT_F2, JIT_F3, JIT_F4, +#ifdef JIT_F7 + JIT_F7, +#endif +#ifdef JIT_F8 + JIT_F8, +#endif +#ifdef JIT_F9 + JIT_F9, +#endif +#ifdef JIT_F10 + JIT_F10, +#endif +#ifdef JIT_F11 + JIT_F11, +#endif +#ifdef JIT_F12 + JIT_F12, +#endif +#ifdef JIT_F13 + JIT_F13, +#endif +#ifdef JIT_F14 + JIT_F14, +#endif +#ifdef JIT_F15 + JIT_F15, +#endif +#ifdef JIT_F16 + JIT_F16, +#endif +}; + +static const jit_fpr_t vfregs[] = { +#ifdef JIT_VF0 + JIT_VF0, +#endif +#ifdef JIT_VF1 + JIT_VF1, +#endif +#ifdef JIT_VF2 + JIT_VF2, +#endif +#ifdef JIT_VF2 + JIT_VF2, +#endif +#ifdef JIT_VF3 + JIT_VF3, +#endif +#ifdef JIT_VF4 + JIT_VF4, +#endif +#ifdef JIT_VF5 + JIT_VF5, +#endif +#ifdef JIT_VF6 + JIT_VF6, +#endif +#ifdef JIT_VF7 + JIT_VF7, +#endif +#ifdef JIT_VF8 + JIT_VF8, +#endif +#ifdef JIT_VF9 + JIT_VF9, +#endif +#ifdef JIT_VF10 + JIT_VF10, +#endif +#ifdef JIT_VF11 + JIT_VF11, +#endif +#ifdef JIT_VF12 + JIT_VF12, +#endif +#ifdef JIT_VF13 + JIT_VF13, +#endif +#ifdef JIT_VF14 + JIT_VF14, +#endif +#ifdef JIT_VF15 + JIT_VF15, +#endif +#ifdef JIT_VF16 + JIT_VF16, +#endif +}; + +#define ARRAY_SIZE(X) (sizeof (X)/sizeof ((X)[0])) +static const size_t r_count = ARRAY_SIZE (rregs); +static const size_t v_count = ARRAY_SIZE (vregs); +static const size_t f_count = ARRAY_SIZE (fregs); +static const size_t vf_count = ARRAY_SIZE (vfregs); +static const size_t gpr_count = r_count + v_count; + +static jit_gpr_t +gpr_ref (uintptr_t i) +{ + if (i < r_count) + return rregs[i]; + if (i < r_count + v_count) + return vregs[i - r_count]; + abort (); +}