Page MenuHomeFreeBSD

Implement sincos, sincosf, and sincosl.
ClosedPublic

Authored by mmel on May 17 2017, 3:48 AM.

Details

Summary

The primary benefit of these functions is that argument
reduction is done once instead of twice in independent
calls to sin() and cos().

  • lib/msun/Makefile: . Add s_sincos[fl].c to the build. . Add sincos.3 documentation. . Add appropriate MLINKS.
  • lib/msun/Symbol.map: . Expose sincos[fl] symbols in dynamic libm.so.
  • lib/msun/man/sincos.3: . Documentation for sincos[fl].
  • lib/msun/src/k_sincos.h: . Kernel for sincos() function. This merges the individual kernels for sin() and cos(). The merger offered an opportunity to re-arrange the individual kernels for better performance.
  • lib/msun/src/k_sincosf.h: . Kernel for sincosf() function. This merges the individual kernels for sinf() and cosf(). The merger offered an opportunity to re-arrange the individual kernels for better performance.
  • lib/msun/src/k_sincosl.h: . Kernel for sincosl() function. This merges the individual kernels for sinl() and cosl(). The merger offered an opportunity to re-arrange the individual kernels for better performance.
  • lib/msun/src/math.h: . Add prototytpes for sincos[fl]().
  • lib/msun/src/math_private.h: . Add RETURNV macros. This is needed to reset fpsetprec on I386 hardware for a function with type void.
  • lib/msun/src/s_sincos.c: . Implementation of sincos() where sin() and cos() were merged into one routine and possibly re-arranged for better performance.
  • lib/msun/src/s_sincosf.c: . Implementation of sincosf() where sinf() and cosf() were merged into one routine and possibly re-arranged for better performance.
  • lib/msun/src/s_sincosl.c: . Implementation of sincosl() where sinl() and cosl() were merged into one routine and possibly re-arranged for better performance.

MFC after: 3 weeks
PR: 215977, 218300
Submitted by: Steven G. Kargl <sgk@troutmask.apl.washington.edu>

Diff Detail

Repository
rS FreeBSD src repository
Lint
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; unit tests not applicable.

Event Timeline

mmel created this revision.May 17 2017, 3:48 AM
mmel added a comment.May 17 2017, 3:56 AM

Oups, the 'lib/msun/src/math.h.orig' will not be included in the final commit, of course.

lidl added inline comments.
lib/msun/src/k_sincos.h
1 ↗(On Diff #28446)

While not part of the kernel, style(9) says that copyright
blocks start with:

/*-

Not just

/*

lib/msun/src/k_sincosf.h
1 ↗(On Diff #28446)

Same here, /*-

lib/msun/src/k_sincosl.h
1 ↗(On Diff #28446)

And here, /*-

lib/msun/src/math.h.orig
1 ↗(On Diff #28446)

And here, /*-

lib/msun/src/s_sincos.c
1 ↗(On Diff #28446)

Ditto. /*-

lib/msun/src/s_sincosf.c
1 ↗(On Diff #28446)

Should this comment block be moved after the copyright block?
I would have guessed so, since "all" the comment is doing is noting
the history of the changes.

7 ↗(On Diff #28446)

Probably should be changed to /*- if the comment blocks are
reordered so this is first.

emaste added inline comments.May 22 2017, 3:54 AM
lib/msun/src/math.h.orig
1 ↗(On Diff #28446)

Except that we don't expect to add .orig files :)

emaste requested changes to this revision.May 22 2017, 3:59 AM
emaste added inline comments.
lib/msun/src/math_private.h
295 ↗(On Diff #28446)

Reported by Steve Kargl,

math_private.h is missing a ENTERV() to match
the RETURNV().  The old patch only includes part of
the ENTERV().  I have attached a patch with missing
part.  This will not apply cleanly to source with 
the old patch applied.  From the new diff, the needed
line is

+#define	ENTERV()

but the new diff contains the entire portion
math_private.h that involved ENTERV and RETURNV.

with an updated patch in PR 218300

This revision now requires changes to proceed.May 22 2017, 3:59 AM
mmel updated this revision to Diff 28662.May 22 2017, 11:53 AM
mmel edited edge metadata.
  • fix style(9) for copyright blocks
  • pair RETURNV() with ENTERV()
mmel updated this revision to Diff 28663.May 22 2017, 12:02 PM
mmel marked 4 inline comments as done.

The one should really save all edited files before updating review :(

mmel marked 3 inline comments as done.May 22 2017, 12:02 PM

No objections - I think you should go ahead. I don't have the background to review it in depth but I trust the author.

dim edited edge metadata.May 24 2017, 6:26 AM

It would be nice to have some tests, even if they are rudimentary... :)

mmel added a comment.May 28 2017, 5:46 AM
In D10765#225464, @dim wrote:

It would be nice to have some tests, even if they are rudimentary... :)

Yes, I agree, thanks for reminder. I hope I'll find some cycles to finish this.

This revision was automatically updated to reflect the committed changes.