bug-gnulib
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [PATCH] ceil, floor: avoid spurious failure with icc


From: Eric Blake
Subject: Re: [PATCH] ceil, floor: avoid spurious failure with icc
Date: Fri, 05 Nov 2010 17:31:45 -0600
User-agent: Mozilla/5.0 (X11; U; Linux x86_64; en-US; rv:1.9.2.12) Gecko/20101027 Fedora/3.1.6-1.fc14 Mnenhy/0.8.3 Thunderbird/3.1.6

On 11/05/2010 04:30 PM, Eric Blake wrote:
> * tests/test-ceilf2.c (ceilf_reference): Avoid icc's use of DAZ
> [denormals-as-zero] when optimizing without -mieee-fp option.
> * tests/test-floorf2.c (floorf_reference): Likewise.
> 
> Signed-off-by: Eric Blake <address@hidden>
> ---
> 
> This fixes failures of icc without -mieee-fp on test-{ceil,floor}f2.c.
> However, it does not fix test-{ceil,floor}f1.c, where my recent testsuite
> improvements flushed out the following compiler quirk:
> 
> icc -mieee-fp, as well as icc -O0 (which is also implied by icc -g),
> emit a call to ceilf(); therefore, if -lm has a working
> implementation, then configure detects that it is working, lib/ceil.c
> is never compiled, and the test passes.  But icc -O2 (which is the
> default with plain icc) recognizes ceilf and inlines it into assembly
> code that unfortunately turns -0.0f into 0.0.  Technically, this is
> not a bug, since POSIX is explicit that compliance of -0.0f is an
> optional feature, if you are not compliant with the <MX> option.
> 
> And, since I'm still not sure I want to force -mieee-fp onto all
> gnulib projects, that means I'm still scratching my head for a way
> to either defeat icc's insistence on inlining ceilf, or I'll have
> to tighten m4/ceilf.m4 to detect when icc is being used without full
> IEEE compliance.

That was tougher than I thought.  icc is good enough at inlining to
recognize a function pointer that points to a trivial wrapper around
ceilf, and replace even that with the compiler's ceilf replacement.  It
took the use of a "non-deterministic" decision between something that is
obviously not ceilf before I got the desired effect.  Here's what I'm
squashing in.

diff --git c/ChangeLog w/ChangeLog
index b853aad..e36599e 100644
--- c/ChangeLog
+++ w/ChangeLog
@@ -4,6 +4,9 @@
        * tests/test-ceilf2.c (ceilf_reference): Avoid icc's use of DAZ
        [denormals-as-zero] when optimizing without -mieee-fp option.
        * tests/test-floorf2.c (floorf_reference): Likewise.
+       * tests/test-ceilf1.c (dummy): New function.
+       (main): Use it to outsmart icc's optimization.
+       * tests/test-floorf1.c (dummy, main): Likewise.

        tests: require working signbit
        * modules/ceilf-tests (Depends-on): Add signbit.
diff --git c/tests/test-ceilf1.c w/tests/test-ceilf1.c
index 4e38247..a77641f 100644
--- c/tests/test-ceilf1.c
+++ w/tests/test-ceilf1.c
@@ -28,40 +28,53 @@ SIGNATURE_CHECK (ceilf, float, (float));
 #include "nan.h"
 #include "macros.h"

+/* If IEEE compliance was not requested, the ICC compiler inlines its
+   own ceilf assembly that turns -0.0f to 0.0f; but that is a correct
+   result when IEEE is not enforced.  To avoid spurious failure, we
+   have to provide this dummy function in order to outsmart ICC's
+   inlining, and call our ceilf through a function pointer.  */
+static float
+dummy (float f)
+{
+  return 0;
+}
+
 int
-main ()
+main (int argc, char **argv _GL_UNUSED)
 {
+  float (*my_ceilf) (float) = argc ? ceilf : dummy;
+
   /* Zero.  */
-  ASSERT (ceilf (0.0f) == 0.0f);
-  ASSERT (!signbit (ceilf (0.0f)));
-  ASSERT (ceilf (minus_zerof) == 0.0f);
-  ASSERT (!!signbit (minus_zerof) == !!signbit (ceilf (minus_zerof)));
+  ASSERT (my_ceilf (0.0f) == 0.0f);
+  ASSERT (!signbit (my_ceilf (0.0f)));
+  ASSERT (my_ceilf (minus_zerof) == 0.0f);
+  ASSERT (!!signbit (minus_zerof) == !!signbit (my_ceilf (minus_zerof)));
   /* Positive numbers.  */
-  ASSERT (ceilf (0.3f) == 1.0f);
-  ASSERT (ceilf (0.7f) == 1.0f);
-  ASSERT (ceilf (1.0f) == 1.0f);
-  ASSERT (ceilf (1.001f) == 2.0f);
-  ASSERT (ceilf (1.5f) == 2.0f);
-  ASSERT (ceilf (1.999f) == 2.0f);
-  ASSERT (ceilf (2.0f) == 2.0f);
-  ASSERT (ceilf (65535.99f) == 65536.0f);
-  ASSERT (ceilf (65536.0f) == 65536.0f);
-  ASSERT (ceilf (2.341e31f) == 2.341e31f);
+  ASSERT (my_ceilf (0.3f) == 1.0f);
+  ASSERT (my_ceilf (0.7f) == 1.0f);
+  ASSERT (my_ceilf (1.0f) == 1.0f);
+  ASSERT (my_ceilf (1.001f) == 2.0f);
+  ASSERT (my_ceilf (1.5f) == 2.0f);
+  ASSERT (my_ceilf (1.999f) == 2.0f);
+  ASSERT (my_ceilf (2.0f) == 2.0f);
+  ASSERT (my_ceilf (65535.99f) == 65536.0f);
+  ASSERT (my_ceilf (65536.0f) == 65536.0f);
+  ASSERT (my_ceilf (2.341e31f) == 2.341e31f);
   /* Negative numbers.  */
-  ASSERT (ceilf (-0.3f) == 0.0f);
-  ASSERT (ceilf (-0.7f) == 0.0f);
-  ASSERT (ceilf (-1.0f) == -1.0f);
-  ASSERT (ceilf (-1.5f) == -1.0f);
-  ASSERT (ceilf (-1.999f) == -1.0f);
-  ASSERT (ceilf (-2.0f) == -2.0f);
-  ASSERT (ceilf (-65535.99f) == -65535.0f);
-  ASSERT (ceilf (-65536.0f) == -65536.0f);
-  ASSERT (ceilf (-2.341e31f) == -2.341e31f);
+  ASSERT (my_ceilf (-0.3f) == 0.0f);
+  ASSERT (my_ceilf (-0.7f) == 0.0f);
+  ASSERT (my_ceilf (-1.0f) == -1.0f);
+  ASSERT (my_ceilf (-1.5f) == -1.0f);
+  ASSERT (my_ceilf (-1.999f) == -1.0f);
+  ASSERT (my_ceilf (-2.0f) == -2.0f);
+  ASSERT (my_ceilf (-65535.99f) == -65535.0f);
+  ASSERT (my_ceilf (-65536.0f) == -65536.0f);
+  ASSERT (my_ceilf (-2.341e31f) == -2.341e31f);
   /* Infinite numbers.  */
-  ASSERT (ceilf (1.0f / 0.0f) == 1.0f / 0.0f);
-  ASSERT (ceilf (-1.0f / 0.0f) == -1.0f / 0.0f);
+  ASSERT (my_ceilf (1.0f / 0.0f) == 1.0f / 0.0f);
+  ASSERT (my_ceilf (-1.0f / 0.0f) == -1.0f / 0.0f);
   /* NaNs.  */
-  ASSERT (isnanf (ceilf (NaNf ())));
+  ASSERT (isnanf (my_ceilf (NaNf ())));

   return 0;
 }
diff --git c/tests/test-floorf1.c w/tests/test-floorf1.c
index f0f7716..4877c41 100644
--- c/tests/test-floorf1.c
+++ w/tests/test-floorf1.c
@@ -28,40 +28,53 @@ SIGNATURE_CHECK (floorf, float, (float));
 #include "nan.h"
 #include "macros.h"

+/* If IEEE compliance was not requested, the ICC compiler inlines its
+   own floorf assembly that turns -0.0f to 0.0f; but that is a correct
+   result when IEEE is not enforced.  To avoid spurious failure, we
+   have to provide this dummy function in order to outsmart ICC's
+   inlining, and call our floorf through a function pointer.  */
+static float
+dummy (float f)
+{
+  return 0;
+}
+
 int
-main ()
+main (int argc, char **argv _GL_UNUSED)
 {
+  float (*my_floorf) (float) = argc ? floorf : dummy;
+
   /* Zero.  */
-  ASSERT (floorf (0.0f) == 0.0f);
-  ASSERT (!signbit (floorf (0.0f)));
-  ASSERT (floorf (minus_zerof) == 0.0f);
-  ASSERT (!!signbit (minus_zerof) == !!signbit (floorf (minus_zerof)));
+  ASSERT (my_floorf (0.0f) == 0.0f);
+  ASSERT (!signbit (my_floorf (0.0f)));
+  ASSERT (my_floorf (minus_zerof) == 0.0f);
+  ASSERT (!!signbit (minus_zerof) == !!signbit (my_floorf (minus_zerof)));
   /* Positive numbers.  */
-  ASSERT (floorf (0.3f) == 0.0f);
-  ASSERT (floorf (0.7f) == 0.0f);
-  ASSERT (floorf (1.0f) == 1.0f);
-  ASSERT (floorf (1.5f) == 1.0f);
-  ASSERT (floorf (1.999f) == 1.0f);
-  ASSERT (floorf (2.0f) == 2.0f);
-  ASSERT (floorf (65535.99f) == 65535.0f);
-  ASSERT (floorf (65536.0f) == 65536.0f);
-  ASSERT (floorf (2.341e31f) == 2.341e31f);
+  ASSERT (my_floorf (0.3f) == 0.0f);
+  ASSERT (my_floorf (0.7f) == 0.0f);
+  ASSERT (my_floorf (1.0f) == 1.0f);
+  ASSERT (my_floorf (1.5f) == 1.0f);
+  ASSERT (my_floorf (1.999f) == 1.0f);
+  ASSERT (my_floorf (2.0f) == 2.0f);
+  ASSERT (my_floorf (65535.99f) == 65535.0f);
+  ASSERT (my_floorf (65536.0f) == 65536.0f);
+  ASSERT (my_floorf (2.341e31f) == 2.341e31f);
   /* Negative numbers.  */
-  ASSERT (floorf (-0.3f) == -1.0f);
-  ASSERT (floorf (-0.7f) == -1.0f);
-  ASSERT (floorf (-1.0f) == -1.0f);
-  ASSERT (floorf (-1.001f) == -2.0f);
-  ASSERT (floorf (-1.5f) == -2.0f);
-  ASSERT (floorf (-1.999f) == -2.0f);
-  ASSERT (floorf (-2.0f) == -2.0f);
-  ASSERT (floorf (-65535.99f) == -65536.0f);
-  ASSERT (floorf (-65536.0f) == -65536.0f);
-  ASSERT (floorf (-2.341e31f) == -2.341e31f);
+  ASSERT (my_floorf (-0.3f) == -1.0f);
+  ASSERT (my_floorf (-0.7f) == -1.0f);
+  ASSERT (my_floorf (-1.0f) == -1.0f);
+  ASSERT (my_floorf (-1.001f) == -2.0f);
+  ASSERT (my_floorf (-1.5f) == -2.0f);
+  ASSERT (my_floorf (-1.999f) == -2.0f);
+  ASSERT (my_floorf (-2.0f) == -2.0f);
+  ASSERT (my_floorf (-65535.99f) == -65536.0f);
+  ASSERT (my_floorf (-65536.0f) == -65536.0f);
+  ASSERT (my_floorf (-2.341e31f) == -2.341e31f);
   /* Infinite numbers.  */
-  ASSERT (floorf (1.0f / 0.0f) == 1.0f / 0.0f);
-  ASSERT (floorf (-1.0f / 0.0f) == -1.0f / 0.0f);
+  ASSERT (my_floorf (1.0f / 0.0f) == 1.0f / 0.0f);
+  ASSERT (my_floorf (-1.0f / 0.0f) == -1.0f / 0.0f);
   /* NaNs.  */
-  ASSERT (isnanf (floorf (NaNf ())));
+  ASSERT (isnanf (my_floorf (NaNf ())));

   return 0;
 }


-- 
Eric Blake   address@hidden    +1-801-349-2682
Libvirt virtualization library http://libvirt.org

Attachment: signature.asc
Description: OpenPGP digital signature


reply via email to

[Prev in Thread] Current Thread [Next in Thread]