Hi All, On 12/22/22 12:09, Daniel P. Berrangé wrote: > On Thu, Dec 22, 2022 at 11:07:31AM +0100, Eric Auger wrote: >> Hi Philippe, >> >> On 12/22/22 10:01, Philippe Mathieu-Daudé wrote: >>> 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? >> https://lore.kernel.org/qemu-devel/3aab489e-9d90-c1ad-0b6b-b2b5d80db...@redhat.com/ >>>>> Signed-off-by: Eric Auger <eric.au...@redhat.com> >>>>> Suggested-by: Stefan Weil <s...@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. >>> >>> -- >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(); >>> } >> I guess this won't fix the fact r0, r1, r2, r3 are not initialized, will it? > This ultimately expands to assert() and the compiler should see that it > terminates the control flow at this point, so shouldn't have a reason > to warn.
OK so with qemu_build_not_reached(); I get /home/augere/UPSTREAM/qemu/include/qemu/osdep.h:184:35: error: call to ‘qemu_build_not_reached_always’ declared with attribute error: code path is reachable 184 | #define qemu_build_not_reached() qemu_build_not_reached_always() | ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ However with g_assert_not_reached(), it does not complain and errors are removed. So I will respin with g_assert_not_reached() if nobody advises me against that. Thanks Eric > > > With regards, > Daniel