Fortran code used in harmonic analysis often contains lines of form p = cmplx(cos(x),sin(x)) ! compute exp(i*x) for real x On platforms with a sincos(x, &s, &c) routine the above should be transformed to call sincos(x, &s, &c) p = cmplx(c, s) The reason is that both sin() and cos() perform argument reduction of x into the range [0,pi/4]. sincos() performs this reduce once, and hence is typically faster. The same optimization should apply to C/C++ code. #include <math.h> double foo(double x) { double c,s,res; c = cos(x); s = sin(x); res = c * s; return res; } % ~/work/x/bin/gcc -o - -S -O3 -o a.s a.c .file "a.c" .text .p2align 4 .globl foo .type foo, @function foo: .LFB3: .cfi_startproc subq $24, %rsp .cfi_def_cfa_offset 32 movsd %xmm0, 8(%rsp) call cos movsd 8(%rsp), %xmm1 movsd %xmm0, (%rsp) movapd %xmm1, %xmm0 call sin mulsd (%rsp), %xmm0 addq $24, %rsp .cfi_def_cfa_offset 8 ret .cfi_endproc .LFE3: .size foo, .-foo .ident "GCC: (GNU) 9.0.1 20190125 (experimental)" .section .note.GNU-stack,"",@progbits
See pr31249.
On Wed, Jan 30, 2019 at 08:06:44PM +0000, dominiq at lps dot ens.fr wrote: > https://gcc.gnu.org/bugzilla/show_bug.cgi?id=89125 > > --- Comment #2 from Dominique d'Humieres <dominiq at lps dot ens.fr> --- > See pr31249. > Don't see the relevance in that I'm not advocating that the middle-end transform the sin(x), cos(x) pair into a complex function cexpi. function foo(x) complex foo real,intent(in) :: x foo = cmplx(cos(x),sin(x)) end gfcx -o - -S -O3 a.f90 .file "a.f90" .text .p2align 4 .globl foo_ .type foo_, @function foo_: .LFB0: .cfi_startproc subq $24, %rsp .cfi_def_cfa_offset 32 movss (%rdi), %xmm2 movaps %xmm2, %xmm0 movss %xmm2, 4(%rsp) call cosf movss 4(%rsp), %xmm2 movss %xmm0, (%rsp) movaps %xmm2, %xmm0 call sinf movss (%rsp), %xmm1 movss %xmm0, 12(%rsp) movss %xmm1, 8(%rsp) movq 8(%rsp), %xmm0 addq $24, %rsp .cfi_def_cfa_offset 8 ret .cfi_endproc .LFE0: .size foo_, .-foo_ .ident "GCC: (GNU) 9.0.1 20190125 (experimental)" .section .note.GNU-stack,"",@progbits On FreeBSD and suspect with glibc, there is a clear win Single precision, 100M calls for x in [0, 20000) ./testf -s -n 100 -m 0 -M 20000 100M sincosf calls in 4.235 seconds. 100M sinf and 100M cosf calls in 5.330 seconds. Double precision, 100M calls for x in [0, 20000) ./testd -s -n 100 -m 0 -M 20000 100M sincos calls in 5.222 seconds. 100M sin and 100M cos calls in 8.105 seconds.
This looks like a target issue, gcc does produce a call to sincos here. So please specify your target precisely.
On Wed, Jan 30, 2019 at 08:47:50PM +0000, glisse at gcc dot gnu.org wrote: > https://gcc.gnu.org/bugzilla/show_bug.cgi?id=89125 > > --- Comment #4 from Marc Glisse <glisse at gcc dot gnu.org> --- > This looks like a target issue, gcc does produce a call to sincos here. So > please specify your target precisely. > Yes, it seems to be a target issue. It's i585-*-freebsd and x86_64-*-freebsd. I've found that gcc/config/freebsd.h contains #define TARGET_LIBC_HAS_FUNCTION no_c99_libc_has_function AFAIK, FreeBSD is C99 compliant. This is an area I haven't worked in. I might be able to change this to default_libc_has_function, but that doesn't include sincos. So, it seems I need a function, perhaps, bsd_libc_has_function in targhooks.[ch] /* Assume that c99 functions are present at the runtime, including sincos. */ bool bsd_libc_has_function (enum function_class fn_class) { if (fn_class == function_c94 || fn_class == function_c99_misc || fn_class == function_c99_math_complex || fn_class == function_sincos) return true; return false; }
Checking with FreeBSD developers on C99 compliance.
The middle-end transforms sin/cos to cexp which means C99 support is required. cexp is then eventually expanded as sincos if the target advertises support via targetm.libc_has_function (function_sincos) or there is an optab handler (x87 has sincos IIRC). If sincos isn't available but the target is C99 we emit a call to cexp() which hopefully has an optimized path when passed a realpart zero (the glibc libm does). FreeBSD folks - can you adjust your targets config if appropriate?
(In reply to kargl from comment #6) > Checking with FreeBSD developers on C99 compliance. The answer is 'no'. FreeBSD's C runtime libraries (libc+libm) are not fully C99 complaint. It is a shame, too. Unfortunately and I should have remembered, FreeBSD's C runtime libraries (ie libc+libm) are not C99 compliant. The problem (for me) is that function_c99_math_complex indicates that libm includes a complete set of C99 complex math function, which of course it doesn't. Testing with GCC trunk gives 1 default_libc_has_function (C99 compliant libc+libm) 2 no_c99_libc_has_function (FreeBSD current setting) 1 2 === gcc Summary === # of expected passes 134923 134887 # of unexpected failures 171 207 <-- This is good. # of unexpected successes 27 27 # of expected failures 550 550 # of unresolved testcases 14 14 # of unsupported tests 2222 2222 === g++ Summary === # of expected passes 124009 124009 # of unexpected failures 41 41 # of expected failures 548 548 # of unsupported tests 5585 5585 === gfortran Summary === # of expected passes 48992 48993 # of unexpected failures 2 1 <-- This is bad. # of expected failures 130 130 # of unsupported tests 88 88 'This is bad' occurs because FreeBSD is missing cexpl and the fallback in libgfortran/intrinsics/c99_functions.c seems to be broken on FreeBSD if default_libc_has_function is used.
Created attachment 45592 [details] targ.diff On Wed, Jan 30, 2019 at 09:10:25PM +0000, sgk at troutmask dot apl.washington.edu wrote: > > --- Comment #5 from Steve Kargl <sgk at troutmask dot apl.washington.edu> --- > > Yes, it seems to be a target issue. It's i585-*-freebsd > and x86_64-*-freebsd. > > I've found that gcc/config/freebsd.h contains > > #define TARGET_LIBC_HAS_FUNCTION no_c99_libc_has_function > > AFAIK, FreeBSD is C99 compliant. This is an area I haven't worked > in. I might be able to change this to default_libc_has_function, > but that doesn't include sincos. So, it seems I need a function, > perhaps, bsd_libc_has_function in targhooks.[ch] > > /* Assume that c99 functions are present at the runtime, > including sincos. */ > bool > bsd_libc_has_function (enum function_class fn_class) > { > if (fn_class == function_c94 > || fn_class == function_c99_misc > || fn_class == function_c99_math_complex > || fn_class == function_sincos) > return true; > > return false; > } > With the attached patch, there is an improvement in the the number of passing tests for gcc. Both g++ and gfortran are unchanged. === gcc Summary === # of expected passes 134938 # of unexpected failures 178 # of unexpected successes 27 # of expected failures 550 # of unresolved testcases 14 # of unsupported tests 2222 === g++ Summary === # of expected passes 124050 # of unexpected failures 42 # of expected failures 548 # of unsupported tests 5594 === gfortran Summary === # of expected passes 48999 # of unexpected failures 1 # of expected failures 130 # of unsupported tests 88 In code spelunking, I've inspected all of the builtins implied by the various function_* keywords. For FreeBSD, this comes down to function_c94 All functions available. function_c99_misc powl, tgammal exist but have precision issues function_c99_math_complex ccoshl, ccosl, cexpl, csinhl, csinl, ctanhl, ctanl. These functions are missing. cpow[fl] is available, but is from Cephes library. These have precisions issues on i686 and maybe 128-bit long double. function_c11_misc function available
I can confirm this finding with the attached patch. There is an improvement in the gcc results but no improvement/degradation in the other results. Tested on yesterday's trunk.
On Fri, Feb 01, 2019 at 09:50:55PM +0000, andreast at gcc dot gnu.org wrote: > https://gcc.gnu.org/bugzilla/show_bug.cgi?id=89125 > > --- Comment #10 from Andreas Tobler <andreast at gcc dot gnu.org> --- > I can confirm this finding with the attached patch. > There is an improvement in the gcc results but no improvement/degradation in > the other results. Tested on yesterday's trunk. > I suspect the patch should not be applied to targhook.[ch]. For freebsd, we prabably need a gcc/config/freebsd.c. This would be similar to how darwin handles the builtins on rs6000. % find config -type f | xargs grep darwin_libc_has_function darwin.c:darwin_libc_has_function (enum function_class fn_class) darwin-protos.h:extern bool darwin_libc_has_function \ (enum function_class fn_class); /rs6000/darwin.h:#define TARGET_LIBC_HAS_FUNCTION darwin_libc_has_function
where ever we place it, it'll be an improvement. Make sense to place it in config/freebsd.c, then it is FreeBSD only.
Created attachment 52785 [details] New patch New patch that deals with the *.c to *.cc rename
*** Bug 105206 has been marked as a duplicate of this bug. ***
It's been over two years, can someone please commit the new path? Testing shows 31 new passes and no regressions. === gcc Summary === w/o patch w patch # of expected passes 175405 175434 # of unexpected failures 1081 1051 # of unexpected successes 20 20 # of expected failures 1459 1459 # of unresolved testcases 10 10 # of unsupported tests 3252 3252 === g++ Summary === w/o patch w patch # of expected passes 225338 225341 # of unexpected failures 678 676 # of expected failures 2071 2071 # of unresolved testcases 11 11 # of unsupported tests 10353 10353 === gfortran Summary === w/o patch w patch # of expected passes 65901 65901 # of unexpected failures 12 12 # of expected failures 272 272 # of unsupported tests 100 100
Can someone please commit the patch?
The master branch has been updated by Richard Biener <rguenth@gcc.gnu.org>: https://gcc.gnu.org/g:b6e22db8564827c82108e0b7fa1a84675379c12b commit r12-8240-gb6e22db8564827c82108e0b7fa1a84675379c12b Author: Steve Kargl <kargl@gcc.gnu.org> Date: Mon Apr 25 09:23:56 2022 +0200 target/89125 - BSD and math functions Back story: When GCC is configured and built on non-glibc platforms, it seems very little to no effort is made to enumerate the available C99 libm functions. It is all or nothing for C99 libm. The patch introduces a new function, used on only FreeBSD, to inform gcc that it has C99 libm functions (minus a few which clearly GCC does not check nor test). 2022-04-15 Steven G. Kargl <kargl@gcc.gnu.org> PR target/89125 * config/freebsd.h: Define TARGET_LIBC_HAS_FUNCTION to be bsd_libc_has_function. * targhooks.cc (bsd_libc_has_function): New function. Expand the supported math functions to inclue C99 libm. * targhooks.h (bsd_libc_has_function): New Prototype.
Fixed for GCC 12.
On Mon, Apr 25, 2022 at 07:28:50AM +0000, rguenth at gcc dot gnu.org wrote: > https://gcc.gnu.org/bugzilla/show_bug.cgi?id=89125 > > Richard Biener <rguenth at gcc dot gnu.org> changed: > > What |Removed |Added > ---------------------------------------------------------------------------- > Resolution|--- |FIXED > Status|NEW |RESOLVED > Known to work| |12.0 > > --- Comment #18 from Richard Biener <rguenth at gcc dot gnu.org> --- > Fixed for GCC 12. > Thanks.