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

[Philippe Mathieu-Daudé]

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:

-- >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.