Skip to content

sincos()#25

Merged
ViralBShah merged 3 commits intomasterfrom
sf/sincos
Jul 16, 2013
Merged

sincos()#25
ViralBShah merged 3 commits intomasterfrom
sf/sincos

Conversation

@staticfloat
Copy link
Copy Markdown
Contributor

Adds in implementations for sincos(), an efficient method of computing the sine and cosine of the same angle.

I'm not entirely sure how to test the 80-bit long double versions, so I haven't attempt to make it more efficient than simply calling sin/cos in succession.

ViralBShah added a commit that referenced this pull request Jul 16, 2013
@ViralBShah ViralBShah merged commit 0e10872 into master Jul 16, 2013
@ViralBShah
Copy link
Copy Markdown
Member

Passes tests for me.

@ViralBShah
Copy link
Copy Markdown
Member

Out of curiosity - why did you not just implement the 4-line trivial version that was in libm-test.c?

@staticfloat
Copy link
Copy Markdown
Contributor Author

Perhaps I'm a glutton for punishment, but I wanted to see if the work that is being saved by performing the common calculations only once was significant. Turns out it's not, (at least, not with -O3 enabled. It seems compilers are smart enough to remove the redundant work after all!) but it was an interesting jaunt through sin/cos implementation at the least.

@staticfloat staticfloat deleted the sf/sincos branch July 16, 2013 07:19
@ViralBShah
Copy link
Copy Markdown
Member

:-)

Comment thread src/s_cosf.c
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Whoa, this isn't a formatting change.

So is this faster than the old code?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This change is paired with the additions below on lines 58-59; I swapped the inequalities to instead match the order found in s_sinf.c for easy comparison between the two. There should be no difference in performance, unfortunately. ;)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants