| Message ID | AANLkTinuzk6qnVsOvGYxu06j2PfYdMh0_mACaTGKSk2B@mail.gmail.com |
|---|---|
| State | New |
| Headers | show |
Hello: On 31/08/2010 1:53 AM, H.J. Lu wrote: > On Mon, Aug 30, 2010 at 7:57 AM, Richard Guenther > <richard.guenther@gmail.com> wrote: >> On Mon, Aug 30, 2010 at 4:40 PM, H.J. Lu<hongjiu.lu@intel.com> wrote: >>> Hi, >>> >>> On x86, sincos is always available if x87 FPU fsincos is available. >>> This patch enables TARGET_HAS_SINCOS for -ffast-math and x87 FPU. Also >>> x86 Bionic C library doesn't provide sincos and we shouldn't enable >>> TARGET_HAS_SINCOS with OPTION_BIONIC on x86. OK for trunk? >> >> Hm. Shouldn't it be conditional on USE_FANCY_MATH_387 >> && !NO_FANCY_MATH_387 as well? > > You are right. Here is the updated patch. > For your info, I used revision 163859, is this patch applied? I get link error for the target arm-linux-androideabi: (sincos isn't used in the source) imdct.c:(.text+0x27ac): undefined reference to `sincos' imdct.c:(.text+0x281c): undefined reference to `sincos' imdct.c:(.text+0x2894): undefined reference to `sincos' imdct.c:(.text+0x2910): undefined reference to `sincos' imdct.c:(.text+0x2980): undefined reference to `sincos' Does this mean that gcc is deliberately looking for sincos? >> Also a flag dependency breaks with the target/option attributes, > > The current target/option attributes have many problems. It > shouldn't prevent us from generating better codes. > >> so I'm not sure it is a good idea. Does anyone care for FP >> execution performance on 32bit anyway? >> > > Intel cares FP execution performance on 32bit > > Thanks. > >
On Sat, Sep 4, 2010 at 9:01 PM, t66667@gmail.com <t66667@gmail.com> wrote: > Hello: > On 31/08/2010 1:53 AM, H.J. Lu wrote: >> >> On Mon, Aug 30, 2010 at 7:57 AM, Richard Guenther >> <richard.guenther@gmail.com> wrote: >>> >>> On Mon, Aug 30, 2010 at 4:40 PM, H.J. Lu<hongjiu.lu@intel.com> wrote: >>>> >>>> Hi, >>>> >>>> On x86, sincos is always available if x87 FPU fsincos is available. >>>> This patch enables TARGET_HAS_SINCOS for -ffast-math and x87 FPU. Also >>>> x86 Bionic C library doesn't provide sincos and we shouldn't enable >>>> TARGET_HAS_SINCOS with OPTION_BIONIC on x86. OK for trunk? >>> >>> Hm. Shouldn't it be conditional on USE_FANCY_MATH_387 >>> && !NO_FANCY_MATH_387 as well? >> >> You are right. Here is the updated patch. >> > For your info, I used revision 163859, is this patch applied? My patch only fixes x86, not ARM. > I get link error for the target arm-linux-androideabi: (sincos isn't used in > the source) > imdct.c:(.text+0x27ac): undefined reference to `sincos' > imdct.c:(.text+0x281c): undefined reference to `sincos' > imdct.c:(.text+0x2894): undefined reference to `sincos' > imdct.c:(.text+0x2910): undefined reference to `sincos' > imdct.c:(.text+0x2980): undefined reference to `sincos' > Does this mean that gcc is deliberately looking for sincos? Gcc uses sincos if TARGET_HAS_SINCOS is true.
diff --git a/gcc/config/i386/i386.h b/gcc/config/i386/i386.h index 5bae99d..16a17c4 100644 --- a/gcc/config/i386/i386.h +++ b/gcc/config/i386/i386.h @@ -2492,6 +2492,13 @@ struct GTY(()) machine_function { #undef TARG_COND_NOT_TAKEN_BRANCH_COST #define TARG_COND_NOT_TAKEN_BRANCH_COST ix86_cost->cond_not_taken_branch_cost +/* Use x87 FPU fsincos if it is available. */ +#undef TARGET_HAS_SINCOS +#define TARGET_HAS_SINCOS \ + (TARGET_USE_FANCY_MATH_387 \ + && flag_unsafe_math_optimizations \ + && (!TARGET_SSE_MATH || TARGET_MIX_SSE_I387)) + /* Local variables: version-control: t diff --git a/gcc/config/i386/linux.h b/gcc/config/i386/linux.h index 2a31880..42db416 100644 --- a/gcc/config/i386/linux.h +++ b/gcc/config/i386/linux.h @@ -35,6 +35,16 @@ along with GCC; see the file COPYING3. If not see #undef TARGET_TLS_DIRECT_SEG_REFS_DEFAULT #define TARGET_TLS_DIRECT_SEG_REFS_DEFAULT MASK_TLS_DIRECT_SEG_REFS +/* Whether we have sincos that follows the GNU extension. There is no + sincos in Bionic C library. We can only use x87 FPU fsincos if it + is available. */ +#undef TARGET_HAS_SINCOS +#define TARGET_HAS_SINCOS \ + (OPTION_GLIBC \ + || (TARGET_USE_FANCY_MATH_387 \ + && flag_unsafe_math_optimizations \ + && (!TARGET_SSE_MATH || TARGET_MIX_SSE_I387))) + #undef ASM_COMMENT_START #define ASM_COMMENT_START "#" diff --git a/gcc/config/i386/linux64.h b/gcc/config/i386/linux64.h index 867de59..7bc12b7 100644 --- a/gcc/config/i386/linux64.h +++ b/gcc/config/i386/linux64.h @@ -50,6 +50,16 @@ see the files COPYING3 and COPYING.RUNTIME respectively. If not, see #undef TARGET_TLS_DIRECT_SEG_REFS_DEFAULT #define TARGET_TLS_DIRECT_SEG_REFS_DEFAULT MASK_TLS_DIRECT_SEG_REFS +/* Whether we have sincos that follows the GNU extension. There is no + sincos in Bionic C library. We can only use x87 FPU fsincos if it + is available. */ +#undef TARGET_HAS_SINCOS +#define TARGET_HAS_SINCOS \ + (OPTION_GLIBC \ + || (TARGET_USE_FANCY_MATH_387 \ + && flag_unsafe_math_optimizations \ + && (!TARGET_SSE_MATH || TARGET_MIX_SSE_I387))) + /* Provide a LINK_SPEC. Here we provide support for the special GCC options -static and -shared, which allow us to link things in one of these three modes by applying the appropriate combinations of diff --git a/gcc/config/linux.h b/gcc/config/linux.h index e283a9a..576a2ac 100644 --- a/gcc/config/linux.h +++ b/gcc/config/linux.h @@ -159,7 +159,9 @@ see the files COPYING3 and COPYING.RUNTIME respectively. If not, see is present in the runtime library. */ #define TARGET_C99_FUNCTIONS (OPTION_GLIBC) +#ifndef TARGET_HAS_SINCOS /* Whether we have sincos that follows the GNU extension. */ #define TARGET_HAS_SINCOS (OPTION_GLIBC || OPTION_BIONIC) +#endif #define TARGET_POSIX_IO