[Bug c/40172] [4.5 Regression] Revision 147596 breaks bootstrap
--- Comment #30 from rguenth at gcc dot gnu dot org 2009-05-21 11:05 --- Fixed. -- rguenth at gcc dot gnu dot org changed: What|Removed |Added Status|REOPENED|RESOLVED Resolution||FIXED http://gcc.gnu.org/bugzilla/show_bug.cgi?id=40172
[Bug c/40172] [4.5 Regression] Revision 147596 breaks bootstrap
--- Comment #29 from hjl at gcc dot gnu dot org 2009-05-19 21:24 --- Subject: Bug 40172 Author: hjl Date: Tue May 19 21:24:23 2009 New Revision: 147720 URL: http://gcc.gnu.org/viewcvs?root=gcc&view=rev&rev=147720 Log: 2009-05-19 H.J. Lu Backport from mainline: 2009-05-19 H.J. Lu PR c/40172 * gcc.dg/pr40172-1.c: New. * gcc.dg/pr40172-2.c: Likewise. * gcc.dg/pr40172-3.c: Likewise. Added: branches/gcc-4_4-branch/gcc/testsuite/gcc.dg/pr40172-1.c - copied unchanged from r147719, trunk/gcc/testsuite/gcc.dg/pr40172-1.c branches/gcc-4_4-branch/gcc/testsuite/gcc.dg/pr40172-2.c - copied unchanged from r147719, trunk/gcc/testsuite/gcc.dg/pr40172-2.c branches/gcc-4_4-branch/gcc/testsuite/gcc.dg/pr40172-3.c - copied, changed from r147719, trunk/gcc/testsuite/gcc.dg/pr40172-3.c Modified: branches/gcc-4_4-branch/gcc/testsuite/ChangeLog -- http://gcc.gnu.org/bugzilla/show_bug.cgi?id=40172
[Bug c/40172] [4.5 Regression] Revision 147596 breaks bootstrap
--- Comment #28 from hjl at gcc dot gnu dot org 2009-05-19 21:17 --- Subject: Bug 40172 Author: hjl Date: Tue May 19 21:17:00 2009 New Revision: 147719 URL: http://gcc.gnu.org/viewcvs?root=gcc&view=rev&rev=147719 Log: 2009-05-19 H.J. Lu PR c/40172 * gcc.dg/pr40172.c: Renamed to ... * gcc.dg/pr40172-1.c: This. * gcc.dg/pr40172-2.c: New. * gcc.dg/pr40172-3.c: Likewise. Added: trunk/gcc/testsuite/gcc.dg/pr40172-1.c - copied unchanged from r147718, trunk/gcc/testsuite/gcc.dg/pr40172.c trunk/gcc/testsuite/gcc.dg/pr40172-2.c trunk/gcc/testsuite/gcc.dg/pr40172-3.c Removed: trunk/gcc/testsuite/gcc.dg/pr40172.c Modified: trunk/gcc/testsuite/ChangeLog -- http://gcc.gnu.org/bugzilla/show_bug.cgi?id=40172
[Bug c/40172] [4.5 Regression] Revision 147596 breaks bootstrap
--- Comment #27 from ddaney at caviumnetworks dot com 2009-05-19 20:58 --- Subject: Re: [4.5 Regression] Revision 147596 breaks bootstrap manu at gcc dot gnu dot org wrote: > --- Comment #26 from manu at gcc dot gnu dot org 2009-05-19 20:29 --- > The case in toplev.c cannot be fixed without tracking macro expansions > somehow, > but I wonder why it warns (multiple times!) for this case: > >> ../../trunk/gcc/config/mips/sb1.md:159: error: logical �or� of >> collectively >> exhaustive tests is always true >> ../../trunk/gcc/config/mips/sb1.md:159: error: logical �or� of >> collectively >> exhaustive tests is always true >> ../../trunk/gcc/config/mips/sb1.md:159: error: logical �or� of >> collectively >> exhaustive tests is always true >> ../../trunk/gcc/config/mips/sb1.md:159: error: logical �or� of >> collectively >> exhaustive tests is always true > > David, could you produce a testcase? > It is in insn-attrtab.c, which is machine generated. I since the fatal warning is now disabled, it should be fine. The problem with insn-attrtab.c is that it is generated from the .md files and then includes all the target macros. So for this file you should probably never use -Wlogical-ops as filters that try to eliminate things in macros will fail. The whole file is conceptually one big macro. -- http://gcc.gnu.org/bugzilla/show_bug.cgi?id=40172
[Bug c/40172] [4.5 Regression] Revision 147596 breaks bootstrap
--- Comment #26 from manu at gcc dot gnu dot org 2009-05-19 20:29 --- The case in toplev.c cannot be fixed without tracking macro expansions somehow, but I wonder why it warns (multiple times!) for this case: > ../../trunk/gcc/config/mips/sb1.md:159: error: logical �or� of > collectively > exhaustive tests is always true > ../../trunk/gcc/config/mips/sb1.md:159: error: logical �or� of > collectively > exhaustive tests is always true > ../../trunk/gcc/config/mips/sb1.md:159: error: logical �or� of > collectively > exhaustive tests is always true > ../../trunk/gcc/config/mips/sb1.md:159: error: logical �or� of > collectively > exhaustive tests is always true David, could you produce a testcase? Anyway, moved out of Wextra, so bootstrap should be fixed now. -- http://gcc.gnu.org/bugzilla/show_bug.cgi?id=40172
[Bug c/40172] [4.5 Regression] Revision 147596 breaks bootstrap
--- Comment #25 from manu at gcc dot gnu dot org 2009-05-19 19:29 --- Subject: Bug 40172 Author: manu Date: Tue May 19 19:29:27 2009 New Revision: 147717 URL: http://gcc.gnu.org/viewcvs?root=gcc&view=rev&rev=147717 Log: 2009-05-19 Manuel López-Ibáñez PR c/40172 gcc/ * c.opt (Wlogical-op): Disabled by default. * c-opt (c_common_post_options): Do not enable Wlogical-op with Wextra. * doc/invoke.texi (Wlogical-op): Likewise. testsuite/ * gcc.dg/pr40172.c: Add -Wlogical-op to dg-options. Modified: trunk/gcc/ChangeLog trunk/gcc/c-opts.c trunk/gcc/c.opt trunk/gcc/doc/invoke.texi trunk/gcc/testsuite/ChangeLog trunk/gcc/testsuite/gcc.dg/pr40172.c -- http://gcc.gnu.org/bugzilla/show_bug.cgi?id=40172
[Bug c/40172] [4.5 Regression] Revision 147596 breaks bootstrap
--- Comment #24 from hjl dot tools at gmail dot com 2009-05-19 02:37
---
(In reply to comment #23)
> Subject: Re: [4.5 Regression] Revision 147596 breaks bootstrap
>
> I still feel that if this patch is put in, we eventually will have the same
> discussion, as the compiler gets smarter about flow control and propigation,
This particular warning comes from front-end. I don't think front-end
will do such transformation.
> since semantically:
>
> if (test1)
> {
> if (test2)
> {
> /* ... */
> }
> }
>
> is the same as:
>
> if (test1 && test2)
> {
> /* ... */
> }
>
Gcc can eliminate many dead codes today. I don't think we will ever warn
any of them, if at all.
--
http://gcc.gnu.org/bugzilla/show_bug.cgi?id=40172
[Bug c/40172] [4.5 Regression] Revision 147596 breaks bootstrap
--- Comment #23 from meissner at linux dot vnet dot ibm dot com 2009-05-19
02:33 ---
Subject: Re: [4.5 Regression] Revision 147596 breaks bootstrap
On Tue, May 19, 2009 at 12:38:08AM -, hjl dot tools at gmail dot com wrote:
>
>
> --- Comment #22 from hjl dot tools at gmail dot com 2009-05-19 00:38
> ---
> Another patch is posted at
>
> http://gcc.gnu.org/ml/gcc-patches/2009-05/msg01187.html
>
>
> --
>
>
> http://gcc.gnu.org/bugzilla/show_bug.cgi?id=40172
>
> --- You are receiving this mail because: ---
> You are on the CC list for the bug, or are watching someone who is.
I still feel that if this patch is put in, we eventually will have the same
discussion, as the compiler gets smarter about flow control and propigation,
since semantically:
if (test1)
{
if (test2)
{
/* ... */
}
}
is the same as:
if (test1 && test2)
{
/* ... */
}
--
http://gcc.gnu.org/bugzilla/show_bug.cgi?id=40172
[Bug c/40172] [4.5 Regression] Revision 147596 breaks bootstrap
--- Comment #22 from hjl dot tools at gmail dot com 2009-05-19 00:38 --- Another patch is posted at http://gcc.gnu.org/ml/gcc-patches/2009-05/msg01187.html -- http://gcc.gnu.org/bugzilla/show_bug.cgi?id=40172
[Bug c/40172] [4.5 Regression] Revision 147596 breaks bootstrap
--- Comment #21 from pinskia at gcc dot gnu dot org 2009-05-19 00:02 --- (In reply to comment #16) > Just to chime in, the warning is a useful warning, but the way rs6000 and mips > define FRAME_GROWS_DOWNWARD, the test in toplev.c will never succeed. Yes and that is correct because we don't want that code to be involved at all. The whole if statement becomes false (which is a good thing we can detect it as being always false). > > I can see a couple of ways to fix this: > 1) Revert the patch that moves this warning to -Wextra. I think this is a bad > idea, since the warning does seem to be useful. > > 2) Disable the check in toplev.c. Again, I think this is useful in general, > but as an immediate palative, it can be useful. > > 3) Add a new macro to say not to do the test in #2, and define it in mips and > rs6000. This is doable, but in general it is not a good idea to add new > global > macros like this. This is not a good option at all. a seperate option which I mentioned in an email rather than this bug report. So the current issue here is that we have: if (!DEFINED && x != 0) Where DEFINED is a macro which says (x != 0). This gets expanded as: if (!(x!=0) && x!=0) Obviously to the trained eye this would be a good warning but guess what DEFINED just happens to be based on x, it does not have to be. It would be nice to say only warn for x1 && !x2 if (obviously x1 == x2) : if either x1 or !x2 is from a macro but not both Thanks, Andrew Pinski -- http://gcc.gnu.org/bugzilla/show_bug.cgi?id=40172
[Bug c/40172] [4.5 Regression] Revision 147596 breaks bootstrap
--- Comment #20 from meissner at linux dot vnet dot ibm dot com 2009-05-18 23:48 --- David, as I said in my message, this patch is just papering over the problem. While we might want to install this patch temporarily, to get the mips and rs6000 building again, I think a better solution is to change the circular dependency between FRAME_GROWS_DOWNWARD and flag_stack_protect, so at the point of the test in toplev.c the compiler won't give this warning. H. J. the problem with your patch is it that the compiler is likely to still issue the warning, since it will be discovered by the dataflow or SSA parts of the compiler. -- http://gcc.gnu.org/bugzilla/show_bug.cgi?id=40172
[Bug c/40172] [4.5 Regression] Revision 147596 breaks bootstrap
--- Comment #19 from daney at gcc dot gnu dot org 2009-05-18 23:32 --- Created an attachment (id=17890) --> (http://gcc.gnu.org/bugzilla/attachment.cgi?id=17890&action=view) Proposed fix. I am testing this patch. -- http://gcc.gnu.org/bugzilla/show_bug.cgi?id=40172
[Bug c/40172] [4.5 Regression] Revision 147596 breaks bootstrap
--- Comment #18 from manu at gcc dot gnu dot org 2009-05-18 23:13 ---
The following patch moves the warning out of Wextra. I haven't tested it,
though.
Index: gcc/doc/invoke.texi
===
--- gcc/doc/invoke.texi (revision 147679)
+++ gcc/doc/invoke.texi (working copy)
@@ -2806,7 +2806,6 @@
@gccoptlist{-Wclobbered @gol
-Wempty-body @gol
-Wignored-qualifiers @gol
--Wlogical-op @gol
-Wmissing-field-initializers @gol
-Wmissing-parameter-type @r{(C only)} @gol
-Wold-style-declaration @r{(C only)} @gol
@@ -3793,8 +3792,7 @@
@opindex Wno-logical-op
Warn about suspicious uses of logical operators in expressions.
This includes using logical operators in contexts where a
-bit-wise operator is likely to be expected. This warning is enabled by
-...@option{-wextra}.
+bit-wise operator is likely to be expected.
@item -Waggregate-return
@opindex Waggregate-return
Index: gcc/c-opts.c
===
--- gcc/c-opts.c(revision 147679)
+++ gcc/c-opts.c(working copy)
@@ -1073,8 +1073,6 @@
warn_override_init = extra_warnings;
if (warn_ignored_qualifiers == -1)
warn_ignored_qualifiers = extra_warnings;
- if (warn_logical_op == -1)
-warn_logical_op = extra_warnings;
/* -Wpointer-sign is disabled by default, but it is enabled if any
of -Wall or -pedantic are given. */
--
manu at gcc dot gnu dot org changed:
What|Removed |Added
Last reconfirmed|2009-05-18 21:10:16 |2009-05-18 23:13:34
date||
http://gcc.gnu.org/bugzilla/show_bug.cgi?id=40172
[Bug c/40172] [4.5 Regression] Revision 147596 breaks bootstrap
--- Comment #17 from hjl dot tools at gmail dot com 2009-05-18 23:03
---
(In reply to comment #16)
> Just to chime in, the warning is a useful warning, but the way rs6000 and mips
> define FRAME_GROWS_DOWNWARD, the test in toplev.c will never succeed.
>
> 5) Move FRAME_GROWS_DOWNWARD (and STACK_GROWS_DOWNWARD, etc.) into the target
> structure, and set the field in the target structure from the macro. I tend
> to
Can't we change it to
if (!FRAME_GROWS_DOWNWARD)
{
if (flag_stack_protect)
{
warning (0, "-fstack-protector not supported for this target");
flag_stack_protect = 0;
}
}
with a comment?
--
http://gcc.gnu.org/bugzilla/show_bug.cgi?id=40172
[Bug c/40172] [4.5 Regression] Revision 147596 breaks bootstrap
--- Comment #16 from meissner at linux dot vnet dot ibm dot com 2009-05-18 22:56 --- Just to chime in, the warning is a useful warning, but the way rs6000 and mips define FRAME_GROWS_DOWNWARD, the test in toplev.c will never succeed. I can see a couple of ways to fix this: 1) Revert the patch that moves this warning to -Wextra. I think this is a bad idea, since the warning does seem to be useful. 2) Disable the check in toplev.c. Again, I think this is useful in general, but as an immediate palative, it can be useful. 3) Add a new macro to say not to do the test in #2, and define it in mips and rs6000. This is doable, but in general it is not a good idea to add new global macros like this. 4) Change mips and rs6000 to have a global variable that is what FRAME_GROWS_DOWNWARD should be. This is certainly doable. The test will be tested at runtime, but never invoke the error message. 5) Move FRAME_GROWS_DOWNWARD (and STACK_GROWS_DOWNWARD, etc.) into the target structure, and set the field in the target structure from the macro. I tend to like this (and eventually move backends to set the field directly and get rid of the macros). I tend to like this idea best. -- meissner at linux dot vnet dot ibm dot com changed: What|Removed |Added CC||meissner at linux dot vnet ||dot ibm dot com http://gcc.gnu.org/bugzilla/show_bug.cgi?id=40172
[Bug c/40172] [4.5 Regression] Revision 147596 breaks bootstrap
--- Comment #15 from daney at gcc dot gnu dot org 2009-05-18 22:35 --- And yet another place: ../../trunk/gcc/config/mips/sb1.md:159: error: logical or of collectively exhaustive tests is always true ../../trunk/gcc/config/mips/sb1.md:159: error: logical or of collectively exhaustive tests is always true ../../trunk/gcc/config/mips/sb1.md:159: error: logical or of collectively exhaustive tests is always true ../../trunk/gcc/config/mips/sb1.md:159: error: logical or of collectively exhaustive tests is always true Can we either get the patch reverted, or move it out of -Wextra? -- http://gcc.gnu.org/bugzilla/show_bug.cgi?id=40172
[Bug c/40172] [4.5 Regression] Revision 147596 breaks bootstrap
--- Comment #14 from daney at gcc dot gnu dot org 2009-05-18 21:10 --- For the record: This affects mips64-linux as well. -- daney at gcc dot gnu dot org changed: What|Removed |Added Last reconfirmed|2009-05-17 17:11:44 |2009-05-18 21:10:16 date|| http://gcc.gnu.org/bugzilla/show_bug.cgi?id=40172
[Bug c/40172] [4.5 Regression] Revision 147596 breaks bootstrap
--- Comment #13 from manu at gcc dot gnu dot org 2009-05-17 20:47 --- If GCC does not want to be warned about if (!x && x) then the warning is not useful for GCC. Then take it out of -Wextra. But it is definitely useful for others, and it found a bug in IRA. -- http://gcc.gnu.org/bugzilla/show_bug.cgi?id=40172
[Bug c/40172] [4.5 Regression] Revision 147596 breaks bootstrap
--- Comment #12 from rguenth at gcc dot gnu dot org 2009-05-17 20:20 --- I don't think this warning is particularly useful if it breaks even GCC. -- http://gcc.gnu.org/bugzilla/show_bug.cgi?id=40172
[Bug c/40172] [4.5 Regression] Revision 147596 breaks bootstrap
--- Comment #11 from pinskia at gcc dot gnu dot org 2009-05-17 20:17 --- Maybe the warning is correct but with macros like this, it is hard to avoid the warning really. -- http://gcc.gnu.org/bugzilla/show_bug.cgi?id=40172
[Bug c/40172] [4.5 Regression] Revision 147596 breaks bootstrap
--- Comment #10 from hjl dot tools at gmail dot com 2009-05-17 20:14 --- (In reply to comment #9) > Still does not fix the PPC-darwin bootstrap: > cc1: warnings being treated as errors > /Users/regress/tbox/svn-gcc/gcc/toplev.c: In function 'process_options': > /Users/regress/tbox/svn-gcc/gcc/toplev.c:2043: error: logical 'and' of > mutually > exclusive tests is always false > > if (!FRAME_GROWS_DOWNWARD && flag_stack_protect) > This warning may be correct: rs6000.h:#define FRAME_GROWS_DOWNWARD (flag_stack_protect != 0) -- http://gcc.gnu.org/bugzilla/show_bug.cgi?id=40172
[Bug c/40172] [4.5 Regression] Revision 147596 breaks bootstrap
--- Comment #9 from pinskia at gcc dot gnu dot org 2009-05-17 20:02 --- Still does not fix the PPC-darwin bootstrap: cc1: warnings being treated as errors /Users/regress/tbox/svn-gcc/gcc/toplev.c: In function 'process_options': /Users/regress/tbox/svn-gcc/gcc/toplev.c:2043: error: logical 'and' of mutually exclusive tests is always false if (!FRAME_GROWS_DOWNWARD && flag_stack_protect) -- pinskia at gcc dot gnu dot org changed: What|Removed |Added Status|RESOLVED|REOPENED Resolution|FIXED | http://gcc.gnu.org/bugzilla/show_bug.cgi?id=40172
[Bug c/40172] [4.5 Regression] Revision 147596 breaks bootstrap
--- Comment #8 from hjl dot tools at gmail dot com 2009-05-17 19:25 --- Fixed. -- hjl dot tools at gmail dot com changed: What|Removed |Added Status|NEW |RESOLVED Resolution||FIXED http://gcc.gnu.org/bugzilla/show_bug.cgi?id=40172
[Bug c/40172] [4.5 Regression] Revision 147596 breaks bootstrap
--- Comment #7 from hjl at gcc dot gnu dot org 2009-05-17 18:37 --- Subject: Bug 40172 Author: hjl Date: Sun May 17 18:36:44 2009 New Revision: 147639 URL: http://gcc.gnu.org/viewcvs?root=gcc&view=rev&rev=147639 Log: gcc/ 2009-05-17 Manuel López-Ibáñez PR c/40172 * c-common.c (warn_logical_operator): Don't warn if one of expression isn't always true or false. gcc/testscase/ 2009-05-17 H.J. Lu PR c/40172 * gcc.dg/pr40172.c: New. Added: trunk/gcc/testsuite/gcc.dg/pr40172.c Modified: trunk/gcc/ChangeLog trunk/gcc/c-common.c trunk/gcc/testsuite/ChangeLog -- http://gcc.gnu.org/bugzilla/show_bug.cgi?id=40172
[Bug c/40172] [4.5 Regression] Revision 147596 breaks bootstrap
--- Comment #6 from hjl dot tools at gmail dot com 2009-05-17 18:05 --- (In reply to comment #5) > This patch seems to fix the problem and still warn for the interesting cases. > Could you all test it in your targets? I can only test > x86-64-unknown-linux-gnu. > The testcase in comment #3 isn't target specific. You should include it in your patch. -- http://gcc.gnu.org/bugzilla/show_bug.cgi?id=40172
[Bug c/40172] [4.5 Regression] Revision 147596 breaks bootstrap
--- Comment #5 from manu at gcc dot gnu dot org 2009-05-17 17:11 --- This patch seems to fix the problem and still warn for the interesting cases. Could you all test it in your targets? I can only test x86-64-unknown-linux-gnu. Index: gcc/c-common.c === --- gcc/c-common.c (revision 147636) +++ gcc/c-common.c (working copy) @@ -1782,14 +1782,12 @@ warn_logical_operator (location_t locati again at the end. */ if (or_op) in0_p = !in0_p, in1_p = !in1_p; /* If both expressions are the same, if we can merge the ranges, and we - can build the range test, return it or it inverted. If one of the - ranges is always true or always false, consider it to be the same - expression as the other. */ - if ((lhs == 0 || rhs == 0 || operand_equal_p (lhs, rhs, 0)) + can build the range test, return it or it inverted. */ + if (lhs && rhs && operand_equal_p (lhs, rhs, 0) && merge_ranges (&in_p, &low, &high, in0_p, low0, high0, in1_p, low1, high1) && 0 != (tem = build_range_check (type, lhs != 0 ? lhs : rhs != 0 ? rhs : integer_zero_node, -- manu at gcc dot gnu dot org changed: What|Removed |Added Status|UNCONFIRMED |NEW Ever Confirmed|0 |1 Last reconfirmed|-00-00 00:00:00 |2009-05-17 17:11:44 date|| http://gcc.gnu.org/bugzilla/show_bug.cgi?id=40172
[Bug c/40172] [4.5 Regression] Revision 147596 breaks bootstrap
--- Comment #4 from danglin at gcc dot gnu dot org 2009-05-17 03:08 --- Also breaks bootstrap on hppa. -- danglin at gcc dot gnu dot org changed: What|Removed |Added CC||danglin at gcc dot gnu dot ||org http://gcc.gnu.org/bugzilla/show_bug.cgi?id=40172
[Bug c/40172] [4.5 Regression] Revision 147596 breaks bootstrap
--- Comment #3 from hjl dot tools at gmail dot com 2009-05-16 16:12 ---
Here is a testcase for more problems:
[...@gnu-12 prev-gcc]$ cat /tmp/x.i
struct rtx_def;
typedef struct rtx_def *rtx;
extern int foo;
extern int bar;
extern int xxx;
int
test (void)
{
if (((rtx) 0 != (rtx) 0) && xxx ? foo : bar)
return 1;
else if ((foo & 0) && xxx)
return 2;
else if (foo & 0)
return 3;
else if (0 && xxx)
return 4;
else if (0)
return 5;
else
return 0;
}
[...@gnu-12 prev-gcc]$ ./xgcc -B./ -Wall -W -O2 -Werror -S /tmp/x.i
cc1: warnings being treated as errors
/tmp/x.i: In function test:
/tmp/x.i:11: error: logical and of mutually exclusive tests is always false
/tmp/x.i:13: error: logical and of mutually exclusive tests is always false
[...@gnu-12 prev-gcc]$
--
http://gcc.gnu.org/bugzilla/show_bug.cgi?id=40172
[Bug c/40172] [4.5 Regression] Revision 147596 breaks bootstrap
--- Comment #2 from dominiq at lps dot ens dot fr 2009-05-16 15:31 --- This has also been reported at http://gcc.gnu.org/ml/gcc-patches/2009-05/msg00974.html -- http://gcc.gnu.org/bugzilla/show_bug.cgi?id=40172
