diff mbox

Use math_narrow_eval more consistently [committed]

Message ID 1443117604.8687.31.camel@ubuntu-sellcey
State New
Headers show

Commit Message

Steve Ellcey Sept. 24, 2015, 6 p.m. UTC 
On Wed, 2015-09-23 at 18:16 +0000, Joseph Myers wrote:

> 
> Tested for x86_64, x86, mips64 and powerpc.  Committed.
> 
> 2015-09-23  Joseph Myers  <joseph@codesourcery.com>
> 
> 	* math/s_nexttowardf.c (__nexttowardf): Use math_narrow_eval.
> 	* stdlib/strtod_l.c: Include <math_private.h>.
> 	(overflow_value): Use math_narrow_eval.
> 	(underflow_value): Likewise.

Joseph,

This patch is causing a build failure on mips16 hard-float builds.  It
dies with:

In file included from strtod_l.c:60:0,
                 from strtof_l.c:45:
../sysdeps/mips/math_private.h: In function 'libc_fesetenv_mips':
../sysdeps/mips/math_private.h:109:17: error: variable 'cw' set but not
used [-Werror=unused-but-set-variable]
   fpu_control_t cw;
                 ^
cc1: all warnings being treated as errors


I have created this patch to fix it, does this look like the right
change to you?

Steve Ellcey
sellcey@imgtec.com



2015-09-24  Steve Ellcey  <sellcey@imgtec.com>

 	* sysdeps/mips/math_private.h (libc_fesetenv_mips): Mark cw as unused.

Comments

Joseph Myers Sept. 24, 2015, 6:08 p.m. UTC | #1 
On Thu, 24 Sep 2015, Steve Ellcey wrote:

> This patch is causing a build failure on mips16 hard-float builds.  It
> dies with:

I don't see why this would have anything to do with my patch (it's 
including math_private.h in more places, but the same set-but-not-used 
condition ought to apply to all of them), but your patch seems right for 
this case (a variable that is deliberately set but not used).
Steve Ellcey Sept. 24, 2015, 10:33 p.m. UTC | #2 
On Thu, 2015-09-24 at 18:08 +0000, Joseph Myers wrote:
> On Thu, 24 Sep 2015, Steve Ellcey wrote:
> 
> > This patch is causing a build failure on mips16 hard-float builds.  It
> > dies with:
> 
> I don't see why this would have anything to do with my patch (it's 
> including math_private.h in more places, but the same set-but-not-used 
> condition ought to apply to all of them), but your patch seems right for 
> this case (a variable that is deliberately set but not used).

It looks like math_private.h was never included in a file that got built
for mips16 before.  If I build an earlier version of glibc and put a
deliberate syntax error in libc_fesetenv_mips that will only trigger
when building for mips16, it never triggers.

All the includes of math_private.h used to be in the ieee754/* and
math/* files.  It looks like none of the ieee754 routines are compiled
for mips16 hard-float glibc and for the math routines we have a Makefile
fragment in sysdeps/mips/mips32/mips16/fpu/Makefile that contains:

ifeq ($(subdir),math)
sysdep-CFLAGS += -mno-mips16
endif

Now we have stdlib/strtod_l.c including math_private.h and this appears
to be the only file outside of ieee754, math, or platform specific fpu
directories to include this header.

Steve Ellcey
sellcey@imgtec.com
Joseph Myers Sept. 24, 2015, 11:03 p.m. UTC | #3 
On Thu, 24 Sep 2015, Steve Ellcey wrote:

> It looks like math_private.h was never included in a file that got built
> for mips16 before.  If I build an earlier version of glibc and put a
> deliberate syntax error in libc_fesetenv_mips that will only trigger
> when building for mips16, it never triggers.

Thanks for the explanation.  Your patch is OK.
Joseph Myers Sept. 24, 2015, 11:36 p.m. UTC | #4 
You appear to have missed the ChangeLog update in your commit; please add 
the missing ChangeLog entry.  We do not have automatic ChangeLog 
generation implemented yet, so commits still need to include a change to 
the ChangeLog file.
Steve Ellcey Sept. 24, 2015, 11:37 p.m. UTC | #5 
On Thu, 2015-09-24 at 23:36 +0000, Joseph Myers wrote:
> You appear to have missed the ChangeLog update in your commit; please add 
> the missing ChangeLog entry.  We do not have automatic ChangeLog 
> generation implemented yet, so commits still need to include a change to 
> the ChangeLog file.

Yes, I did miss that.  I will add it now.

Steve Ellcey
sellcey@imgtec.com
diff mbox

Patch

diff --git a/sysdeps/mips/math_private.h b/sysdeps/mips/math_private.h
index baa5f28..3db0273 100644
--- a/sysdeps/mips/math_private.h
+++ b/sysdeps/mips/math_private.h
@@ -106,7 +106,7 @@  libc_feholdexcept_setround_mips (fenv_t *envp, int round)
 static __always_inline void
 libc_fesetenv_mips (fenv_t *envp)
 {
-  fpu_control_t cw;
+  fpu_control_t cw __attribute__ ((unused));
 
   /* Read current state to flush fpu pipeline.  */
   _FPU_GETCW (cw);