Re: [PATCH 118/147] target/arm/cpu: remove inline stubs for aarch32 emulation

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

On 4/23/25 03:35, Philippe Mathieu-Daudé wrote:
> On 22/4/25 21:27, Richard Henderson wrote:
>> From: Pierrick Bouvier <pierrick.bouvier@linaro.org>
>>
>> Directly condition associated calls in target/arm/helper.c for now.
>>
>> Reviewed-by: Richard Henderson <richard.henderson@linaro.org>
>> Signed-off-by: Pierrick Bouvier <pierrick.bouvier@linaro.org>
>> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
>> Message-ID: <20250325045915.994760-23-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 b1c3e46326..c1a0faed3a 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);
>>    
> 
> Should we complete squashing:
> 
> -- >8 --
> diff --git a/target/arm/internals.h b/target/arm/internals.h
> index cf4ab17bc08..f9353887415 100644
> --- a/target/arm/internals.h
> +++ b/target/arm/internals.h
> @@ -1810,7 +1810,6 @@ static inline uint64_t
> pmu_counter_mask(CPUARMState *env)
>      return (1ULL << 31) | ((1ULL << pmu_num_counters(env)) - 1);
>    }
> 
> -#ifdef TARGET_AARCH64
>    GDBFeature *arm_gen_dynamic_svereg_feature(CPUState *cpu, int base_reg);
>    int aarch64_gdb_get_sve_reg(CPUState *cs, GByteArray *buf, int reg);
>    int aarch64_gdb_set_sve_reg(CPUState *cs, uint8_t *buf, int reg);
> @@ -1820,7 +1819,6 @@ int aarch64_gdb_get_pauth_reg(CPUState *cs,
> GByteArray *buf, int reg);
>    int aarch64_gdb_set_pauth_reg(CPUState *cs, uint8_t *buf, int reg);
>    int aarch64_gdb_get_tag_ctl_reg(CPUState *cs, GByteArray *buf, int reg);
>    int aarch64_gdb_set_tag_ctl_reg(CPUState *cs, uint8_t *buf, int reg);
> -#endif
>    void arm_cpu_sve_finalize(ARMCPU *cpu, Error **errp);
>    void arm_cpu_sme_finalize(ARMCPU *cpu, Error **errp);
>    void arm_cpu_pauth_finalize(ARMCPU *cpu, Error **errp);
> ---
> 
> ?
> 

This part of the series focus on hw/arm, so it was not needed to clean 
target/arm/internals.h as part of it.
That's why I "pushed" the TARGET_AARCH64 #ifdef to target/arm/helper.c, 
allowing to do it later.
I tried to cleanup that completely at the time, as requested by Peter, 
but it was pulling too many things, so I just dropped it.

So I think we should not squash it here.

>> diff --git a/target/arm/helper.c b/target/arm/helper.c
>> index becbbbd0d8..7fb6e88630 100644
>> --- a/target/arm/helper.c
>> +++ b/target/arm/helper.c
>> @@ -6563,7 +6563,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
>>            aarch64_sve_narrow_vq(env, new_len + 1);
>> +#endif
>>        }
>>    }
>>    
>> @@ -10628,7 +10630,9 @@ static void arm_cpu_do_interrupt_aarch64(CPUState *cs)
>>             * Note that new_el can never be 0.  If cur_el is 0, then
>>             * el0_a64 is is_a64(), else el0_a64 is ignored.
>>             */
>> +#ifdef TARGET_AARCH64
>>            aarch64_sve_change_el(env, cur_el, new_el, is_a64(env));
>> +#endif
>>        }
>>    
>>        if (cur_el < new_el) {
>> @@ -11640,7 +11644,9 @@ void aarch64_sve_change_el(CPUARMState *env, int old_el,
>>    
>>        /* When changing vector length, clear inaccessible state.  */
>>        if (new_len < old_len) {
>> +#ifdef TARGET_AARCH64
>>            aarch64_sve_narrow_vq(env, new_len + 1);
>> +#endif
>>        }
>>    }
>>    #endif
>