Fix arg types mismatch in noise call (LLVM)

We were calling noise functions osl_psnoise_dfdfdfff with the wrong type for the last two arguments. That was triggering an assert from LLVM. This patch correctly produces floats for those two arguments whose derivatives are ignored.

[ckulla, 2010-08-11 21:50:05]

http://codereview.appspot.com/1976041/diff/1/2
File src/liboslexec/llvm_instance.cpp (right):

http://codereview.appspot.com/1976041/diff/1/2#newcode2420
src/liboslexec/llvm_instance.cpp:2420: bool use_derivs = any_deriv_args && i <
firstperiod && Result.has_derivs() && s->has_derivs() &&
!s->typespec().is_matrix();
Why do we check for !is_matrix() ?

http://codereview.appspot.com/1976041/diff/1/2#newcode2430
src/liboslexec/llvm_instance.cpp:2430: if (s->typespec().simpletype().aggregate
> 1 || use_derivs)
Couldn't this logic be inside of llvm_load_value?

Aren't floats/ints always passed by value, while other types are always passed
by void* ?

[aconty, 2010-08-11 22:00:04]

any_deriv_args && i <
> firstperiod && Result.has_derivs() && s->has_derivs() &&
> !s->typespec().is_matrix();
> Why do we check for !is_matrix() ?

You'll have to ask Larry, I just tried to change the code as less as possible to
make it work.

> 
> http://codereview.appspot.com/1976041/diff/1/2#newcode2430
> src/liboslexec/llvm_instance.cpp:2430: if
(s->typespec().simpletype().aggregate
> > 1 || use_derivs)
> Couldn't this logic be inside of llvm_load_value?

Well, I didn't want to get into that for this review.

> 
> Aren't floats/ints always passed by value, while other types are always passed
> by void* ?

Not sure about this, but it seems that when a symbol has derivatives you can
either grab the value or the pointer to the Dual.

[larrygritz, 2010-08-11 23:16:33]

LGTM

http://codereview.appspot.com/1976041/diff/1/2
File src/liboslexec/llvm_instance.cpp (right):

http://codereview.appspot.com/1976041/diff/1/2#newcode2420
src/liboslexec/llvm_instance.cpp:2420: bool use_derivs = any_deriv_args && i <
firstperiod && Result.has_derivs() && s->has_derivs() &&
!s->typespec().is_matrix();
On 2010/08/11 21:50:05, ckulla wrote:
> Why do we check for !is_matrix() ?

I think we don't currently support matrix derivatives anywhere.

http://codereview.appspot.com/1976041/diff/1/2#newcode2430
src/liboslexec/llvm_instance.cpp:2430: if (s->typespec().simpletype().aggregate
> 1 || use_derivs)
On 2010/08/11 21:50:05, ckulla wrote:
> Couldn't this logic be inside of llvm_load_value?

No, because sometimes you want to pass by value, other times you need a pointer
(like if it's an output arg, or needs the derivs accessable).  llvm_load_value
isn't smart enough to know how it's going to be used.


> Aren't floats/ints always passed by value, while other types are always passed
> by void* ?

Yes.  But also pass by reference (i.e. void*) if it's an output var, or needs
derivs.

[aconty, 2010-08-12 00:22:27]

Checked in to r799

[ckulla, 2010-08-12 02:31:48]

http://codereview.appspot.com/1976041/diff/1/2
File src/liboslexec/llvm_instance.cpp (right):

http://codereview.appspot.com/1976041/diff/1/2#newcode2420
src/liboslexec/llvm_instance.cpp:2420: bool use_derivs = any_deriv_args && i <
firstperiod && Result.has_derivs() && s->has_derivs() &&
!s->typespec().is_matrix();
On 2010/08/11 23:16:33, larrygritz wrote:
> On 2010/08/11 21:50:05, ckulla wrote:
> > Why do we check for !is_matrix() ?
> 
> I think we don't currently support matrix derivatives anywhere.

So wouldn't this be caught by s->has_derivs() ? And which noise function call
takes a matrix as an argument?

>