Fix issue with closure keyparams batched (#1619)

When evaluating varying closure keyword parameters in batched execution,
the value of the first lane in the batch would be used for the whole
batch.

Author: danieldresser-ie

src/liboslexec/batched_llvm_gen.cpp

@@ -7064,7 +7064,7 @@ LLVMGEN(llvm_gen_spline)
 static void
 llvm_gen_keyword_fill(BatchedBackendLLVM& rop, Opcode& op,
                       const ClosureRegistry::ClosureEntry* clentry,
-                      ustring clname, llvm::Value* mem_void_ptr, int argsoffset)
+                      ustring clname, llvm::Value* mem_void_ptr, int argsoffset, int lane_index)
 {
     OSL_DASSERT(((op.nargs() - argsoffset) % 2) == 0);
 
@@ -7088,10 +7088,43 @@ llvm_gen_keyword_fill(BatchedBackendLLVM& rop, Opcode& op,
             if (equivalent(p.type, ValueType) && !strcmp(key->c_str(), p.key)) {
                 // store data
                 OSL_DASSERT(p.offset + p.field_size <= clentry->struct_size);
-                llvm::Value* dst = rop.ll.offset_ptr(mem_void_ptr, p.offset);
-                llvm::Value* src = rop.llvm_void_ptr(Value);
-                rop.ll.op_memcpy(dst, src, (int)p.type.size(),
-                                 4 /* use 4 byte alignment for now */);
+
+                bool arg_is_uniform = Value.is_uniform();
+
+                TypeDesc simpletype(Value.typespec().simpletype());
+                int num_elements   = simpletype.numelements();
+                int num_components = simpletype.aggregate;
+
+                llvm::Value* dest_base = rop.ll.offset_ptr(mem_void_ptr, p.offset);
+                dest_base              = rop.llvm_ptr_cast(dest_base, p.type);
+
+                for (int a = 0; a < num_elements; ++a) {
+                    llvm::Value* arrind = simpletype.arraylen ? rop.ll.constant(a)
+                                                               : NULL;
+                    for (int c = 0; c < num_components; c++) {
+                        llvm::Value* dest_elem;
+                        if (num_components > 1)
+                            dest_elem = rop.ll.GEP(dest_base, a, c);
+                        else
+                            dest_elem = dest_base;
+
+                        // NOTE:  We don't want any uniform arguments to be
+                        // widened, so our typical op_is_uniform doesn't do what we
+                        // want for this when loading.  So just pass arg_is_uniform
+                        // which will avoid widening any uniform arguments.
+                        llvm::Value* loaded
+                            = rop.llvm_load_value(Value, 0, arrind, c,
+                                                  TypeDesc::UNKNOWN,
+                                                  /*op_is_uniform*/ arg_is_uniform,
+                                                  /*index_is_uniform*/ true);
+
+                        if (!arg_is_uniform) {
+                            loaded = rop.ll.op_extract(loaded, lane_index);
+                        }
+                        rop.ll.op_unmasked_store(loaded, dest_elem);
+                    }
+                }
+
                 legal = true;
                 break;
             }
@@ -7275,7 +7308,7 @@ LLVMGEN(llvm_gen_closure)
         }
 
         llvm_gen_keyword_fill(rop, op, clentry, closure_name, mem_void_ptr,
-                              2 + weighted + clentry->nformal);
+                              2 + weighted + clentry->nformal, lane_index);
 
         if (next_block)
             rop.ll.op_branch(next_block);

testsuite/closure/ref/out.txt

@@ -20,6 +20,13 @@ adding holdout:  Ci = (0.25, 0.25, 0.25) * diffuse ((0, 0, 1), "label", "second"
 	+ (1, 1, 1) * emission ()
 	+ (0.25, 0.25, 0.25) * debug ("MyAOV")
 	+ (0.5, 0.5, 0.5) * holdout ()
+adding varying keyword parameter:  Ci = (0.25, 0.25, 0.25) * diffuse ((0, 0, 1), "label", "second")
+	+ (0.25, 0.25, 0.25) * phong ((0, 0, 1), 20, "label", "one")
+	+ (0.5, 0.5, 0.5) * transparent ()
+	+ (1, 1, 1) * emission ()
+	+ (0.25, 0.25, 0.25) * debug ("MyAOV")
+	+ (0.5, 0.5, 0.5) * holdout ()
+	+ (1, 1, 1) * diffuse ((0, 0, 1), "label", "0.000000")
   Ci = (0.5, 0.5, 0.5) * diffuse ((0, 0, 1), "label", "second")
 adding specular term:  Ci = (0.5, 0.5, 0.5) * diffuse ((0, 0, 1), "label", "second")
 	+ (0.5, 0.5, 0.5) * phong ((0, 0, 1), 20, "label", "one")
@@ -41,6 +48,13 @@ adding holdout:  Ci = (0.25, 0.25, 0.25) * diffuse ((0, 0, 1), "label", "second"
 	+ (1, 1, 1) * emission ()
 	+ (0.25, 0.25, 0.25) * debug ("MyAOV")
 	+ (0.5, 0.5, 0.5) * holdout ()
+adding varying keyword parameter:  Ci = (0.25, 0.25, 0.25) * diffuse ((0, 0, 1), "label", "second")
+	+ (0.25, 0.25, 0.25) * phong ((0, 0, 1), 20, "label", "one")
+	+ (0.5, 0.5, 0.5) * transparent ()
+	+ (1, 1, 1) * emission ()
+	+ (0.25, 0.25, 0.25) * debug ("MyAOV")
+	+ (0.5, 0.5, 0.5) * holdout ()
+	+ (1, 1, 1) * diffuse ((0, 0, 1), "label", "1.000000")
   Ci = (0.5, 0.5, 0.5) * diffuse ((0, 0, 1), "label", "second")
 adding specular term:  Ci = (0.5, 0.5, 0.5) * diffuse ((0, 0, 1), "label", "second")
 	+ (0.5, 0.5, 0.5) * phong ((0, 0, 1), 20, "label", "one")
@@ -62,6 +76,13 @@ adding holdout:  Ci = (0.25, 0.25, 0.25) * diffuse ((0, 0, 1), "label", "second"
 	+ (1, 1, 1) * emission ()
 	+ (0.25, 0.25, 0.25) * debug ("MyAOV")
 	+ (0.5, 0.5, 0.5) * holdout ()
+adding varying keyword parameter:  Ci = (0.25, 0.25, 0.25) * diffuse ((0, 0, 1), "label", "second")
+	+ (0.25, 0.25, 0.25) * phong ((0, 0, 1), 20, "label", "one")
+	+ (0.5, 0.5, 0.5) * transparent ()
+	+ (1, 1, 1) * emission ()
+	+ (0.25, 0.25, 0.25) * debug ("MyAOV")
+	+ (0.5, 0.5, 0.5) * holdout ()
+	+ (1, 1, 1) * diffuse ((0, 0, 1), "label", "0.000000")
   Ci = (0.5, 0.5, 0.5) * diffuse ((0, 0, 1), "label", "second")
 adding specular term:  Ci = (0.5, 0.5, 0.5) * diffuse ((0, 0, 1), "label", "second")
 	+ (0.5, 0.5, 0.5) * phong ((0, 0, 1), 20, "label", "one")
@@ -83,4 +104,11 @@ adding holdout:  Ci = (0.25, 0.25, 0.25) * diffuse ((0, 0, 1), "label", "second"
 	+ (1, 1, 1) * emission ()
 	+ (0.25, 0.25, 0.25) * debug ("MyAOV")
 	+ (0.5, 0.5, 0.5) * holdout ()
+adding varying keyword parameter:  Ci = (0.25, 0.25, 0.25) * diffuse ((0, 0, 1), "label", "second")
+	+ (0.25, 0.25, 0.25) * phong ((0, 0, 1), 20, "label", "one")
+	+ (0.5, 0.5, 0.5) * transparent ()
+	+ (1, 1, 1) * emission ()
+	+ (0.25, 0.25, 0.25) * debug ("MyAOV")
+	+ (0.5, 0.5, 0.5) * holdout ()
+	+ (1, 1, 1) * diffuse ((0, 0, 1), "label", "1.000000")
 

testsuite/closure/test.osl

@@ -35,6 +35,11 @@ test (float Kd = 0.5, float Ks = 0.5, float exponent = 20, color opacity = 0.5,
     Ci += 0.5 * holdout();
     printf ("  Ci = %s\n", Ci);
 
+    // add varying keyword parameter
+    printf ("adding varying keyword parameter:");
+    Ci += diffuse( N, "label", format( "%f", u ) );
+    printf ("  Ci = %s\n", Ci);
+
     closure color xclosure = 0;
     xclosure = 0;
 }