From e002775c4a54a8d9ba82d768168f57dcd86a81b3 Mon Sep 17 00:00:00 2001 From: Aleksey Demakov Date: Sun, 10 Jun 2007 16:26:22 +0000 Subject: [PATCH] disable copy propagation if JIT_OP_COPY_INT is applied to bytes and shorts; some tweaks for better code generation of copy instructions --- ChangeLog | 13 ++++++ jit/jit-live.c | 85 +++++++++++++++++++++++++--------------- jit/jit-rules-x86.ins | 41 ++++++++----------- tools/gen-rules-parser.y | 32 ++++++++++++++- 4 files changed, 114 insertions(+), 57 deletions(-) diff --git a/ChangeLog b/ChangeLog index e706a05..54941cd 100644 --- a/ChangeLog +++ b/ChangeLog @@ -1,3 +1,16 @@ +2007-06-10 Aleksey Demakov + + * tools/gen-rules-parser.y (gensel_output_clauses): alter selection + logic so that "reg" rules are chosen over "local" in case the value + is used again in the same basic block. + + * jit/jit-rules-x86.ins: tweak COPY rules for bytes, shorts, and + ints. + + * jit/jit-live.c (forward_propagation, backward_propagation) + (is_copy_insn): do not perform copy propagation if JIT_OP_COPY_INT + is used for byte and short values. + 2007-05-28 Aleksey Demakov * jit/jit-live.c (forward_propagation, backward_propagation): do not diff --git a/jit/jit-live.c b/jit/jit-live.c index faa7666..80ee1fd 100644 --- a/jit/jit-live.c +++ b/jit/jit-live.c @@ -170,19 +170,55 @@ compute_liveness_for_block(jit_block_t block) } #if defined(USE_FORWARD_PROPAGATION) || defined(USE_BACKWARD_PROPAGATION) +/* + * Check if the instruction is eligible for copy propagation. + */ static int -is_copy_opcode(int opcode) +is_copy_insn(jit_insn_t insn) { - switch(opcode) + jit_type_t dtype; + jit_type_t vtype; + + if (!insn || !insn->dest || !insn->value1) + { + return 0; + } + + switch(insn->opcode) { case JIT_OP_COPY_INT: + /* Currently JIT_INSN_COPY_INT is used not only for int-to-int + copying but for byte-to-int and short-to-int copying too + (see jit_insn_convert). Propagation of byte and short values + to instructions that expect ints might confuse them. */ + dtype = jit_type_normalize(insn->dest->type); + vtype = jit_type_normalize(insn->value1->type); + if(dtype != vtype) + { + /* signed/unsigned int conversion should be safe */ + if((dtype->kind == JIT_TYPE_INT || dtype->kind == JIT_TYPE_UINT) + && (vtype->kind == JIT_TYPE_INT || vtype->kind == JIT_TYPE_UINT)) + { + return 1; + } + return 0; + } + return 1; + + case JIT_OP_COPY_LOAD_SBYTE: + case JIT_OP_COPY_LOAD_UBYTE: + case JIT_OP_COPY_LOAD_SHORT: + case JIT_OP_COPY_LOAD_USHORT: case JIT_OP_COPY_LONG: case JIT_OP_COPY_FLOAT32: case JIT_OP_COPY_FLOAT64: case JIT_OP_COPY_NFLOAT: - /*case JIT_OP_STRUCT:*/ + case JIT_OP_COPY_STRUCT: + case JIT_OP_COPY_STORE_BYTE: + case JIT_OP_COPY_STORE_SHORT: return 1; } + return 0; } #endif @@ -226,22 +262,13 @@ forward_propagation(jit_block_t block) jit_insn_iter_init(&iter, block); while((insn = jit_insn_iter_next(&iter)) != 0) { - if(!is_copy_opcode(insn->opcode)) + if(!is_copy_insn(insn)) { continue; } dest = insn->dest; - if(dest == 0 || dest->is_constant) - { - continue; - } - value = insn->value1; - if(value == 0) - { - continue; - } /* Discard copy to itself */ if(dest == value) @@ -267,6 +294,10 @@ forward_propagation(jit_block_t block) { continue; } + if(value->is_addressable || value->is_volatile) + { + continue; + } iter2 = iter; while((insn2 = jit_insn_iter_next(&iter2)) != 0) @@ -378,30 +409,13 @@ backward_propagation(jit_block_t block) jit_insn_iter_init_last(&iter, block); while((insn = jit_insn_iter_previous(&iter)) != 0) { - if(!is_copy_opcode(insn->opcode)) + if(!is_copy_insn(insn)) { continue; } dest = insn->dest; - if(dest == 0 || dest->is_constant) - { - continue; - } - if(dest->is_addressable || dest->is_volatile) - { - continue; - } - value = insn->value1; - if(value == 0) - { - continue; - } - if(value->is_addressable || value->is_volatile) - { - continue; - } /* Discard copy to itself */ if(dest == value) @@ -422,6 +436,15 @@ backward_propagation(jit_block_t block) continue; } + if(dest->is_addressable || dest->is_volatile) + { + continue; + } + if(value->is_addressable || value->is_volatile) + { + continue; + } + iter2 = iter; while((insn2 = jit_insn_iter_previous(&iter2)) != 0) { diff --git a/jit/jit-rules-x86.ins b/jit/jit-rules-x86.ins index 2f80e6f..8e84119 100644 --- a/jit/jit-rules-x86.ins +++ b/jit/jit-rules-x86.ins @@ -1863,19 +1863,28 @@ JIT_OP_ADDRESS_OF_LABEL: * Data manipulation. */ -JIT_OP_COPY_LOAD_SBYTE: copy - [reg] -> {} - -JIT_OP_COPY_LOAD_UBYTE: copy - [reg] -> {} - -JIT_OP_COPY_LOAD_SHORT: copy +JIT_OP_COPY_LOAD_SBYTE, JIT_OP_COPY_LOAD_UBYTE, JIT_OP_COPY_STORE_BYTE: copy + [=local, imm] -> { + x86_mov_membase_imm(inst, X86_EBP, $1, $2, 1); + } + [=local, breg] -> { + x86_mov_membase_reg(inst, X86_EBP, $1, $2, 1); + } [reg] -> {} -JIT_OP_COPY_LOAD_USHORT: copy +JIT_OP_COPY_LOAD_SHORT, JIT_OP_COPY_LOAD_USHORT, JIT_OP_COPY_STORE_SHORT: copy + [=local, imm] -> { + x86_mov_membase_imm(inst, X86_EBP, $1, $2, 2); + } + [=local, reg] -> { + x86_mov_membase_reg(inst, X86_EBP, $1, $2, 2); + } [reg] -> {} JIT_OP_COPY_INT: copy + [=local, imm] -> { + x86_mov_membase_imm(inst, X86_EBP, $1, $2, 4); + } [reg] -> {} JIT_OP_COPY_LONG: copy @@ -1896,22 +1905,6 @@ JIT_OP_COPY_STRUCT: jit_type_get_size(jit_value_get_type(insn->dest))); } -JIT_OP_COPY_STORE_BYTE: - [=frame, imm] -> { - x86_mov_membase_imm(inst, X86_EBP, $1, $2, 1); - } - [=frame, breg] -> { - x86_mov_membase_reg(inst, X86_EBP, $1, $2, 1); - } - -JIT_OP_COPY_STORE_SHORT: - [=frame, imm] -> { - x86_mov_membase_imm(inst, X86_EBP, $1, $2, 2); - } - [=frame, reg] -> { - x86_mov_membase_reg(inst, X86_EBP, $1, $2, 2); - } - JIT_OP_ADDRESS_OF: [=reg, frame] -> { x86_lea_membase(inst, $1, X86_EBP, $2); diff --git a/tools/gen-rules-parser.y b/tools/gen-rules-parser.y index 8bebf8d..10e4405 100644 --- a/tools/gen-rules-parser.y +++ b/tools/gen-rules-parser.y @@ -21,6 +21,7 @@ #include #include +#include #ifdef HAVE_STRING_H #include #elif defined(HAVE_STRINGS_H) @@ -283,7 +284,7 @@ gensel_create_value(int type) } /* - * Create string value. + * Create literal string value. */ static gensel_value_t gensel_create_string_value(char *value) @@ -297,7 +298,7 @@ gensel_create_string_value(char *value) } /* - * Create string value. + * Create register class value. */ static gensel_value_t gensel_create_regclass_value(char *value) @@ -938,6 +939,24 @@ gensel_output_register_pattern(char *name, gensel_option_t pattern) gensel_output_register(name, pattern->values->value, pattern->values->next); } +/* + * Create an upper-case copy of a string. + */ +static char * +gensel_string_upper(char *string) +{ + char *cp; + if(string) + { + string = strdup(string); + for(cp = string; *cp; cp++) + { + *cp = toupper(*cp); + } + } + return string; +} + /* * Output the clauses for a rule. */ @@ -956,6 +975,7 @@ static void gensel_output_clauses(gensel_clause_t clauses, gensel_option_t optio int ternary, free_dest; int contains_registers; gensel_regclass_t regclass; + char *uc_arg; /* If the clause is manual, then output it as-is */ if(gensel_search_option(options, GENSEL_OPT_MANUAL)) @@ -1107,6 +1127,14 @@ static void gensel_output_clauses(gensel_clause_t clauses, gensel_option_t optio printf("!insn->%s->is_constant && ", args[index]); printf("!insn->%s->in_register && ", args[index]); printf("!insn->%s->has_global_register", args[index]); + /* If the value is used again in the same basic block + it is highly likely that using a register instead + of the stack will be a win. Assume that if the + "local" pattern is not the last one then it must + be followed by a "reg" pattern. */ + uc_arg = gensel_string_upper(args[index]); + printf("&& (insn->flags & JIT_INSN_%s_NEXT_USE) == 0", uc_arg); + free(uc_arg); seen_option = 1; ++index; break; -- 2.47.3