From b2282e6f8e303d74b18386f71f9c27d75b37debd Mon Sep 17 00:00:00 2001 From: Aleksey Demakov Date: Sat, 3 Jun 2006 13:39:53 +0000 Subject: [PATCH] fix problem with push float freeing wrong register; fix problem with restarted compilation getting constants as already loaded. allow long pairs to use EBX even if it is used as a global register. --- ChangeLog | 34 ++++++++ jit/jit-function.c | 5 +- jit/jit-reg-alloc.c | 184 ++++++++++++++++++++++++++++++++++++--- jit/jit-reg-alloc.h | 4 + jit/jit-rules-x86.ins | 10 +-- tools/gen-rules-parser.y | 150 ++++++++++++++++--------------- 6 files changed, 292 insertions(+), 95 deletions(-) diff --git a/ChangeLog b/ChangeLog index 760f54d..2689d99 100644 --- a/ChangeLog +++ b/ChangeLog @@ -1,3 +1,37 @@ +2006-06-03 avd + + * jit/jit-rules-x86.ins: remove _jit_regs_free_reg() call from + JIT_OP_RETURN_FLOAT32, JIT_OP_PUSH_FLOAT32, JIT_OP_RETURN_FLOAT64, + JIT_OP_RETURN_NFLOAT, JIT_OP_PUSH_FLOAT64, and JIT_OP_PUSH_NFLOAT + rules. With new ``gen-rules'' tool it frees wrong register and + the right register is anyway freed automatically by the new + allocator. + + * jit/jit-reg-alloc.h, jit/jit-reg-alloc.c (_jit_regs_abort): add + function to unbind values from registers if the code generation + failed. This is required because new allocator may temporary bind + constants to registers and if neither _jit_regs_commit() nor + _jit_regs_abort() is called the constant value will be left in + inconsistent state. The problem shows up when the end of a cache + block has been reached and the compilation is restarted with a new + block. + + * jit/jit-reg-alloc.c (_jit_regs_begin, _jit_regs_end): add + convenience functions. + + * tools/gen-rules-parser.y (gensel_output_clauses): make generated + code more readable by using _jit_regs_begin() and _jit_regs_end() + where appropriate. + + * jit/jit-reg-alloc.c: allow to load the second part of a register + pair to a global register. This is because x86 long arithmetics + use 4 registers at once: EAX:EDX and ECX:EBX. At the same time + EBX is the global register candidate. If the instruction clobbers + the global register, the new allocator automatically pushes it on + the stack before and pops after the instruction. + + * jit/jit-function.c (compile_block): modify trace messages. + 2006-05-28 Klaus Treichel * jit/jit-insn.c: Add a code_label in initialize_setjmp_block just diff --git a/jit/jit-function.c b/jit/jit-function.c index d2f354c..0876f2f 100644 --- a/jit/jit-function.c +++ b/jit/jit-function.c @@ -456,7 +456,8 @@ static void compile_block(jit_gencode_t gen, jit_function_t func, #ifdef _JIT_COMPILE_DEBUG unsigned char *p1, *p2; p1 = gen->posn.ptr; - printf("Insn %5d: 0x%04x - ", func->builder->insn_count++, insn->opcode); + printf("Insn: %5d, Opcode: 0x%04x\n", func->builder->insn_count++, insn->opcode); + printf("Start of binary code: 0x%08x\n", p1); #endif switch(insn->opcode) @@ -553,7 +554,7 @@ static void compile_block(jit_gencode_t gen, jit_function_t func, #ifdef _JIT_COMPILE_DEBUG p2 = gen->posn.ptr; - printf("%d\n", p2 - p1); + printf("Length of binary code: %d\n\n", p2 - p1); #endif } } diff --git a/jit/jit-reg-alloc.c b/jit/jit-reg-alloc.c index 09dfaa4..3bb167e 100644 --- a/jit/jit-reg-alloc.c +++ b/jit/jit-reg-alloc.c @@ -344,7 +344,15 @@ static void spill_all_stack(jit_gencode_t gen, int reg) @*/ void _jit_regs_spill_all(jit_gencode_t gen) { +#ifdef JIT_REG_DEBUG + printf("enter _jit_regs_spill_all\n"); +#endif + spill_all_between(gen, 0, JIT_NUM_REGS - 1); + +#ifdef JIT_REG_DEBUG + printf("leave _jit_regs_spill_all\n"); +#endif } /* @@ -636,7 +644,15 @@ int _jit_regs_want_reg(jit_gencode_t gen, int reg, int for_long) @*/ void _jit_regs_free_reg(jit_gencode_t gen, int reg, int value_used) { +#ifdef JIT_REG_DEBUG + printf("enter _jit_regs_free_reg(reg = %d, value_used = %d)\n", reg, value_used); +#endif + free_reg_and_spill(gen, reg, value_used, 0); + +#ifdef JIT_REG_DEBUG + printf("leave _jit_regs_free_reg\n"); +#endif } /*@ @@ -1646,6 +1662,9 @@ void _jit_regs_get_reg_pair(jit_gencode_t gen, int not_this1, int not_this2, #define COST_SPILL_DIRTY_GLOBAL 2 #define COST_SPILL_CLEAN 1 #define COST_SPILL_CLEAN_GLOBAL 1 +#define COST_CLOBBER_GLOBAL 1000 + +#define ALLOW_CLOBBER_GLOBAL 1 /* Value usage flags. */ #define VALUE_INPUT 1 @@ -1831,6 +1850,10 @@ is_register_alive(jit_gencode_t gen, _jit_regs_t *regs, int reg) if(reg >= 0) { + if(jit_reg_is_used(gen->permanent, reg)) + { + return 1; + } for(index = 0; index < gen->contents[reg].num_values; index++) { usage = value_usage(regs, gen->contents[reg].values[index]); @@ -2083,12 +2106,31 @@ collect_register_info(jit_gencode_t gen, _jit_regs_t *regs, int index) return 1; } + if(desc->value->in_register) + { + reg = desc->value->reg; + if(gen->contents[reg].is_long_start) + { + other_reg = OTHER_REG(reg); + } + else + { + other_reg = -1; + } + } + else + { + reg = -1; + other_reg = -1; + } + if(index > 0 || regs->ternary) { + /* See if the value needs to be loaded or copied or none. */ if(desc->value->has_global_register) { if(desc->value->global_reg != desc->reg - && !(desc->value->in_register && desc->value->reg == desc->reg)) + && !(desc->value->in_register && reg == desc->reg)) { desc->copy = 1; } @@ -2099,28 +2141,21 @@ collect_register_info(jit_gencode_t gen, _jit_regs_t *regs, int index) { desc->load = 1; } - else if(desc->value->reg != desc->reg) + else if(reg != desc->reg) { desc->copy = 1; } } + /* See if the input value needs to be saved before the + instruction and if it stays or not in the register + after the instruction. */ if(desc->value->is_constant) { desc->kill = 1; } else if(desc->value->in_register) { - reg = desc->value->reg; - if(gen->contents[reg].is_long_start) - { - other_reg = OTHER_REG(reg); - } - else - { - other_reg = -1; - } - if(desc->used) { if(jit_reg_is_used(regs->clobber, reg) @@ -2158,6 +2193,16 @@ collect_register_info(jit_gencode_t gen, _jit_regs_t *regs, int index) } } + /* See if the value clobbers a global register. In this case the global + register is pushed onto stack before the instruction and popped back + after it. */ + if((!desc->value->has_global_register || desc->value->global_reg != desc->reg) + && (jit_reg_is_used(gen->permanent, desc->reg) + || (desc->other_reg >= 0 && jit_reg_is_used(gen->permanent, desc->other_reg)))) + { + desc->kill = 1; + } + if(IS_STACK_REG(desc->reg)) { stack_start = get_stack_start(desc->reg); @@ -2518,10 +2563,12 @@ use_cheapest_register(jit_gencode_t gen, _jit_regs_t *regs, int index, jit_regus { continue; } +#if !ALLOW_CLOBBER_GLOBAL if(jit_reg_is_used(gen->permanent, other_reg)) { continue; } +#endif } else { @@ -2561,6 +2608,12 @@ use_cheapest_register(jit_gencode_t gen, _jit_regs_t *regs, int index, jit_regus copy_cost = 0; cost = compute_spill_cost(gen, regs, reg, other_reg); } +#if ALLOW_CLOBBER_GLOBAL + if(other_reg >= 0 && jit_reg_is_used(gen->permanent, other_reg)) + { + cost += COST_CLOBBER_GLOBAL; + } +#endif } #if COST_COPY != 1 @@ -3511,6 +3564,46 @@ move_input_value(jit_gencode_t gen, _jit_regs_t *regs, int index) } } +static void +abort_input_value(jit_gencode_t gen, _jit_regs_t *regs, int index) +{ + _jit_regdesc_t *desc; + int reg, other_reg; + +#ifdef JIT_REG_DEBUG + printf("abort_input_value(%d)\n", index); +#endif + + desc = ®s->descs[index]; + if(!desc->value || desc->duplicate) + { + return; + } + + if(desc->load && desc->value->in_register) + { + reg = desc->value->reg; + if(gen->contents[reg].is_long_start) + { + other_reg = OTHER_REG(reg); + } + else + { + other_reg = -1; + } + + if(IS_STACK_REG(reg)) + { + unbind_value(gen, desc->value, reg, -1); + remap_stack_down(gen, regs->stack_start, reg); + } + else + { + unbind_value(gen, desc->value, reg, other_reg); + } + } +} + static void commit_input_value(jit_gencode_t gen, _jit_regs_t *regs, int index) { @@ -3610,6 +3703,10 @@ commit_output_value(jit_gencode_t gen, _jit_regs_t *regs) free_value(gen, desc->value, desc->reg, desc->other_reg); } } + else if(desc->kill) + { + save_value(gen, desc->value, desc->reg, desc->other_reg, 1); + } #ifdef JIT_REG_DEBUG printf("value = "); @@ -4040,13 +4137,16 @@ _jit_regs_gen(jit_gencode_t gen, _jit_regs_t *regs) if(jit_reg_is_used(gen->permanent, reg)) { /* Oops, global register. */ +#ifdef JIT_REG_DEBUG + printf("*** Spill global register: %d ***\n", reg); +#endif if(regs->branch) { /* After the branch is taken there is no way to load global register back. */ return 0; } - _jit_gen_spill_global(gen, reg, gen->contents[reg].values[0]); + _jit_gen_spill_global(gen, reg, 0); continue; } @@ -4213,7 +4313,7 @@ _jit_regs_commit(jit_gencode_t gen, _jit_regs_t *regs) { if(jit_reg_is_used(regs->spill, reg) && jit_reg_is_used(gen->permanent, reg)) { - _jit_gen_load_global(gen, reg, gen->contents[reg].values[0]); + _jit_gen_load_global(gen, reg, 0); } } @@ -4222,6 +4322,62 @@ _jit_regs_commit(jit_gencode_t gen, _jit_regs_t *regs) #endif } +void +_jit_regs_abort(jit_gencode_t gen, _jit_regs_t *regs) +{ + if(regs->ternary) + { + abort_input_value(gen, regs, 0); + } + abort_input_value(gen, regs, 1); + abort_input_value(gen, regs, 2); +} + +unsigned char * +_jit_regs_inst_ptr(jit_gencode_t gen, int space) +{ + unsigned char *inst; + + inst = (unsigned char *)(gen->posn.ptr); + if(!jit_cache_check_for_n(&(gen->posn), space)) + { + jit_cache_mark_full(&(gen->posn)); + return 0; + } + + return inst; +} + +unsigned char * +_jit_regs_begin(jit_gencode_t gen, _jit_regs_t *regs, int space) +{ + unsigned char *inst; + + if(!_jit_regs_assign(gen, regs)) + { + return 0; + } + if(!_jit_regs_gen(gen, regs)) + { + return 0; + } + + inst = _jit_regs_inst_ptr(gen, space); + if(!inst) + { + _jit_regs_abort(gen, regs); + } + + return inst; +} + +void +_jit_regs_end(jit_gencode_t gen, _jit_regs_t *regs, unsigned char *inst) +{ + gen->posn.ptr = inst; + _jit_regs_commit(gen, regs); +} + /*@ * @deftypefun void _jit_regs_lookup (char *name) * Get register by name. diff --git a/jit/jit-reg-alloc.h b/jit/jit-reg-alloc.h index 366b1c0..ffb9460 100644 --- a/jit/jit-reg-alloc.h +++ b/jit/jit-reg-alloc.h @@ -186,6 +186,10 @@ int _jit_regs_assign(jit_gencode_t gen, _jit_regs_t *regs); int _jit_regs_gen(jit_gencode_t gen, _jit_regs_t *regs); int _jit_regs_select(_jit_regs_t *regs); void _jit_regs_commit(jit_gencode_t gen, _jit_regs_t *regs); +void _jit_regs_abort(jit_gencode_t gen, _jit_regs_t *regs); +unsigned char *_jit_regs_inst_ptr(jit_gencode_t gen, int space); +unsigned char *_jit_regs_begin(jit_gencode_t gen, _jit_regs_t *regs, int space); +void _jit_regs_end(jit_gencode_t gen, _jit_regs_t *regs, unsigned char *inst); int _jit_regs_dest(_jit_regs_t *regs); int _jit_regs_value1(_jit_regs_t *regs); diff --git a/jit/jit-rules-x86.ins b/jit/jit-rules-x86.ins index 22ad810..db6df43 100644 --- a/jit/jit-rules-x86.ins +++ b/jit/jit-rules-x86.ins @@ -1707,19 +1707,16 @@ JIT_OP_RETURN_LONG: unary_branch JIT_OP_RETURN_FLOAT32: unary_note, stack, only [freg] -> { - _jit_regs_free_reg(gen, reg, 1); inst = jump_to_epilog(gen, inst, block); } JIT_OP_RETURN_FLOAT64: unary_note, stack, only [freg] -> { - _jit_regs_free_reg(gen, reg, 1); inst = jump_to_epilog(gen, inst, block); } JIT_OP_RETURN_NFLOAT: unary_note, stack, only [freg] -> { - _jit_regs_free_reg(gen, reg, 1); inst = jump_to_epilog(gen, inst, block); } @@ -2135,10 +2132,9 @@ JIT_OP_PUSH_FLOAT32: unary_note, stack x86_push_membase(inst, X86_EBP, $1); gen->stack_changed = 1; } - [reg] -> { + [freg] -> { x86_alu_reg_imm(inst, X86_SUB, X86_ESP, sizeof(jit_float32)); x86_fst_membase(inst, X86_ESP, 0, 0, 1); - _jit_regs_free_reg(gen, reg, 1); gen->stack_changed = 1; } @@ -2154,10 +2150,9 @@ JIT_OP_PUSH_FLOAT64: unary_note, stack x86_push_membase(inst, X86_EBP, $1); gen->stack_changed = 1; } - [reg] -> { + [freg] -> { x86_alu_reg_imm(inst, X86_SUB, X86_ESP, sizeof(jit_float64)); x86_fst_membase(inst, X86_ESP, 0, 1, 1); - _jit_regs_free_reg(gen, reg, 1); gen->stack_changed = 1; } @@ -2192,7 +2187,6 @@ JIT_OP_PUSH_NFLOAT: unary_note, stack x86_alu_reg_imm(inst, X86_SUB, X86_ESP, sizeof(jit_float64)); x86_fst_membase(inst, X86_ESP, 0, 1, 1); } - _jit_regs_free_reg(gen, reg, 1); gen->stack_changed = 1; } diff --git a/tools/gen-rules-parser.y b/tools/gen-rules-parser.y index 9389e98..c280b03 100644 --- a/tools/gen-rules-parser.y +++ b/tools/gen-rules-parser.y @@ -880,65 +880,6 @@ gensel_output_clause_code( gensel_output_code(clause->pattern, clause->code, names, other_names, 0); } -/* - * Output a single clause for a rule. - */ -static void gensel_output_clause( - gensel_clause_t clause, - char *names[MAX_PATTERN], - char *other_names[MAX_PATTERN], - gensel_option_t options) -{ - gensel_option_t space, more_space; - - /* Cache the instruction pointer into "inst" */ - if(gensel_new_inst_type) - { - printf("\t\tjit_gen_load_inst_ptr(gen, inst);\n"); - } - else - { - space = gensel_search_option(clause->pattern, GENSEL_PATT_SPACE); - more_space = gensel_search_option(options, GENSEL_OPT_MORE_SPACE); - - printf("\t\tinst = (%s)(gen->posn.ptr);\n", gensel_inst_type); - printf("\t\tif(!jit_cache_check_for_n(&(gen->posn), "); - if(space && space->values && space->values->value) - { - printf("("); - gensel_output_code( - clause->pattern, - space->values->value, - names, other_names, 1); - printf(")"); - } - else - { - printf("%d", ((more_space == 0) - ? gensel_reserve_space - : gensel_reserve_more_space)); - } - printf("))\n"); - printf("\t\t{\n"); - printf("\t\t\tjit_cache_mark_full(&(gen->posn));\n"); - printf("\t\t\treturn;\n"); - printf("\t\t}\n"); - } - - /* Output the clause code */ - gensel_output_clause_code(clause, names, other_names); - - /* Copy "inst" back into the generation context */ - if(gensel_new_inst_type) - { - printf("\t\tjit_gen_save_inst_ptr(gen, inst);\n"); - } - else - { - printf("\t\tgen->posn.ptr = (unsigned char *)inst;\n"); - } -} - /* * Output the clauses for a rule. */ @@ -949,6 +890,7 @@ static void gensel_output_clauses(gensel_clause_t clauses, gensel_option_t optio char *other_names[MAX_PATTERN]; gensel_clause_t clause; gensel_option_t pattern; + gensel_option_t space, more_space; gensel_value_t values, child; int first, seen_option; int regs, imms, locals, scratch, index; @@ -1461,17 +1403,70 @@ static void gensel_output_clauses(gensel_clause_t clauses, gensel_option_t optio pattern = pattern->next; } - if(contains_registers) + if(gensel_new_inst_type) { - printf("\t\tif(!_jit_regs_assign(gen, ®s))\n"); - printf("\t\t{\n"); - printf("\t\t\treturn;\n"); - printf("\t\t}\n"); - printf("\t\tif(!_jit_regs_gen(gen, ®s))\n"); - printf("\t\t{\n"); - printf("\t\t\treturn;\n"); - printf("\t\t}\n"); + if(contains_registers) + { + printf("\t\tif(!_jit_regs_assign(gen, ®s))\n"); + printf("\t\t{\n"); + printf("\t\t\treturn;\n"); + printf("\t\t}\n"); + printf("\t\tif(!_jit_regs_gen(gen, ®s))\n"); + printf("\t\t{\n"); + printf("\t\t\treturn;\n"); + printf("\t\t}\n"); + } + printf("\t\tjit_gen_load_inst_ptr(gen, inst);\n"); } + else + { + space = gensel_search_option(clause->pattern, GENSEL_PATT_SPACE); + more_space = gensel_search_option(options, GENSEL_OPT_MORE_SPACE); + + if(contains_registers) + { + printf("\t\tif(!(inst = (%s)_jit_regs_begin(gen, ®s, ", gensel_inst_type); + } + else + { + printf("\t\tinst = (%s)(gen->posn.ptr);\n", gensel_inst_type); + printf("\t\tif(!jit_cache_check_for_n(&(gen->posn), "); + } + if(space && space->values && space->values->value) + { + printf("("); + gensel_build_imm_arg_index( + clause->pattern, MAX_PATTERN, + names, other_names, ternary, free_dest); + gensel_output_code( + clause->pattern, + space->values->value, + names, other_names, 1); + printf(")"); + } + else + { + printf("%d", ((more_space == 0) + ? gensel_reserve_space + : gensel_reserve_more_space)); + } + if(contains_registers) + { + printf(")))\n"); + printf("\t\t{\n"); + printf("\t\t\treturn;\n"); + printf("\t\t}\n"); + } + else + { + printf("))\n"); + printf("\t\t{\n"); + printf("\t\t\tjit_cache_mark_full(&(gen->posn));\n"); + printf("\t\t\treturn;\n"); + printf("\t\t}\n"); + } + } + regs = 0; imms = 0; @@ -1543,11 +1538,24 @@ static void gensel_output_clauses(gensel_clause_t clauses, gensel_option_t optio } gensel_build_var_index(clause->pattern, names, other_names); - gensel_output_clause(clause, names, other_names, options); + gensel_output_clause_code(clause, names, other_names); - if(contains_registers) + /* Copy "inst" back into the generation context */ + if(gensel_new_inst_type) + { + printf("\t\tjit_gen_save_inst_ptr(gen, inst);\n"); + if(contains_registers) + { + printf("\t\t_jit_regs_commit(gen, ®s);\n"); + } + } + else if(contains_registers) + { + printf("\t\t_jit_regs_end(gen, ®s, (unsigned char *)inst);\n"); + } + else { - printf("\t\t_jit_regs_commit(gen, ®s);\n"); + printf("\t\tgen->posn.ptr = (unsigned char *)inst;\n"); } printf("\t}\n"); -- 2.47.3