Patchwork [BUILDROBOT] Fix mmix (unused variable)

login
register
mail settings
Submitter Jan-Benedict Glaw
Date July 18, 2014, 2:01 a.m.
Message ID <20140718020136.GT21544@lug-owl.de>
Download mbox | patch
Permalink /patch/371293/
State New
Headers show

Comments

Jan-Benedict Glaw - July 18, 2014, 2:01 a.m.
Hi!

As a leftover of r210931, an unused variable resulted in:

 g++ -c   -g -O2 -DIN_GCC  -DCROSS_DIRECTORY_STRUCTURE  -fno-exceptions -fno-rtti -fasynchronous-unwind-tables -W -Wall -Wno-narrowing -Wwrite-strings -Wcast-qual -Wmissing-format-attribute -Woverloaded-virtual -pedantic -Wno-long-long -Wno-variadic-macros -Wno-overlength-strings -Werror -fno-common  -DHAVE_CONFIG_H -I. -I. -I../../../gcc/gcc -I../../../gcc/gcc/. -I../../../gcc/gcc/../include -I../../../gcc/gcc/../libcpp/include -I/opt/cfarm/mpc/include  -I../../../gcc/gcc/../libdecnumber -I../../../gcc/gcc/../libdecnumber/dpd -I../libdecnumber -I../../../gcc/gcc/../libbacktrace    -o mmix.o -MT mmix.o -MMD -MP -MF ./.deps/mmix.TPo ../../../gcc/gcc/config/mmix/mmix.c
../../../gcc/gcc/config/mmix/mmix.c: In function ‘int64_t mmix_intval(const_rtx)’:
../../../gcc/gcc/config/mmix/mmix.c:2694:12: error: unused variable ‘retval’ [-Werror=unused-variable]
   uint64_t retval;
            ^
cc1plus: all warnings being treated as errors
make[2]: *** [mmix.o] Error 1


(See eg.  http://toolchain.lug-owl.de/buildbot/show_build_details.php?id=305352)



Committed as obvious, fixed like this:


2014-07-18  Jan-Benedict Glaw  <jbglaw@lug-owl.de>
    
	* config/mmix/mmix.c (mmix_intval): Drop unused automatic variable.
Hans-Peter Nilsson - July 18, 2014, 7:10 a.m.
On Fri, 18 Jul 2014, Jan-Benedict Glaw wrote:
> Hi!
>
> As a leftover of r210931, an unused variable resulted in:
>
>  g++ -c   -g -O2 -DIN_GCC  -DCROSS_DIRECTORY_STRUCTURE  -fno-exceptions -fno-rtti -fasynchronous-unwind-tables -W -Wall -Wno-narrowing -Wwrite-strings -Wcast-qual -Wmissing-format-attribute -Woverloaded-virtual -pedantic -Wno-long-long -Wno-variadic-macros -Wno-overlength-strings -Werror -fno-common  -DHAVE_CONFIG_H -I. -I. -I../../../gcc/gcc -I../../../gcc/gcc/. -I../../../gcc/gcc/../include -I../../../gcc/gcc/../libcpp/include -I/opt/cfarm/mpc/include  -I../../../gcc/gcc/../libdecnumber -I../../../gcc/gcc/../libdecnumber/dpd -I../libdecnumber -I../../../gcc/gcc/../libbacktrace    -o mmix.o -MT mmix.o -MMD -MP -MF ./.deps/mmix.TPo ../../../gcc/gcc/config/mmix/mmix.c
> ../../../gcc/gcc/config/mmix/mmix.c: In function ?int64_t mmix_intval(const_rtx)?:
> ../../../gcc/gcc/config/mmix/mmix.c:2694:12: error: unused variable ?retval? [-Werror=unused-variable]
>    uint64_t retval;
>             ^
> cc1plus: all warnings being treated as errors

Weird that I haven't seen this with a host g++ >= 4.7 (4.7.2-2);
I've certainly built post-r210931 (post-2014-05-26) for example
r212486, but obviously so, thanks.

I see the warning in my logs but -Werror wasn't isn't used and I
didn't pay attention. Did something change more recently re:
building with -Werror?

brgds, H-P
Jan-Benedict Glaw - July 18, 2014, 12:44 p.m.
On Fri, 2014-07-18 03:10:09 -0400, Hans-Peter Nilsson <hp@bitrange.com> wrote:
> On Fri, 18 Jul 2014, Jan-Benedict Glaw wrote:
> > As a leftover of r210931, an unused variable resulted in:
> >
> >  g++ -c   -g -O2 -DIN_GCC  -DCROSS_DIRECTORY_STRUCTURE  -fno-exceptions -fno-rtti -fasynchronous-unwind-tables -W -Wall -Wno-narrowing -Wwrite-strings -Wcast-qual -Wmissing-format-attribute -Woverloaded-virtual -pedantic -Wno-long-long -Wno-variadic-macros -Wno-overlength-strings -Werror -fno-common  -DHAVE_CONFIG_H -I. -I. -I../../../gcc/gcc -I../../../gcc/gcc/. -I../../../gcc/gcc/../include -I../../../gcc/gcc/../libcpp/include -I/opt/cfarm/mpc/include  -I../../../gcc/gcc/../libdecnumber -I../../../gcc/gcc/../libdecnumber/dpd -I../libdecnumber -I../../../gcc/gcc/../libbacktrace    -o mmix.o -MT mmix.o -MMD -MP -MF ./.deps/mmix.TPo ../../../gcc/gcc/config/mmix/mmix.c
> > ../../../gcc/gcc/config/mmix/mmix.c: In function ?int64_t mmix_intval(const_rtx)?:
> > ../../../gcc/gcc/config/mmix/mmix.c:2694:12: error: unused variable ?retval? [-Werror=unused-variable]
> >    uint64_t retval;
> >             ^
> > cc1plus: all warnings being treated as errors
> 
> Weird that I haven't seen this with a host g++ >= 4.7 (4.7.2-2);
> I've certainly built post-r210931 (post-2014-05-26) for example
> r212486, but obviously so, thanks.
> 
> I see the warning in my logs but -Werror wasn't isn't used and I
> didn't pay attention. Did something change more recently re:
> building with -Werror?

This was a build using GCC's ./contrib/config-list.mk to do the build.
It passes --enable-werror-always to top-level `configure', this is
where the -Werror comes from.

MfG, JBG
Hans-Peter Nilsson - July 19, 2014, 12:36 a.m.
On Fri, 18 Jul 2014, Jan-Benedict Glaw wrote:
> This was a build using GCC's ./contrib/config-list.mk to do the build.
> It passes --enable-werror-always to top-level `configure', this is
> where the -Werror comes from.

Aha.  Looks like it's of more use than theoretical pain; sounds
like this should (effectively) be the default for *non-releases*
when cross-compiling, with the possibility to override
per-target.  Agreement?  Anyone against?  1/2 :)

It should be per-target because there *may* be port-specific
constructs warned about by buggy previous-but-not-ancient
gcc-versions, where working around the warnings would cause
unwanted obfuscation.  (IIRC gdb does something like this.)

Is that the reason it's not the default, that there are such
constructs in the non-port-specific parts?  But then that would
have already been noticed through use of the config-list.mk, no?

brgds, H-P
Jan-Benedict Glaw - July 19, 2014, 4:57 p.m.
On Fri, 2014-07-18 20:36:20 -0400, Hans-Peter Nilsson <hp@bitrange.com> wrote:
> On Fri, 18 Jul 2014, Jan-Benedict Glaw wrote:
> > This was a build using GCC's ./contrib/config-list.mk to do the build.
> > It passes --enable-werror-always to top-level `configure', this is
> > where the -Werror comes from.
> 
> Aha.  Looks like it's of more use than theoretical pain; sounds
> like this should (effectively) be the default for *non-releases*
> when cross-compiling, with the possibility to override
> per-target.  Agreement?  Anyone against?  1/2 :)

I'm all for it.

> It should be per-target because there *may* be port-specific
> constructs warned about by buggy previous-but-not-ancient
> gcc-versions, where working around the warnings would cause
> unwanted obfuscation.  (IIRC gdb does something like this.)

For example, the PDP11 target has an unresoved warning:

http://toolchain.lug-owl.de/buildbot/show_build_details.php?id=306062

g++ -c   -g -O2 -DIN_GCC  -DCROSS_DIRECTORY_STRUCTURE  -fno-exceptions -fno-rtti -fasynchronous-unwind-tables -W -Wall -Wno-narrowing -Wwrite-strings -Wcast-qual -Wmissing-format-attribute -Woverloaded-virtual -pedantic -Wno-long-long -Wno-variadic-macros -Wno-overlength-strings -Werror -fno-common  -DHAVE_CONFIG_H -I. -I. -I../../../gcc/gcc -I../../../gcc/gcc/. -I../../../gcc/gcc/../include -I../../../gcc/gcc/../libcpp/include -I/opt/cfarm/mpc/include  -I../../../gcc/gcc/../libdecnumber -I../../../gcc/gcc/../libdecnumber/dpd -I../libdecnumber -I../../../gcc/gcc/../libbacktrace    -o cfgexpand.o -MT cfgexpand.o -MMD -MP -MF ./.deps/cfgexpand.TPo ../../../gcc/gcc/cfgexpand.c
../../../gcc/gcc/cfgexpand.c: In function ‘basic_block_def* expand_gimple_cond(basic_block, gimple)’:
../../../gcc/gcc/cfgexpand.c:2100:65: error: comparison is always true due to limited range of data type [-Werror=type-limits]
    else if (BRANCH_COST (optimize_insn_for_speed_p (), false) < 4)
                                                                 ^
cc1plus: all warnings being treated as errors
make[2]: *** [cfgexpand.o] Error 1


ISTR that I brought this up on the list, but there wasn't a final
solution about how to "fix" that; the warning is kind of questionable
in this context, but in itself is correct.

> Is that the reason it's not the default, that there are such
> constructs in the non-port-specific parts?  But then that would
> have already been noticed through use of the config-list.mk, no?

Don't know :)

MfG, JBG
Richard Guenther - July 22, 2014, 12:57 p.m.
On Fri, 18 Jul 2014, Hans-Peter Nilsson wrote:

> On Fri, 18 Jul 2014, Jan-Benedict Glaw wrote:
> > This was a build using GCC's ./contrib/config-list.mk to do the build.
> > It passes --enable-werror-always to top-level `configure', this is
> > where the -Werror comes from.
> 
> Aha.  Looks like it's of more use than theoretical pain; sounds
> like this should (effectively) be the default for *non-releases*
> when cross-compiling, with the possibility to override
> per-target.  Agreement?  Anyone against?  1/2 :)
> 
> It should be per-target because there *may* be port-specific
> constructs warned about by buggy previous-but-not-ancient
> gcc-versions, where working around the warnings would cause
> unwanted obfuscation.  (IIRC gdb does something like this.)
> 
> Is that the reason it's not the default, that there are such
> constructs in the non-port-specific parts?  But then that would
> have already been noticed through use of the config-list.mk, no?

The reason it's not the default is because the warnings are coming
from the host compiler which may be fairly old or even broken.

If we want it to make the default the we should restrict it
based on the host compiler (version).

Richard.

Patch

diff --git a/gcc/config/mmix/mmix.c b/gcc/config/mmix/mmix.c
index e0b8ce7..b9edc3c 100644
--- a/gcc/config/mmix/mmix.c
+++ b/gcc/config/mmix/mmix.c
@@ -2691,8 +2691,6 @@  mmix_output_condition (FILE *stream, const_rtx x, int reversed)
 int64_t
 mmix_intval (const_rtx x)
 {
-  uint64_t retval;
-
   if (GET_CODE (x) == CONST_INT)
     return INTVAL (x);