sincos opcode (issue205045)


larry...@...
 

Dude, use the C++ sincos() function to compute the sines and cosines!
You may as well make the underlying implementation faster in the same
way.

Other than that, LGTM.


http://codereview.appspot.com/205045/show


cku...@...
 

On 2010/02/08 23:18:07, larrygritz wrote:
Dude, use the C++ sincos() function to compute the sines and cosines!
You may
as well make the underlying implementation faster in the same way.
Other than that, LGTM.
This is a GNU extension, it isn't part of any standard to my knowledge.
Can you think of a cross platform way of detecting it?

http://codereview.appspot.com/205045/show


Larry Gritz <l...@...>
 

I would put in fmath.h:

#ifndef __GNUC__
... define it in terms of sin and cos...
#endif



On Feb 8, 2010, at 3:22 PM, <cku...@...> wrote:

On 2010/02/08 23:18:07, larrygritz wrote:
Dude, use the C++ sincos() function to compute the sines and cosines!
You may
as well make the underlying implementation faster in the same way.
Other than that, LGTM.
This is a GNU extension, it isn't part of any standard to my knowledge.
Can you think of a cross platform way of detecting it?

http://codereview.appspot.com/205045/show
--
Larry Gritz
l...@...


cku...@...
 

Its more complicated than that. The symbol doesn't show up by default.
You need to do:

#define _GNU_SOURCE
#include <math.h>

And of course, its hard to know where math.h (or cmath) is first
included (this can change depending on the order of inclusion of the
headers).

This only works on linux by the way, os x does not support this
extension as far as I can tell.


http://codereview.appspot.com/205045/show


Larry Gritz <l...@...>
 

Of course, please substitute whatever compiler or version checks *correctly* discern whether sincos is available.


On Feb 8, 2010, at 3:25 PM, Larry Gritz wrote:

I would put in fmath.h:

#ifndef __GNUC__
... define it in terms of sin and cos...
#endif



On Feb 8, 2010, at 3:22 PM, <cku...@...> wrote:

On 2010/02/08 23:18:07, larrygritz wrote:
Dude, use the C++ sincos() function to compute the sines and cosines!
You may
as well make the underlying implementation faster in the same way.
Other than that, LGTM.
This is a GNU extension, it isn't part of any standard to my knowledge.
Can you think of a cross platform way of detecting it?

http://codereview.appspot.com/205045/show
--
Larry Gritz
l...@...


--
Larry Gritz
l...@...


Larry Gritz <l...@...>
 

1. At the top of fmath.h, before #include <cmath>:

#ifndef __MATH_H__
#define _GNU_SOURCE
#endif


2. In fmath.h, define sincos

#if !defined(_GNU_SOURCE) || !defined(__GNUC__) || !defined(__linux__)
inline void sincos (...) {...}
#endif

3. Tough luck for users who #include <math.h> BEFORE fmath.h and don't define _GNU_SOURCE, they'll get the backup definition, that calls sin and cos separately.

This will work for everybody and never be slower than using sin and cos separately. But users who care enough to do the right #defines and #includes will get something faster.


On Feb 8, 2010, at 3:31 PM, <cku...@...> wrote:

Its more complicated than that. The symbol doesn't show up by default.
You need to do:

#define _GNU_SOURCE
#include <math.h>

And of course, its hard to know where math.h (or cmath) is first
included (this can change depending on the order of inclusion of the
headers).

This only works on linux by the way, os x does not support this
extension as far as I can tell.


http://codereview.appspot.com/205045/show

--
You received this message because you are subscribed to the Google Groups "OSL Developers" group.
To post to this group, send email to osl...@....
To unsubscribe from this group, send email to osl...@....
For more options, visit this group at http://groups.google.com/group/osl-dev?hl=en.

--
Larry Gritz
l...@...


cku...@...
 

I found an alternative that gets around having to use _GNU_SOURCE (which
might have other weird side effects).

http://codereview.appspot.com/205045/show


cku...@...
 

using portable sincos instead of calls to sin and cos

http://codereview.appspot.com/205045/show


cku...@...
 

On 2010/02/09 01:27:22, ckulla wrote:
using portable sincos instead of calls to sin and cos
Here is the patch to fmath.h:

Index: src/include/fmath.h
===================================================================
--- src/include/fmath.h (revision 1392)
+++ src/include/fmath.h (working copy)
@@ -779,7 +779,29 @@
}


+inline void
+sincos(float x, float* sine, float* cosine)
+{
+#if defined(__GNUC__) && defined(__linux__)
+ __builtin_sincosf(x, sin, cosine);
+#else
+ *sine = std::sin(x);
+ *cosine = std::cos(x);
+#endif
+}

+inline void
+sincos(double x, double* sine, double* cosine)
+{
+#if defined(__GNUC__) && defined(__linux__)
+ __builtin_sincos(x, sin, cosine);
+#else
+ *sine = std::sin(x);
+ *cosine = std::cos(x);
+#endif
+}
+
+
#ifdef OPENIMAGEIO_NAMESPACE
}; // end namespace OPENIMAGEIO_NAMESPACE
using namespace OPENIMAGEIO_NAMESPACE;


http://codereview.appspot.com/205045/show


Blair Zajac <bl...@...>
 

Solaris provides sincos as part of math.h.

Given that OSL is now an open-source project, probably at some point it'll need to grow a configure based solution to find functions.

If configure was used, then it would build a config.h that would define or not define HAVE_SINCOS and then the code could just rely upon that, instead of trusting a developer to include headers in the correct order or checking if it was a particular platform.

Regards,
Blair

On 02/08/2010 03:48 PM, Larry Gritz wrote:
1. At the top of fmath.h, before #include<cmath>:

#ifndef __MATH_H__
#define _GNU_SOURCE
#endif


2. In fmath.h, define sincos

#if !defined(_GNU_SOURCE) || !defined(__GNUC__) || !defined(__linux__)
inline void sincos (...) {...}
#endif

3. Tough luck for users who #include<math.h> BEFORE fmath.h and don't define _GNU_SOURCE, they'll get the backup definition, that calls sin and cos separately.

This will work for everybody and never be slower than using sin and cos separately. But users who care enough to do the right #defines and #includes will get something faster.


On Feb 8, 2010, at 3:31 PM,<cku...@...> wrote:

Its more complicated than that. The symbol doesn't show up by default.
You need to do:

#define _GNU_SOURCE
#include<math.h>

And of course, its hard to know where math.h (or cmath) is first
included (this can change depending on the order of inclusion of the
headers).

This only works on linux by the way, os x does not support this
extension as far as I can tell.


http://codereview.appspot.com/205045/show


cku...@...
 

gcc documentation suggests that it can (macro TARGET_HAS_SINCOS) - but
our version doesn't seem to do it automatically.

In theory its true that it would be nice to just let the compiler take
care of this.

http://codereview.appspot.com/205045/show