| Message ID | CAK=A3=2PQh5RiuDWn9yGv-jxkC5G-s2JRSQTF0PEmaQSpsnyZg@mail.gmail.com |
|---|---|
| State | New |
| Headers | show |
On Tue, 3 Sep 2013, Cong Hou wrote: > + 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; Please reread my previous messages on this subject and try again, with regard to both the patch itself and the accompanying analysis.
From Joseph: "The conversion is not safe for sqrt if the two types are double and long double and long double is x86 extended, for example." This is not reflected in the patch. David On Tue, Sep 3, 2013 at 2:27 PM, Joseph S. Myers <joseph@codesourcery.com> wrote: > On Tue, 3 Sep 2013, Cong Hou wrote: > >> + 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; > > Please reread my previous messages on this subject and try again, with > regard to both the patch itself and the accompanying analysis. > > -- > Joseph S. Myers > joseph@codesourcery.com
On Tue, 3 Sep 2013, Xinliang David Li wrote: > >From Joseph: > > "The > conversion is not safe for sqrt if the two types are double and long > double and long double is x86 extended, for example." > > This is not reflected in the patch. No, the problem is that it tries to reflect it but hardcodes the specific example I gave, rather than following the logic I explained regarding the precisions of the types involved, which depend on the target. And since I only gave a simplified analysis, for two types when this function deals with cases involving three types, the patch submission needs to include its own analysis for the full generality of three types to justify the logic used (as inequalities involving the three precisions). (I suspect it reduces to the case of two types so you don't need to go into the details of reasoning about floating point to produce the more general analysis. But in any case, it's for the patch submitter to give the full explanation.)
Could you please tell me how to check the precision of long double in GCC on different platforms? Thank you! Cong On Tue, Sep 3, 2013 at 2:43 PM, Joseph S. Myers <joseph@codesourcery.com> wrote: > On Tue, 3 Sep 2013, Xinliang David Li wrote: > >> >From Joseph: >> >> "The >> conversion is not safe for sqrt if the two types are double and long >> double and long double is x86 extended, for example." >> >> This is not reflected in the patch. > > No, the problem is that it tries to reflect it but hardcodes the specific > example I gave, rather than following the logic I explained regarding the > precisions of the types involved, which depend on the target. And since I > only gave a simplified analysis, for two types when this function deals > with cases involving three types, the patch submission needs to include > its own analysis for the full generality of three types to justify the > logic used (as inequalities involving the three precisions). (I suspect > it reduces to the case of two types so you don't need to go into the > details of reasoning about floating point to produce the more general > analysis. But in any case, it's for the patch submitter to give the full > explanation.) > > -- > Joseph S. Myers > joseph@codesourcery.com
On Tue, 3 Sep 2013, Cong Hou wrote: > Could you please tell me how to check the precision of long double in > GCC on different platforms? REAL_MODE_FORMAT (TYPE_MODE (long_double_type_node))->p (but you should be referring to the relevant types - "type", the type being converted to, "itype", the type of the function being called in the source code, "TREE_TYPE (arg0)", the type of the argument after extensions have been removed, and "newtype", computed from those - so you should have expressions like the above with two or more of those four types, but not with long_double_type_node directly). The patch submission will need to include a proper analysis to justify to the reader why the particular inequality with particular types from those four is correct in all cases where the relevant code may be executed.
On 4 September 2013 00:17:00 Cong Hou <congh@google.com> wrote: > Could you please tell me how to check the precision of long double in > GCC on different platforms? I did not follow your discussion but.. http://uclibc.org/~aldot/precision_check.f Or something along those lines in your favourite language. HTH.. > Thank you! > > > Cong Sent with AquaMail for Android http://www.aqua-mail.com
=================================================================== --- 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 (); }
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 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