[Bug bootstrap/92433] [10 regression] r276645 breaks bootstrap on powerpc
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=92433 Jakub Jelinek changed: What|Removed |Added Status|UNCONFIRMED |RESOLVED Resolution|--- |FIXED Assignee|unassigned at gcc dot gnu.org |jakub at gcc dot gnu.org --- Comment #8 from Jakub Jelinek --- Should be fixed now.
[Bug bootstrap/92433] [10 regression] r276645 breaks bootstrap on powerpc
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=92433 --- Comment #7 from Jakub Jelinek --- Author: jakub Date: Mon Nov 11 20:05:49 2019 New Revision: 278066 URL: https://gcc.gnu.org/viewcvs?rev=278066=gcc=rev Log: PR bootstrap/92433 * config/rs6000/rs6000-c.c (altivec_build_resolved_builtin): Guard ALTIVEC_BUILTIN_VEC_VCMPGE_P argument swapping with n == 3 check. Use std::swap. Modified: trunk/gcc/ChangeLog trunk/gcc/config/rs6000/rs6000-c.c
[Bug bootstrap/92433] [10 regression] r276645 breaks bootstrap on powerpc
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=92433 --- Comment #6 from Jakub Jelinek --- (In reply to Segher Boessenkool from comment #5) > if (y) > { > gcc_assert (n == 3); > std::swap (arg_type[1], arg_type[2]); > } > > > ? gcc_assert will work too unless the host compiler is older than 4.5 (or is clang), but I think we don't really make -Werror on in that case, at least not by default (only in second stage that is built with trunk gcc already). So, I can bootstrap/regtest: 2019-11-11 Jakub Jelinek PR bootstrap/92433 * config/rs6000/rs6000-c.c (altivec_build_resolved_builtin): Add assertion that n is 3 for ALTIVEC_BUILTIN_VEC_VCMPGE_P. Use std::swap. --- gcc/config/rs6000/rs6000-c.c.jj 2019-08-27 12:26:30.115019661 +0200 +++ gcc/config/rs6000/rs6000-c.c2019-11-11 15:03:57.299102261 +0100 @@ -6080,10 +6080,10 @@ altivec_build_resolved_builtin (tree *ar && desc->overloaded_code != ALTIVEC_BUILTIN_VCMPGEFP_P && desc->overloaded_code != VSX_BUILTIN_XVCMPGEDP_P) { - tree t; - t = args[2], args[2] = args[1], args[1] = t; - t = arg_type[2], arg_type[2] = arg_type[1], arg_type[1] = t; - + gcc_assert (n == 3); + std::swap (args[1], args[2]); + std::swap (arg_type[1], arg_type[2]); + args[0] = fold_build2 (BIT_XOR_EXPR, TREE_TYPE (args[0]), args[0], build_int_cst (NULL_TREE, 2)); } too if you prefer it that way. Richi said on IRC he prefers a rs6000-c.c change in this case.
[Bug bootstrap/92433] [10 regression] r276645 breaks bootstrap on powerpc
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=92433 --- Comment #5 from Segher Boessenkool --- if (y) { gcc_assert (n == 3); std::swap (arg_type[1], arg_type[2]); } ?
[Bug bootstrap/92433] [10 regression] r276645 breaks bootstrap on powerpc
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=92433 --- Comment #4 from Jakub Jelinek --- Reduced testcase that results in the warning also on x86_64-linux with -O2 -Wall: struct S { void *p; struct S *q; }; void bar (int, ...); void foo (struct S *x, int n, int y) { void *arg_type[3]; for (int i = 0; i < n; i++) { arg_type[i] = x->p; x = x->q; } if (y) { void *t = arg_type[2]; arg_type[2] = arg_type[1]; arg_type[1] = t; } switch (n) { case 0: bar (0); break; case 1: bar (1, arg_type[0]); break; case 2: bar (2, arg_type[0], arg_type[1]); break; case 3: bar (3, arg_type[0], arg_type[1], arg_type[2]); break; } } Using if (n == 3 && y) makes the warning go away. The compiler indeed doesn't know if y implies n == 3, although the caller guarantees it. So, we could do: --- gcc/config/rs6000/rs6000-c.c.jj 2019-08-27 12:26:30.115019661 +0200 +++ gcc/config/rs6000/rs6000-c.c2019-11-11 10:12:00.954282097 +0100 @@ -6076,13 +6076,13 @@ altivec_build_resolved_builtin (tree *ar condition (LT vs. EQ, which is recognizable by bit 1 of the first argument) is reversed. Patch the arguments here before building the resolved CALL_EXPR. */ - if (desc->code == ALTIVEC_BUILTIN_VEC_VCMPGE_P + if (n == 3 + && desc->code == ALTIVEC_BUILTIN_VEC_VCMPGE_P && desc->overloaded_code != ALTIVEC_BUILTIN_VCMPGEFP_P && desc->overloaded_code != VSX_BUILTIN_XVCMPGEDP_P) { - tree t; - t = args[2], args[2] = args[1], args[1] = t; - t = arg_type[2], arg_type[2] = arg_type[1], arg_type[1] = t; + std::swap (args[1], args[2]); + std::swap (arg_type[1], arg_type[2]); args[0] = fold_build2 (BIT_XOR_EXPR, TREE_TYPE (args[0]), args[0], build_int_cst (NULL_TREE, 2)); to make it clear to the compiler that VCMPGE_P has 3 arguments and that it is actually safe to use all of them, or of course we could clear arg_type first. I don't find the may be used uninitialized warning wrong (at least not completely) when the compiler sees a possible code path where it is uninitialized (desc->code == ALTIVEC_BULTIN_VEC_VCMPGE_P with n < 3).
[Bug bootstrap/92433] [10 regression] r276645 breaks bootstrap on powerpc
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=92433 --- Comment #3 from Jakub Jelinek --- (In reply to Andreas Schwab from comment #2) > That doesn't fix the spurious warning, though. Sure, it doesn't, that was just a random comment when I have spent a minute looking at the code in question.
[Bug bootstrap/92433] [10 regression] r276645 breaks bootstrap on powerpc
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=92433 Richard Biener changed: What|Removed |Added CC||rguenth at gcc dot gnu.org Blocks||24639 Target Milestone|--- |10.0 Referenced Bugs: https://gcc.gnu.org/bugzilla/show_bug.cgi?id=24639 [Bug 24639] [meta-bug] bug to track all Wuninitialized issues
[Bug bootstrap/92433] [10 regression] r276645 breaks bootstrap on powerpc
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=92433 --- Comment #2 from Andreas Schwab --- That doesn't fix the spurious warning, though.
[Bug bootstrap/92433] [10 regression] r276645 breaks bootstrap on powerpc
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=92433 Jakub Jelinek changed: What|Removed |Added CC||jakub at gcc dot gnu.org, ||segher at gcc dot gnu.org --- Comment #1 from Jakub Jelinek --- The: tree t; t = args[2], args[2] = args[1], args[1] = t; t = arg_type[2], arg_type[2] = arg_type[1], arg_type[1] = t; would be much clearer if written as: std::swap (args[1], args[2]); std::swap (arg_type[1], arg_type[2]);