This is the mail archive of the libc-alpha@sourceware.org mailing list for the glibc 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: SSE sin and cos for x86 and x86_64


Fixed and reattached.
(fixed Change Logs. Sun copyright, overlong lines)

Thanks a lot for the review.

Change Logs:

#1
2012-09-03  Liubov Dmitrieva  <liubov.dmitrieva@gmail.com>

	* math/libm-test.inc (cos_test): Add more test cases.
	(sin_test): Likewise.
	(sincos_test): Likewise.


#2
2012-09-03  Liubov Dmitrieva  <liubov.dmitrieva@gmail.com>

	* sysdeps/i386/i686/fpu/multiarch/Makefile (sysdep_routines):
	Add s_sinf-sse2, s_conf-sse2

	* sysdeps/i386/i686/fpu/multiarch/s_sinf-sse2.S: New file
	* sysdeps/i386/i686/fpu/multiarch/s_cosf-sse2.S: New file
	* sysdeps/i386/i686/fpu/multiarch/s_sinf.c: New file
	* sysdeps/i386/i686/fpu/multiarch/s_cosf.c: New file

	* sysdeps/ieee754/flt-32/s_sinf.c (SINF, SINF_FUNC): Add macros
	for using routine as __sinf_ia32.
	Use macro for function declaration and weak_alias.
	* sysdeps/ieee754/flt-32/s_cosf.c (COSF, COSF_FUNC): Add macros
	for using routine as __cosf_ia32.
	Use macro for function declaration and weak_alias.

	* sysdeps/i386/i686/fpu/multiarch/e_expf-sse2.S: Fix Copyright
	* sysdeps/i386/i686/fpu/multiarch/e_expf.c: Fix Copyright


#3
2012-09-03  Liubov Dmitrieva  <liubov.dmitrieva@gmail.com>

       * sysdeps/x86_64/fpu/s_sinf.S: New file.
       * sysdeps/x86_64/fpu/s_cosf.S: New file.
       * sysdeps/x86_64/fpu/libm-test-ulps: Update.


--
Liubov Dmitrieva
Intel Corporation

2012/8/30 Andreas Jaeger <aj@suse.com>:
> On Thursday, August 23, 2012 17:28:26 Dmitrieva Liubov wrote:
>> Ping.
>>
>> SSE2 sin and cos patches (up to 10 times faster) are waiting for
>> review and my patch with new test cases as well.
>> I've attached all 3 again (the same as last time)
>
> The patches are all fine with one minor nit: Please reformat them so that
> we do not have overlong lines (> 78 chars). A couple of comments are
> going over.
>
> Note: do not wrap libm-test.inc, those long lines are fine.
>> #1
>> 2012-08-16  Liubov Dmitrieva  <liubov.dmitrieva@gmail.com>
>>
>>           * math/libm-test.inc: Update
>>           Add new test cases in large arguments path.
>
> This should name the functions you change:
>            * math/libm-test.inc (cos_test): Add more test cases.
>                 (sin_test): Likewise.
>                 (sincos_test): Likewise.
>
>
>
>> #2
>> 2012-08-13  Liubov Dmitrieva  <liubov.dmitrieva@gmail.com>
>>
>>          * sysdeps/i386/i686/fpu/multiarch/Makefile: Update
>>          (sysdep_routines): Add s_sinf-sse2, s_conf-sse2
>
>
> Just name the variable you update, no need to say "Update" for the file:
>
>           * sysdeps/i386/i686/fpu/multiarch/Makefile (sysdep_routines):
>          Add s_sinf-sse2, s_conf-sse2
>
>>
>>          * sysdeps/i386/i686/fpu/multiarch/s_sinf-sse2.S: New file
>>          * sysdeps/i386/i686/fpu/multiarch/s_cosf-sse2.S: New file
>>          * sysdeps/i386/i686/fpu/multiarch/s_sinf.c: New file
>>          * sysdeps/i386/i686/fpu/multiarch/s_cosf.c: New file
>>          * sysdeps/ieee754/flt-32/s_sinf.c: Update
>>          (SINF): Add macro for using routine as __sinf_ia32
>
> see below
>>          * sysdeps/ieee754/flt-32/s_cosf.c: Update
>>          (COSF): Add macro for using routine as __cosf_ia32
>
> Don't update the Sun copyrights.
> And write the changelog as:
>           * sysdeps/ieee754/flt-32/s_cosf.c (COSF, COFS_FUNC): Add macros
> for using routine as __cosf_ia32.
>             Use macro for function declaration and weak_alias.
>
>
>
>
>>          * sysdeps/i386/i686/fpu/multiarch/e_expf-sse2.S: Fix
>> Copyright * sysdeps/i386/i686/fpu/multiarch/e_expf.c: Fix Copyright
>>
>> #3
>> 2012-08-15  Liubov Dmitrieva  <liubov.dmitrieva@gmail.com>
>>
>>        * sysdeps/x86_64/fpu/s_sinf.S: New file.
>>        * sysdeps/x86_64/fpu/s_cosf.S: New file.
>>        * sysdeps/x86_64/fpu/libm-test-ulps: Update.
>
> Could you resend everything with the above changes, please? I'll commit
> then on your behalf...
>
> Thanks a lot!
> Andreas
>
>> --
>> Liubov Dmitrieva
>> Intel Corporation
>>
>> 2012/8/16 Dmitrieva Liubov <liubov.dmitrieva@gmail.com>:
>> > False alarm. Our functions work correctly.
>> >
>> > But anyway I have a separate patch for adding that test cases to
>> > "make check" (attached).
>> > It does not reveal new fails in current GLIBC, but several 1-ulp new
>> > errors on IA.
>> > And it does not reveal new fails in our new sinf/cosf  functions
>> > (and
>> > no 1-ulp new errors)
>> >
>> >
>> > 2012-08-16  Liubov Dmitrieva  <liubov.dmitrieva@gmail.com>
>> >
>> >          * math/libm-test.inc: Update
>> >          Add new test cases in large arguments path.
>> >
>> > So, no need to fix here, both patches are ok.
>> >
>> > The latest version were attached to:
>> > http://sourceware.org/ml/libc-alpha/2012-08/msg00267.html
>> > http://sourceware.org/ml/libc-alpha/2012-08/msg00265.html
>> >
>> > --
>> > Liubov Dmitrieva
>> >
>> > 2012/8/15 Joseph S. Myers <joseph@codesourcery.com>:
>> >> On Wed, 15 Aug 2012, Dmitrieva Liubov wrote:
>> >>> > This code is wrong. You cannot perform argument reduction for
>> >>> > large
>> >>> > arguments by using a single, double, or even extended-precision
>> >>> > approximation for pi.
>> >>>
>> >>> Yes, that's wrong in x86_32 version and will be fixed but 64 bit
>> >>> version looks ok.
>> >>
>> >> If that didn't get detected by the testsuite, I suppose we should
>> >> add
>> >> 0x1p+120 (or some such value that detects the problem) to the tests
>> >> for cos and sin in libm-test.inc.  (The larest float value for cos
>> >> in the testsuite is 0x1p65; sin also tests 0x1.7f4134p+103.)
>> >>
>> >> --
>> >> Joseph S. Myers
>> >> joseph@codesourcery.com
> --
>  Andreas Jaeger aj@{suse.com,opensuse.org} Twitter/Identica: jaegerandi
>   SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany
>    GF: Jeff Hawn,Jennifer Guild,Felix Imendörffer,HRB16746 (AG Nürnberg)
>     GPG fingerprint = 93A3 365E CE47 B889 DF7F  FED1 389A 563C C272 A126
>

Attachment: new_test_cases.patch
Description: Binary data

Attachment: sinf_cosf_x86_32.patch
Description: Binary data

Attachment: sinf_cosf_x86_64.patch
Description: Binary data


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