| Message ID | 20160406173255.GA27512@flamenco (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On 04/06/2016 10:32 AM, Emilio G. Cota wrote: > On Wed, Apr 06, 2016 at 08:06:57 +0200, Laurent Desnogues wrote: >> On Tue, Apr 5, 2016 at 7:19 PM, Richard Henderson <rth@twiddle.net> wrote: >>> On 04/05/2016 09:33 AM, Laurent Desnogues wrote: >>>> The 'flags' field is 64-bit. You're thinking of cflags, I guess. >>> >>> Well that's silly. Since it's filled in via >>> >>> static inline void cpu_get_tb_cpu_state(CPUMIPSState *env, target_ulong *pc, >>> target_ulong *cs_base, int *flags) >>> >>> and passed back in to generate code with >>> >>> TranslationBlock *tb_gen_code(CPUState *cpu, >>> target_ulong pc, target_ulong cs_base, int flags, >>> int cflags); >>> >>> So while TranslationBlock stores "uint64_t", the producer and consumer see "int". >> >> I agree. I guess TranslationBlock should be fixed to use uint32_t >> (note several functions have to be changed from using int to uint32_t >> or aarch64-softmmu will fail). > > Can you please elaborate on this? The arm port is using some high bits, including #define ARM_TBFLAG_AARCH64_STATE_SHIFT 31 #define ARM_TBFLAG_AARCH64_STATE_MASK (1U << ARM_TBFLAG_AARCH64_STATE_SHIFT) So, I would certainly be ok switching everything to use uint32_t over int. r~
On Wed, Apr 6, 2016 at 7:42 PM, Richard Henderson <rth@twiddle.net> wrote: > On 04/06/2016 10:32 AM, Emilio G. Cota wrote: >> On Wed, Apr 06, 2016 at 08:06:57 +0200, Laurent Desnogues wrote: >>> On Tue, Apr 5, 2016 at 7:19 PM, Richard Henderson <rth@twiddle.net> wrote: >>>> On 04/05/2016 09:33 AM, Laurent Desnogues wrote: >>>>> The 'flags' field is 64-bit. You're thinking of cflags, I guess. >>>> >>>> Well that's silly. Since it's filled in via >>>> >>>> static inline void cpu_get_tb_cpu_state(CPUMIPSState *env, target_ulong *pc, >>>> target_ulong *cs_base, int *flags) >>>> >>>> and passed back in to generate code with >>>> >>>> TranslationBlock *tb_gen_code(CPUState *cpu, >>>> target_ulong pc, target_ulong cs_base, int flags, >>>> int cflags); >>>> >>>> So while TranslationBlock stores "uint64_t", the producer and consumer see "int". >>> >>> I agree. I guess TranslationBlock should be fixed to use uint32_t >>> (note several functions have to be changed from using int to uint32_t >>> or aarch64-softmmu will fail). >> >> Can you please elaborate on this? > > The arm port is using some high bits, including > > #define ARM_TBFLAG_AARCH64_STATE_SHIFT 31 > #define ARM_TBFLAG_AARCH64_STATE_MASK (1U << ARM_TBFLAG_AARCH64_STATE_SHIFT) Yes, that's why I advocate the use of uint32_t over int. Emilio, I can't reproduce the failure I had, but fixing the warnings gcc issues when using uint32_t instead of int for cpu_get_tb_cpu_state should be enough to make the change safe. Note this doesn't really belong to this patch set :-) Laurent
diff --git a/cpu-exec.c b/cpu-exec.c index bbfcbfb..5abbf57 100644 --- a/cpu-exec.c +++ b/cpu-exec.c @@ -220,7 +220,7 @@ static void cpu_exec_nocache(CPUState *cpu, int max_cycles, static TranslationBlock *tb_find_physical(CPUState *cpu, target_ulong pc, target_ulong cs_base, - uint64_t flags) + int flags) { CPUArchState *env = (CPUArchState *)cpu->env_ptr; TranslationBlock *tb, **ptb1; @@ -271,7 +271,7 @@ static TranslationBlock *tb_find_physical(CPUState *cpu, static TranslationBlock *tb_find_slow(CPUState *cpu, target_ulong pc, target_ulong cs_base, - uint64_t flags) + int flags) { TranslationBlock *tb; diff --git a/include/exec/exec-all.h b/include/exec/exec-all.h index 7362095..277e6f1 100644 --- a/include/exec/exec-all.h +++ b/include/exec/exec-all.h @@ -235,7 +235,7 @@ static inline void tlb_flush_by_mmuidx(CPUState *cpu, ...) struct TranslationBlock { target_ulong pc; /* simulated PC corresponding to this block (EIP + CS base) */ target_ulong cs_base; /* CS base for this block */ - uint64_t flags; /* flags defining in which context the code was generated */ + int flags; /* flags defining in which context the code was generated */ uint16_t size; /* size of target code for this block (1 <= size <= TARGET_PAGE_SIZE) */ uint16_t icount; diff --git a/target-i386/translate.c b/target-i386/translate.c index 1a1214d..4024ad4 100644 --- a/target-i386/translate.c +++ b/target-i386/translate.c @@ -8178,7 +8178,7 @@ void gen_intermediate_code(CPUX86State *env, TranslationBlock *tb) CPUState *cs = CPU(cpu); DisasContext dc1, *dc = &dc1; target_ulong pc_ptr; - uint64_t flags; + int flags; target_ulong pc_start; target_ulong cs_base; int num_insns; diff --git a/translate-all.c b/translate-all.c index 8329ea6..27b4d57 100644 --- a/translate-all.c +++ b/translate-all.c @@ -1593,7 +1593,7 @@ void cpu_io_recompile(CPUState *cpu, uintptr_t retaddr) TranslationBlock *tb; uint32_t n, cflags; target_ulong pc, cs_base; - uint64_t flags; + int flags; tb = tb_find_pc(retaddr); if (!tb) {