sincos opcode

Add a new opcode that computes sin and cos together

[larrygritz, 2010-02-08 23:18:07]

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.

[lg_larrygritz.com, 2010-02-08 23:20:32]

PS. I'll take care of editing the spec to add this function.


On Feb 8, 2010, at 3:18 PM, larrygritz@gmail.com 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.
> 
> 
> http://codereview.appspot.com/205045/show

--
Larry Gritz
lg@larrygritz.com

[ckulla, 2010-02-08 23:22:43]

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?

[lg_imageworks.com, 2010-02-08 23:25:37]

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, <ckulla@gmail.com> 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
lg@imageworks.com

[ckulla, 2010-02-08 23:31:22]

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.

[lg_imageworks.com, 2010-02-08 23:32:32]

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, <ckulla@gmail.com> 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
> lg@imageworks.com
> 
> 
> 

--
Larry Gritz
lg@imageworks.com

[lg_imageworks.com, 2010-02-08 23:48:06]

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, <ckulla@gmail.com> 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-dev@googlegroups.com.
> To unsubscribe from this group, send email to
osl-dev+unsubscribe@googlegroups.com.
> For more options, visit this group at
http://groups.google.com/group/osl-dev?hl=en.
> 
> 

--
Larry Gritz
lg@imageworks.com

[ckulla, 2010-02-09 01:16:10]

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

[ckulla, 2010-02-09 01:27:22]

using portable sincos instead of calls to sin and cos

[ckulla, 2010-02-09 01:28:16]

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;

[blair, 2010-02-09 01:35:15]

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,<ckulla@gmail.com>  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

[lg_larrygritz.com, 2010-02-09 01:35:45]

LGTM!

Feel free to check it into the OIIO trunk.


On Feb 8, 2010, at 5:28 PM, ckulla@gmail.com wrote:

> 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

--
Larry Gritz
lg@larrygritz.com

[sboulos_imageworks.com, 2010-02-09 02:08:34]

I'm surprised this didn't get raised already: does gcc not perform  
this transformation automatically in this case?

On Feb 8, 2010, at 17:35, "Larry Gritz" <lg@larrygritz.com> wrote:

> LGTM!
>
> Feel free to check it into the OIIO trunk.
>
>
> On Feb 8, 2010, at 5:28 PM, ckulla@gmail.com wrote:
>
>> 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
>
> --
> Larry Gritz
> lg@larrygritz.com
>
>
>

[ckulla, 2010-02-09 19:21:10]

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.