On Tue, 18 Mar 2025 at 17:52, Pierrick Bouvier
<pierrick.bouv...@linaro.org> wrote:
>
> On 3/18/25 10:50, Peter Maydell wrote:
> > On Tue, 18 Mar 2025 at 17:42, Philippe Mathieu-Daudé <phi...@linaro.org>
> > 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.bouv...@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) {
> >>
> >
> > That would be a dead check: it is not possible to get here
> > unless ARM_FEATURE_AARCH64 is set.
> >
>
> We can then assert it, to make sure there is no regression around that.
We have a lot of write/read/access fns for AArch64-only sysregs, and
we don't need to assert in all of them that they're called only when
the CPU has AArch64 enabled.
> We now have another conversation and something to decide in another
> file, and that's why I chose to do the minimal change ("ifdef the
> issue") instead of trying to do any change.
I think we can fairly easily avoid ifdeffing the callsite of
aarch64_sve_narrow_vq(). Currently we have:
* a real version of the function, whose definition is inside
an ifdef TARGET_AARCH64 in target/arm/helper.c
* a stub version, inline in the cpu.h header
If we don't want to have the stub version with ifdefs, then we can
move the real implementation of the function to not be inside the
ifdef (matching the fact that the prototype is no longer inside
an ifdef). The function doesn't call any other functions that are
TARGET_AARCH64 only, so it shouldn't be a "now we have to move
50 other things" problem, I hope.
thanks
-- PMM
