On Mon, 14 Jul 2025 at 16:41, Pierrick Bouvier
<pierrick.bouv...@linaro.org> wrote:
> As folding is not guaranteed by C standard, I'm not sure it's really
> possible to file a bug. However, since we rely on this behaviour in
> other parts, maybe it would be better to rewrite the condition on our side.
>
> By changing the code to this, the folding happens as expected.
>
> diff --git a/target/arm/cpu64.c b/target/arm/cpu64.c
> index 26cf7e6dfa2..af5788dafab 100644
> --- a/target/arm/cpu64.c
> +++ b/target/arm/cpu64.c
> @@ -318,9 +318,11 @@ static void cpu_arm_set_sve(Object *obj, bool
> value, Error **errp)
> {
> ARMCPU *cpu = ARM_CPU(obj);
>
> - if (value && kvm_enabled() && !kvm_arm_sve_supported()) {
> - error_setg(errp, "'sve' feature not supported by KVM on this
> host");
> - return;
> + if (value) {
> + if (kvm_enabled() && !kvm_arm_sve_supported()) {
> + error_setg(errp, "'sve' feature not supported by KVM on
> this host");
> + return;
> + }
> }
>
> FIELD_DP64_IDREG(&cpu->isar, ID_AA64PFR0, SVE, value);
>
> If you prefer keeping your patch, I'm ok, but fixing the condition looks
> better to me (as we already rely on constant folding in other places).
I'm not really a fan of relying on the compiler to fold stuff
away -- it's fragile and there's no guarantee the compiler
will actually do it. In this example it would be really easy
for somebody coming along to tidy this up later to put the
nested if()s back into a single if() condition and reintroduce
the problem, for instance.
I've applied this patch to target-arm.next; thanks for the review.
-- PMM
