Re: [PATCH v2] target/i386: Remove compilation errors when -Werror=maybe-uninitialized

[Bernhard Beschow]

Am 22. Dezember 2022 09:01:34 UTC schrieb "Philippe Mathieu-Daudé" 
<philmd@linaro.org>:
>On 22/12/22 09:18, Paolo Bonzini wrote:
>> On 12/21/22 17:36, Eric Auger wrote:
>>> To avoid compilation errors when -Werror=maybe-uninitialized is used,
>>> replace 'case 3' by 'default'.
>>> 
>>> Otherwise we get:
>>> 
>>> ../target/i386/ops_sse.h: In function ‘helper_vpermdq_ymm’:
>>> ../target/i386/ops_sse.h:2495:13: error: ‘r3’ may be used
>>> uninitialized in this function [-Werror=maybe-uninitialized]
>>>     2495 |     d->Q(3) = r3;
>>>          |     ~~~~~~~~^~~~
>>> ../target/i386/ops_sse.h:2494:13: error: ‘r2’ may be used
>>> uninitialized in this function [-Werror=maybe-uninitialized]
>>>     2494 |     d->Q(2) = r2;
>>>          |     ~~~~~~~~^~~~
>>> ../target/i386/ops_sse.h:2493:13: error: ‘r1’ may be used
>>> uninitialized in this function [-Werror=maybe-uninitialized]
>>>     2493 |     d->Q(1) = r1;
>>>          |     ~~~~~~~~^~~~
>>> ../target/i386/ops_sse.h:2492:13: error: ‘r0’ may be used
>>> uninitialized in this function [-Werror=maybe-uninitialized]
>>>     2492 |     d->Q(0) = r0;
>>>          |     ~~~~~~~~^~~~
>
>With what compiler? Is that a supported one?
>
>>> Signed-off-by: Eric Auger <eric.auger@redhat.com>
>>> Suggested-by: Stefan Weil <sw@weilnetz.de>
>>> Fixes: 790684776861 ("target/i386: reimplement 0x0f 0x3a, add AVX")
>>> ---
>>>   target/i386/ops_sse.h | 4 ++--
>>>   1 file changed, 2 insertions(+), 2 deletions(-)
>>> 
>>> diff --git a/target/i386/ops_sse.h b/target/i386/ops_sse.h
>>> index 3cbc36a59d..c442c8c10c 100644
>>> --- a/target/i386/ops_sse.h
>>> +++ b/target/i386/ops_sse.h
>>> @@ -2466,7 +2466,7 @@ void helper_vpermdq_ymm(Reg *d, Reg *v, Reg *s, 
>>> uint32_t order)
>>>           r0 = s->Q(0);
>>>           r1 = s->Q(1);
>>>           break;
>>> -    case 3:
>>> +    default:
>>>           r0 = s->Q(2);
>>>           r1 = s->Q(3);
>>>           break;
>>> @@ -2484,7 +2484,7 @@ void helper_vpermdq_ymm(Reg *d, Reg *v, Reg *s, 
>>> uint32_t order)
>>>           r2 = s->Q(0);
>>>           r3 = s->Q(1);
>>>           break;
>>> -    case 3:
>>> +    default:
>>>           r2 = s->Q(2);
>>>           r3 = s->Q(3);
>>>           break;
>> 
>> Queued, but this compiler sucks. :)
>
>Can't we simply add a dumb 'default' case? So when reviewing we don't
>have to evaluate 'default' means 3 here.

I agree. At least it deserves a comment to preserve the original intention.

Best regards,
Bernhard

>-- >8 --
>--- a/target/i386/ops_sse.h
>+++ b/target/i386/ops_sse.h
>@@ -2470,6 +2470,8 @@ void helper_vpermdq_ymm(Reg *d, Reg *v, Reg *s, uint32_t 
>order)
>         r0 = s->Q(2);
>         r1 = s->Q(3);
>         break;
>+    default:
>+        qemu_build_not_reached();
>     }
>     switch ((order >> 4) & 3) {
>     case 0:
>@@ -2488,6 +2490,8 @@ void helper_vpermdq_ymm(Reg *d, Reg *v, Reg *s, uint32_t 
>order)
>         r2 = s->Q(2);
>         r3 = s->Q(3);
>         break;
>+    default:
>+        qemu_build_not_reached();
>     }
>     d->Q(0) = r0;
>     d->Q(1) = r1;
>---
>