| Message ID | 20240119144024.14289-28-anjo@rev.ng (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | Compile accel/tcg once (partially) | expand |
On 1/20/24 00:40, Anton Johansson wrote: > Makes translate-all.c independent of softmmu target by switching > > TARGET_LONG_BITS -> target_long_bits() > > TARGET_INSN_START_WORDS -> tcg_ctx->insn_start_words, > target_insn_start_words(), > > TCG_GUEST_DEFAULT_MO -> target_default_memory_order() > > MO_TE -> target_endian_memory_order() > > Signed-off-by: Anton Johansson <anjo@rev.ng> > --- > accel/tcg/translate-all.c | 38 ++++++++++++++++++-------------------- > 1 file changed, 18 insertions(+), 20 deletions(-) > > diff --git a/accel/tcg/translate-all.c b/accel/tcg/translate-all.c > index 9c981d1750..a3ae0c6910 100644 > --- a/accel/tcg/translate-all.c > +++ b/accel/tcg/translate-all.c > @@ -65,7 +65,6 @@ > #include "internal-common.h" > #include "internal-target.h" > #include "perf.h" > -#include "tcg/insn-start-words.h" > > TBContext tb_ctx; > > @@ -106,7 +105,7 @@ static int64_t decode_sleb128(const uint8_t **pp) > val |= (int64_t)(byte & 0x7f) << shift; > shift += 7; > } while (byte & 0x80); > - if (shift < TARGET_LONG_BITS && (byte & 0x40)) { > + if (shift < target_long_bits() && (byte & 0x40)) { You just make TARGET_PAGE_SIZE etc target independent, right? So you don't need this? Or is this because of user-only still. > static int encode_search(TranslationBlock *tb, uint8_t *block) > { > + const uint8_t insn_start_words = tcg_ctx->insn_start_words; Ok, because we're still inside the compilation context. > static int cpu_unwind_data_from_tb(TranslationBlock *tb, uintptr_t host_pc, > uint64_t *data) > { > + const uint8_t insn_start_words = tcg_ctx->insn_start_words; Not ok, because we're outside of the compilation context. r~
On 24/01/24, Richard Henderson wrote: > On 1/20/24 00:40, Anton Johansson wrote: > > Makes translate-all.c independent of softmmu target by switching > > > > TARGET_LONG_BITS -> target_long_bits() > > > > TARGET_INSN_START_WORDS -> tcg_ctx->insn_start_words, > > target_insn_start_words(), > > > > TCG_GUEST_DEFAULT_MO -> target_default_memory_order() > > > > MO_TE -> target_endian_memory_order() > > > > Signed-off-by: Anton Johansson <anjo@rev.ng> > > --- > > accel/tcg/translate-all.c | 38 ++++++++++++++++++-------------------- > > 1 file changed, 18 insertions(+), 20 deletions(-) > > > > diff --git a/accel/tcg/translate-all.c b/accel/tcg/translate-all.c > > index 9c981d1750..a3ae0c6910 100644 > > --- a/accel/tcg/translate-all.c > > +++ b/accel/tcg/translate-all.c > > @@ -65,7 +65,6 @@ > > #include "internal-common.h" > > #include "internal-target.h" > > #include "perf.h" > > -#include "tcg/insn-start-words.h" > > TBContext tb_ctx; > > @@ -106,7 +105,7 @@ static int64_t decode_sleb128(const uint8_t **pp) > > val |= (int64_t)(byte & 0x7f) << shift; > > shift += 7; > > } while (byte & 0x80); > > - if (shift < TARGET_LONG_BITS && (byte & 0x40)) { > > + if (shift < target_long_bits() && (byte & 0x40)) { > > You just make TARGET_PAGE_SIZE etc target independent, right? > So you don't need this? Or is this because of user-only still. Hi, Richard! I missed this piece of feedback previously. I don't see how TARGET_PAGE_[SIZE|BITS] would be used here. Are the values we're encoding always TARGET_PAGE_BITS in size? //Anton
On 6/13/24 02:50, Anton Johansson wrote: > On 24/01/24, Richard Henderson wrote: >> On 1/20/24 00:40, Anton Johansson wrote: >>> Makes translate-all.c independent of softmmu target by switching >>> >>> TARGET_LONG_BITS -> target_long_bits() >>> >>> TARGET_INSN_START_WORDS -> tcg_ctx->insn_start_words, >>> target_insn_start_words(), >>> >>> TCG_GUEST_DEFAULT_MO -> target_default_memory_order() >>> >>> MO_TE -> target_endian_memory_order() >>> >>> Signed-off-by: Anton Johansson <anjo@rev.ng> >>> --- >>> accel/tcg/translate-all.c | 38 ++++++++++++++++++-------------------- >>> 1 file changed, 18 insertions(+), 20 deletions(-) >>> >>> diff --git a/accel/tcg/translate-all.c b/accel/tcg/translate-all.c >>> index 9c981d1750..a3ae0c6910 100644 >>> --- a/accel/tcg/translate-all.c >>> +++ b/accel/tcg/translate-all.c >>> @@ -65,7 +65,6 @@ >>> #include "internal-common.h" >>> #include "internal-target.h" >>> #include "perf.h" >>> -#include "tcg/insn-start-words.h" >>> TBContext tb_ctx; >>> @@ -106,7 +105,7 @@ static int64_t decode_sleb128(const uint8_t **pp) >>> val |= (int64_t)(byte & 0x7f) << shift; >>> shift += 7; >>> } while (byte & 0x80); >>> - if (shift < TARGET_LONG_BITS && (byte & 0x40)) { >>> + if (shift < target_long_bits() && (byte & 0x40)) { >> >> You just make TARGET_PAGE_SIZE etc target independent, right? >> So you don't need this? Or is this because of user-only still. > > Hi, Richard! > > I missed this piece of feedback previously. I don't see how > TARGET_PAGE_[SIZE|BITS] would be used here. Are the values we're > encoding always TARGET_PAGE_BITS in size? I was obviously tired here, since they're obviously unrelated. However in this case I think TARGET_LONG_BITS is a red herring, and we should be using int64_t not target_long at all, and thus the shift count must be less than 64. r~
On 18/06/24, Richard Henderson wrote: > On 6/13/24 02:50, Anton Johansson wrote: > > On 24/01/24, Richard Henderson wrote: > > > On 1/20/24 00:40, Anton Johansson wrote: > > > > Makes translate-all.c independent of softmmu target by switching > > > > > > > > TARGET_LONG_BITS -> target_long_bits() > > > > > > > > TARGET_INSN_START_WORDS -> tcg_ctx->insn_start_words, > > > > target_insn_start_words(), > > > > > > > > TCG_GUEST_DEFAULT_MO -> target_default_memory_order() > > > > > > > > MO_TE -> target_endian_memory_order() > > > > > > > > Signed-off-by: Anton Johansson <anjo@rev.ng> > > > > --- > > > > accel/tcg/translate-all.c | 38 ++++++++++++++++++-------------------- > > > > 1 file changed, 18 insertions(+), 20 deletions(-) > > > > > > > > diff --git a/accel/tcg/translate-all.c b/accel/tcg/translate-all.c > > > > index 9c981d1750..a3ae0c6910 100644 > > > > --- a/accel/tcg/translate-all.c > > > > +++ b/accel/tcg/translate-all.c > > > > @@ -65,7 +65,6 @@ > > > > #include "internal-common.h" > > > > #include "internal-target.h" > > > > #include "perf.h" > > > > -#include "tcg/insn-start-words.h" > > > > TBContext tb_ctx; > > > > @@ -106,7 +105,7 @@ static int64_t decode_sleb128(const uint8_t **pp) > > > > val |= (int64_t)(byte & 0x7f) << shift; > > > > shift += 7; > > > > } while (byte & 0x80); > > > > - if (shift < TARGET_LONG_BITS && (byte & 0x40)) { > > > > + if (shift < target_long_bits() && (byte & 0x40)) { > > > > > > You just make TARGET_PAGE_SIZE etc target independent, right? > > > So you don't need this? Or is this because of user-only still. > > > > Hi, Richard! > > > > I missed this piece of feedback previously. I don't see how > > TARGET_PAGE_[SIZE|BITS] would be used here. Are the values we're > > encoding always TARGET_PAGE_BITS in size? > > I was obviously tired here, since they're obviously unrelated. > > However in this case I think TARGET_LONG_BITS is a red herring, and we > should be using int64_t not target_long at all, and thus the shift count > must be less than 64. > > > r~ > Ah I see, thanks!:) I'll give that a go then.
diff --git a/accel/tcg/translate-all.c b/accel/tcg/translate-all.c index 9c981d1750..a3ae0c6910 100644 --- a/accel/tcg/translate-all.c +++ b/accel/tcg/translate-all.c @@ -65,7 +65,6 @@ #include "internal-common.h" #include "internal-target.h" #include "perf.h" -#include "tcg/insn-start-words.h" TBContext tb_ctx; @@ -106,7 +105,7 @@ static int64_t decode_sleb128(const uint8_t **pp) val |= (int64_t)(byte & 0x7f) << shift; shift += 7; } while (byte & 0x80); - if (shift < TARGET_LONG_BITS && (byte & 0x40)) { + if (shift < target_long_bits() && (byte & 0x40)) { val |= -(int64_t)1 << shift; } @@ -117,7 +116,7 @@ static int64_t decode_sleb128(const uint8_t **pp) /* Encode the data collected about the instructions while compiling TB. Place the data at BLOCK, and return the number of bytes consumed. - The logical table consists of TARGET_INSN_START_WORDS target_ulong's, + The logical table consists of tcg_ctx->insn_start_words target_ulong's, which come from the target's insn_start data, followed by a uintptr_t which comes from the host pc of the end of the code implementing the insn. @@ -128,6 +127,7 @@ static int64_t decode_sleb128(const uint8_t **pp) static int encode_search(TranslationBlock *tb, uint8_t *block) { + const uint8_t insn_start_words = tcg_ctx->insn_start_words; uint8_t *highwater = tcg_ctx->code_gen_highwater; uint64_t *insn_data = tcg_ctx->gen_insn_data; uint16_t *insn_end_off = tcg_ctx->gen_insn_end_off; @@ -137,13 +137,13 @@ static int encode_search(TranslationBlock *tb, uint8_t *block) for (i = 0, n = tb->icount; i < n; ++i) { uint64_t prev, curr; - for (j = 0; j < TARGET_INSN_START_WORDS; ++j) { + for (j = 0; j < insn_start_words; ++j) { if (i == 0) { prev = (!(tb_cflags(tb) & CF_PCREL) && j == 0 ? tb->pc : 0); } else { - prev = insn_data[(i - 1) * TARGET_INSN_START_WORDS + j]; + prev = insn_data[(i - 1) * insn_start_words + j]; } - curr = insn_data[i * TARGET_INSN_START_WORDS + j]; + curr = insn_data[i * insn_start_words + j]; p = encode_sleb128(p, curr - prev); } prev = (i == 0 ? 0 : insn_end_off[i - 1]); @@ -165,6 +165,7 @@ static int encode_search(TranslationBlock *tb, uint8_t *block) static int cpu_unwind_data_from_tb(TranslationBlock *tb, uintptr_t host_pc, uint64_t *data) { + const uint8_t insn_start_words = tcg_ctx->insn_start_words; uintptr_t iter_pc = (uintptr_t)tb->tc.ptr; const uint8_t *p = tb->tc.ptr + tb->tc.size; int i, j, num_insns = tb->icount; @@ -175,7 +176,7 @@ static int cpu_unwind_data_from_tb(TranslationBlock *tb, uintptr_t host_pc, return -1; } - memset(data, 0, sizeof(uint64_t) * TARGET_INSN_START_WORDS); + memset(data, 0, sizeof(uint64_t) * insn_start_words); if (!(tb_cflags(tb) & CF_PCREL)) { data[0] = tb->pc; } @@ -185,7 +186,7 @@ static int cpu_unwind_data_from_tb(TranslationBlock *tb, uintptr_t host_pc, * at which the end of the insn exceeds host_pc. */ for (i = 0; i < num_insns; ++i) { - for (j = 0; j < TARGET_INSN_START_WORDS; ++j) { + for (j = 0; j < insn_start_words; ++j) { data[j] += decode_sleb128(&p); } iter_pc += decode_sleb128(&p); @@ -203,7 +204,7 @@ static int cpu_unwind_data_from_tb(TranslationBlock *tb, uintptr_t host_pc, void cpu_restore_state_from_tb(CPUState *cpu, TranslationBlock *tb, uintptr_t host_pc) { - uint64_t data[TARGET_INSN_START_WORDS]; + uint64_t data[tcg_ctx->insn_start_words]; int insns_left = cpu_unwind_data_from_tb(tb, host_pc, data); if (insns_left < 0) { @@ -341,19 +342,15 @@ TranslationBlock *tb_gen_code(CPUState *cpu, } tcg_ctx->gen_tb = tb; - tcg_ctx->addr_type = TARGET_LONG_BITS == 32 ? TCG_TYPE_I32 : TCG_TYPE_I64; + tcg_ctx->addr_type = target_long_bits() == 32 ? TCG_TYPE_I32 : TCG_TYPE_I64; #ifdef CONFIG_SOFTMMU - tcg_ctx->mo_te = MO_TE; + tcg_ctx->mo_te = target_endian_memory_order(); tcg_ctx->page_bits = TARGET_PAGE_BITS; tcg_ctx->page_mask = TARGET_PAGE_MASK; - tcg_ctx->tlb_dyn_max_bits = CPU_TLB_DYN_MAX_BITS; -#endif - tcg_ctx->insn_start_words = TARGET_INSN_START_WORDS; -#ifdef TCG_GUEST_DEFAULT_MO - tcg_ctx->guest_mo = TCG_GUEST_DEFAULT_MO; -#else - tcg_ctx->guest_mo = TCG_MO_ALL; + tcg_ctx->tlb_dyn_max_bits = target_tlb_dyn_max_bits(); #endif + tcg_ctx->insn_start_words = target_insn_start_words(); + tcg_ctx->guest_mo = target_default_memory_order(); restart_translate: trace_translate_block(tb, pc, tb->tc.ptr); @@ -441,6 +438,7 @@ TranslationBlock *tb_gen_code(CPUState *cpu, qemu_log_in_addr_range(pc)) { FILE *logfile = qemu_log_trylock(); if (logfile) { + const uint8_t insn_start_words = tcg_ctx->insn_start_words; int code_size, data_size; const tcg_target_ulong *rx_data_gen_ptr; size_t chunk_start; @@ -460,7 +458,7 @@ TranslationBlock *tb_gen_code(CPUState *cpu, fprintf(logfile, "OUT: [size=%d]\n", gen_code_size); fprintf(logfile, " -- guest addr 0x%016" PRIx64 " + tb prologue\n", - tcg_ctx->gen_insn_data[insn * TARGET_INSN_START_WORDS]); + tcg_ctx->gen_insn_data[insn * insn_start_words]); chunk_start = tcg_ctx->gen_insn_end_off[insn]; disas(logfile, tb->tc.ptr, chunk_start); @@ -473,7 +471,7 @@ TranslationBlock *tb_gen_code(CPUState *cpu, size_t chunk_end = tcg_ctx->gen_insn_end_off[insn]; if (chunk_end > chunk_start) { fprintf(logfile, " -- guest addr 0x%016" PRIx64 "\n", - tcg_ctx->gen_insn_data[insn * TARGET_INSN_START_WORDS]); + tcg_ctx->gen_insn_data[insn * insn_start_words]); disas(logfile, tb->tc.ptr + chunk_start, chunk_end - chunk_start); chunk_start = chunk_end;
Makes translate-all.c independent of softmmu target by switching TARGET_LONG_BITS -> target_long_bits() TARGET_INSN_START_WORDS -> tcg_ctx->insn_start_words, target_insn_start_words(), TCG_GUEST_DEFAULT_MO -> target_default_memory_order() MO_TE -> target_endian_memory_order() Signed-off-by: Anton Johansson <anjo@rev.ng> --- accel/tcg/translate-all.c | 38 ++++++++++++++++++-------------------- 1 file changed, 18 insertions(+), 20 deletions(-)