Re: [Qemu-devel] [PATCH 7/7] target/xtensa: move non-HELPER functions to helper.c

[Philippe Mathieu-Daudé]

On 5/17/21 3:10 PM, Philippe Mathieu-Daudé wrote:
> On 5/17/21 2:11 PM, Max Filippov wrote:
>> On Mon, May 17, 2021 at 4:50 AM Max Filippov <jcmvbkbc@gmail.com> wrote:
>>>
>>> Hi Philippe,
>>>
>>> On Sun, May 16, 2021 at 10:05 PM Philippe Mathieu-Daudé
>>> <philippe@mathieu-daude.net> wrote:
>>>>
>>>> Hi Max,
>>>>
>>>> On Mon, Jan 14, 2019 at 8:52 AM Max Filippov <jcmvbkbc@gmail.com> wrote:
>>>>>
>>>>> Move remaining non-HELPER functions from op_helper.c to helper.c.
>>>>> No functional changes.
>>>>>
>>>>> Signed-off-by: Max Filippov <jcmvbkbc@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:

I meant 'IIUC' (if I understand correctly)...

> -- >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));
> +    assert(!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);
>  }
> ---
> 
> ?
> 
> Thanks for the quick response btw :)
> 
> Phil.
>