This is the mail archive of the gcc-patches@gcc.gnu.org mailing list for the GCC project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

Re: [PATCH] Fixing improper conversion from sin() to sinf() in optimization mode.


I have fixed my test code and replaced those aliasing violations with
unions. Now the test result shows logb() is safe for the conversions.

The conclusion is, logb() and fabs() are always safe for the
converion, and sqrt() is unsafe for the conversion from sqrtl(double)
to sqrt(double). Other math functions are not safe for the conversion.

The new test code I used is shown below:

#include <time.h>
#include <stdlib.h>
#include <math.h>
#include <stdio.h>

typedef union
{
    int i;
    float f;
} T32;

typedef union
{
    long long int i;
    double f;
} T64;

#define N 10000000

#define test_math_func(func) \
for (i = 0; i < N; ++i) \
{ \
  int d = rand(), e = rand(); \
  if (d == 0) continue; \
  T32 v, r1, r2; \
  v.f = (float)e / d; \
  r1.f = func(v.f), r2.f = func##f(v.f); \
  if (r1.f != r2.f) \
  { \
    printf("%s double -> float (%X) %X %X\n", #func, v.i, r1.i, r2.i); \
    break; \
  } \
} \
for (i = 0; i < N; ++i) \
{ \
  int d = rand(), e = rand(); \
  if (d == 0) continue; \
  T32 v, r1, r2; \
  v.f = (float)e / d; \
  r1.f = func##l(v.f), r2.f = func##f(v.f); \
  if (r1.f != r2.f) \
  { \
    printf("%s long double -> float (%X) %X %X\n", #func, v.i, r1.i, r2.i); \
    break; \
  } \
} \
for (i = 0; i < N; ++i) \
{ \
  int d = rand(), e = rand(); \
  if (d == 0) continue; \
  T64 v, r1, r2; \
  v.f = (double)e / d; \
  r1.f = func##l(v.f), r2.f = func(v.f); \
  if (r1.f != r2.f) \
  { \
    printf("%s long double -> double (%016llX) %016llX %016llX\n",
#func, v.i, r1.i, r2.i); \
    break; \
  } \
}

int main()
{
  int i;
  test_math_func(sin);
  test_math_func(cos);
  test_math_func(sinh);
  test_math_func(cosh);
  test_math_func(asin);
  test_math_func(acos);
  test_math_func(asinh);
  test_math_func(acosh);
  test_math_func(tan);
  test_math_func(tanh);
  test_math_func(atan);
  test_math_func(atanh);
  test_math_func(log);
  test_math_func(log10);
  test_math_func(log1p);
  test_math_func(log2);
  test_math_func(logb);
  test_math_func(cbrt);
  test_math_func(erf);
  test_math_func(erfc);
  test_math_func(exp);
  test_math_func(exp2);
  test_math_func(expm1);
  test_math_func(sqrt);
  test_math_func(fabs);
}


I have modified the patch according to this new conclusion. The patch
is pasted as below.

thanks,
Cong


===================================================================
--- gcc/convert.c (revision 201891)
+++ gcc/convert.c (working copy)
@@ -135,16 +135,24 @@ convert_to_real (tree type, tree expr)
   CASE_MATHFN (COS)
   CASE_MATHFN (ERF)
   CASE_MATHFN (ERFC)
-  CASE_MATHFN (FABS)
   CASE_MATHFN (LOG)
   CASE_MATHFN (LOG10)
   CASE_MATHFN (LOG2)
   CASE_MATHFN (LOG1P)
-  CASE_MATHFN (LOGB)
   CASE_MATHFN (SIN)
-  CASE_MATHFN (SQRT)
   CASE_MATHFN (TAN)
   CASE_MATHFN (TANH)
+            /* The above functions are not safe to do this conversion. */
+            if (!flag_unsafe_math_optimizations)
+              break;
+  CASE_MATHFN (SQRT)
+            /* sqrtl(double) cannot be safely converted to sqrt(double). */
+            if (fcode == BUILT_IN_SQRTL &&
+                (TYPE_MODE (type) == TYPE_MODE (double_type_node)) &&
+                !flag_unsafe_math_optimizations)
+              break;
+  CASE_MATHFN (FABS)
+  CASE_MATHFN (LOGB)
 #undef CASE_MATHFN
     {
       tree arg0 = strip_float_extensions (CALL_EXPR_ARG (expr, 0));
Index: gcc/testsuite/gcc.c-torture/execute/20030125-1.c
===================================================================
--- gcc/testsuite/gcc.c-torture/execute/20030125-1.c (revision 201891)
+++ gcc/testsuite/gcc.c-torture/execute/20030125-1.c (working copy)
@@ -44,11 +44,11 @@ __attribute__ ((noinline))
 double
 sin(double a)
 {
- abort ();
+ return a;
 }
 __attribute__ ((noinline))
 float
 sinf(float a)
 {
- return a;
+ abort ();
 }

On Sat, Aug 31, 2013 at 9:24 AM, Joseph S. Myers
<joseph@codesourcery.com> wrote:
> On Sat, 31 Aug 2013, Cong Hou wrote:
>
>> > I don't see why it would be unsafe for logb - can you give an example
>> > (exact float input value as hex float, and the values you believe logb
>> > should return for float and double).
>> >
>>
>> Please try the following code (you will get different results whether to
>> use optimization mode):
>>
>> #include <math.h>
>> #include <stdio.h>
>>
>> int main()
>> {
>>   int i = 0x3edc67d5;
>>   float f = *((float*)&i);
>>   float r1 = logb(f);
>>   float r2 = logbf(f);
>>   printf("%x %x\n", *((int*)&r1), *((int*)&r2));
>> }
>
> (a) Please stop sending HTML email, so your messages reach the mailing
> list, and resend your messages so far to the list.  The mailing list needs
> to see the whole of both sides of the discussion of any patch being
> proposed for GCC.
>
> (b) I referred to the values *you believe logb should return*.
> Optimization is not meant to preserve library bugs; the comparison should
> be on the basis of correctly rounded results from both float and double
> functions.  The correct return from logb appears to be -2 here, and I get
> that from both logb and logbf with current git glibc.  The existence of a
> bug in some old library is not relevant here.
>
> (c) I always advise writing such tests as *valid C code* using hex floats
> (or if really necessary, unions), not *invalid C code* with aliasing
> violations.
>
> --
> Joseph S. Myers
> joseph@codesourcery.com


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]