On Mon, May 17, 2021 at 6:10 AM Philippe Mathieu-Daudé <f4...@amsat.org> wrote: > > On 5/17/21 2:11 PM, Max Filippov wrote: > > On Mon, May 17, 2021 at 4:50 AM Max Filippov <jcmvb...@gmail.com> wrote: > >> > >> Hi Philippe, > >> > >> On Sun, May 16, 2021 at 10:05 PM Philippe Mathieu-Daudé > >> <phili...@mathieu-daude.net> wrote: > >>> > >>> Hi Max, > >>> > >>> On Mon, Jan 14, 2019 at 8:52 AM Max Filippov <jcmvb...@gmail.com> wrote: > >>>> > >>>> Move remaining non-HELPER functions from op_helper.c to helper.c. > >>>> No functional changes. > >>>> > >>>> Signed-off-by: Max Filippov <jcmvb...@gmail.com> > >>>> --- > >>>> target/xtensa/helper.c | 61 > >>>> ++++++++++++++++++++++++++++++++++++++++++++--- > >>>> target/xtensa/op_helper.c | 56 > >>>> ------------------------------------------- > >>>> 2 files changed, 58 insertions(+), 59 deletions(-) > >>> > >>>> +void xtensa_cpu_do_unaligned_access(CPUState *cs, > >>>> + vaddr addr, MMUAccessType > >>>> access_type, > >>>> + int mmu_idx, uintptr_t retaddr) > >>>> +{ > >>>> + XtensaCPU *cpu = XTENSA_CPU(cs); > >>>> + CPUXtensaState *env = &cpu->env; > >>>> + > >>>> + if (xtensa_option_enabled(env->config, > >>>> XTENSA_OPTION_UNALIGNED_EXCEPTION) && > >>>> + !xtensa_option_enabled(env->config, > >>>> XTENSA_OPTION_HW_ALIGNMENT)) { > >>> > >>> I know this is a simple code movement, but I wonder, what should > >>> happen when there is > >>> an unaligned fault and the options are disabled? Is this an impossible > >>> case (unreachable)? > >> > >> It should be unreachable when XTENSA_OPTION_UNALIGNED_EXCEPTION > >> is disabled. In that case the translation code generates access on aligned > >> addresses according to the xtensa ISA, see the function > >> gen_load_store_alignment in target/xtensa/translate.c > > > > There's also a case when both options are enabled, i.e. the > > xtensa core has support for transparent unaligned access. > > In that case the helper does nothing and the generic TCG > > code is supposed to deal with the unaligned access correctly, > > IIRC we can simplify as: > > -- >8 -- > diff --git a/target/xtensa/helper.c b/target/xtensa/helper.c > index eeffee297d1..6e8a6cdc99e 100644 > --- a/target/xtensa/helper.c > +++ b/target/xtensa/helper.c > @@ -270,13 +270,14 @@ void xtensa_cpu_do_unaligned_access(CPUState *cs, > XtensaCPU *cpu = XTENSA_CPU(cs); > CPUXtensaState *env = &cpu->env; > > - if (xtensa_option_enabled(env->config, > XTENSA_OPTION_UNALIGNED_EXCEPTION) && > - !xtensa_option_enabled(env->config, XTENSA_OPTION_HW_ALIGNMENT)) { > - cpu_restore_state(CPU(cpu), retaddr, true); > - HELPER(exception_cause_vaddr)(env, > - env->pc, LOAD_STORE_ALIGNMENT_CAUSE, > - addr); > - } > + assert(xtensa_option_enabled(env->config, > + XTENSA_OPTION_UNALIGNED_EXCEPTION));
This part -- yes. > + assert(!xtensa_option_enabled(env->config, > XTENSA_OPTION_HW_ALIGNMENT)); This part -- no, because the call to the TCGCPUOps::do_unaligned_access is unconditional and would happen on CPUs that have XTENSA_OPTION_HW_ALIGNMENT enabled. They could have a different CPUClass::tcg_ops, but I'm not sure it's worth it. -- Thanks. -- Max