Re: [PATCH 11/13] target/arm/cpu: remove inline stubs for aarch32 emulation

[Pierrick Bouvier <pierrick.bouvier@linaro.org>]

On 3/18/25 10:42, Philippe Mathieu-Daudé wrote:
> On 18/3/25 05:51, Pierrick Bouvier wrote:
>> Directly condition associated calls in target/arm/helper.c for now.
>>
>> Signed-off-by: Pierrick Bouvier <pierrick.bouvier@linaro.org>
>> ---
>>    target/arm/cpu.h    | 8 --------
>>    target/arm/helper.c | 6 ++++++
>>    2 files changed, 6 insertions(+), 8 deletions(-)
>>
>> diff --git a/target/arm/cpu.h b/target/arm/cpu.h
>> index 51b6428cfec..9205cbdec43 100644
>> --- a/target/arm/cpu.h
>> +++ b/target/arm/cpu.h
>> @@ -1222,7 +1222,6 @@ int arm_cpu_write_elf32_note(WriteCoreDumpFunction f, CPUState *cs,
>>     */
>>    void arm_emulate_firmware_reset(CPUState *cpustate, int target_el);
>>    
>> -#ifdef TARGET_AARCH64
>>    int aarch64_cpu_gdb_read_register(CPUState *cpu, GByteArray *buf, int reg);
>>    int aarch64_cpu_gdb_write_register(CPUState *cpu, uint8_t *buf, int reg);
>>    void aarch64_sve_narrow_vq(CPUARMState *env, unsigned vq);
>> @@ -1254,13 +1253,6 @@ static inline uint64_t *sve_bswap64(uint64_t *dst, uint64_t *src, int nr)
>>    #endif
>>    }
>>    
>> -#else
>> -static inline void aarch64_sve_narrow_vq(CPUARMState *env, unsigned vq) { }
>> -static inline void aarch64_sve_change_el(CPUARMState *env, int o,
>> -                                         int n, bool a)
>> -{ }
>> -#endif
>> -
>>    void aarch64_sync_32_to_64(CPUARMState *env);
>>    void aarch64_sync_64_to_32(CPUARMState *env);
>>    
>> diff --git a/target/arm/helper.c b/target/arm/helper.c
>> index b46b2bffcf3..774e1ee0245 100644
>> --- a/target/arm/helper.c
>> +++ b/target/arm/helper.c
>> @@ -6562,7 +6562,9 @@ static void zcr_write(CPUARMState *env, const ARMCPRegInfo *ri,
>>         */
>>        new_len = sve_vqm1_for_el(env, cur_el);
>>        if (new_len < old_len) {
>> +#ifdef TARGET_AARCH64
> 
> What about using runtime check instead?
> 
>    if (arm_feature(&cpu->env, ARM_FEATURE_AARCH64) && new_len < old_len) {
> 

This is the right way to deal with it, but I would prefer to do it when 
the concerned file (target/arm/helper.c) will be modified, in a future 
series.

The current one focuses on hw/arm and "pushes back" dependencies in 
other places, that we can deal with later.

Nothing wrong with the change proposed, just trying to focus on the 
minimal set needed to reach the goal, without any detour.

>>            aarch64_sve_narrow_vq(env, new_len + 1);
>> +#endif
>>        }
>>    }
>