Fix batched support for closures with array types (#1630)

Fixed batched support for closures with array types.
Enabled testsuite/userdata-custom/BATCHED (follow up to PR1620/PR1621).

Added modified version of PR1625 (Fix Issues with Derivative of getattribute) from danieldresser-ie
changing default behavior of MaskedData<DataT>::assign_all_from_scalar(const void *) to zero out Dx Dy when DataT has derviatives.
Added testsuite/userdata-custom
Fixed bug in testshades handling of userdata with derivatives.

Added bool LLVM_Util::is_type_array(llvm::Type* type)
      llvm::Type* LLVM_Util::element_type_of(llvm::Type* array_type)

Fixed icx compiler warning with src/liboslexec/osogram.y
Fixed unreferenced function warning in src/testrender/shading.cpp

Signed-off-by: Alex M. Wells <[email protected]>

Author: AlexMWells

src/cmake/testing.cmake

@@ -352,7 +352,7 @@ macro (osl_add_all_tests)
                 transform transform-reg transformc transformc-reg trig trig-reg
                 typecast
                 unknown-instruction
-                userdata userdata-passthrough
+                userdata userdata-custom userdata-passthrough
                 vararray-connect vararray-default
                 vararray-deserialize vararray-param
                 vecctr vector vector-reg

src/include/OSL/llvm_util.h

@@ -638,6 +638,12 @@ class OSLEXECPUBLIC LLVM_Util {
     /// llvm type.
     llvm::Type* type_array(llvm::Type* type, int n);
 
+    /// Return the true if the llvm::Type is an array
+    bool is_type_array(llvm::Type* type);
+
+    /// Return the llvm::Type of element that the array_type is made up of
+    llvm::Type* element_type_of(llvm::Type* array_type);
+
     /// Return an llvm::FunctionType that describes a function with the
     /// given return types, parameter types (in a cspan), and whether it
     /// uses varargs conventions.

src/include/OSL/wide.h

@@ -2845,7 +2845,7 @@ template<int WidthT> class MaskedData {
     // Populate all data lanes of the MaskedData by broadcasting the
     // single instance of data pointed to.
     // Masked off lanes are not overwritten.
-    // Includes value, as well as Dx and Dy when has_derivs() is true
+    // when has_derivs() is true, Dx and Dy will be zeroed
     OSL_FORCEINLINE void assign_all_from_scalar(const void* ptr_data) const;
 
 
@@ -2863,6 +2863,9 @@ template<int WidthT> class MaskedData {
     template<typename DataT>
     OSL_NOINLINE void assign_all_from_scalar_type(const void* ptr_data,
                                                   int deriv_index) const;
+
+    template<typename DataT>
+    OSL_NOINLINE void zero_all_type(int deriv_index) const;
 };
 
 
@@ -3378,6 +3381,33 @@ MaskedData<WidthT>::assign_all_from_scalar_type(const void* ptr_data,
     }
 }
 
+template<int WidthT>
+template<typename DataT>
+OSL_NOINLINE void
+MaskedData<WidthT>::zero_all_type(int deriv_index) const
+{
+    // We can't just do a memset because some lanes may be masked off
+    // Use a SIMD loop to perform masked assignment.
+    auto* dst_blocks = pvt::block_cast<DataT, WidthT>(m_ptr);
+    int elem_count   = static_cast<int>(m_type.numelements());
+    int comp_count   = m_type.aggregate;
+    int deriv_offset = deriv_index * elem_count * comp_count;
+    for (int array_index = 0; array_index < elem_count; ++array_index) {
+        int array_offset = array_index * comp_count;
+        for (int comp_index = 0; comp_index < comp_count; ++comp_index) {
+            int combined_index = deriv_offset + array_offset + comp_index;
+            auto& bdst         = dst_blocks[combined_index];
+
+            Masked<DataT, WidthT> wdest(bdst, mask());
+
+            OSL_OMP_PRAGMA(omp simd simdlen(WidthT))
+            for (int lane = 0; lane < WidthT; ++lane) {
+                wdest[lane] = DataT {};
+            }
+        }
+    }
+}
+
 template<int WidthT>
 OSL_FORCEINLINE void
 MaskedData<WidthT>::assign_all_from_scalar(const void* ptr_data) const
@@ -3393,16 +3423,12 @@ MaskedData<WidthT>::assign_all_from_scalar(const void* ptr_data) const
         assign_all_from_scalar_type<ustring>(ptr_data, /*deriv_index*/ 0);
     }
     if (has_derivs()) {
-        size_t src_size    = type().size();
-        const char* dx_src = reinterpret_cast<const char*>(ptr_data) + src_size;
-        const char* dy_src = dx_src + src_size;
-
         if (isBase32bit) {
-            assign_all_from_scalar_type<int>(dx_src, /*deriv_index*/ 1);
-            assign_all_from_scalar_type<int>(dy_src, /*deriv_index*/ 2);
+            zero_all_type<int>(/*deriv_index*/ 1);
+            zero_all_type<int>(/*deriv_index*/ 2);
         } else {
-            assign_all_from_scalar_type<ustring>(dx_src, /*deriv_index*/ 1);
-            assign_all_from_scalar_type<ustring>(dy_src, /*deriv_index*/ 2);
+            zero_all_type<ustring>(/*deriv_index*/ 1);
+            zero_all_type<ustring>(/*deriv_index*/ 2);
         }
     }
 }

src/liboslexec/batched_llvm_gen.cpp

@@ -7270,15 +7270,40 @@ LLVMGEN(llvm_gen_closure)
             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);
+            llvm::Type* dest_type  = rop.ll.llvm_type(
+                 static_cast<const TypeSpec&>(p.type).simpletype());
+            dest_base = rop.ll.ptr_to_cast(dest_base, dest_type);
+
+            if (num_elements > 1) {
+                OSL_DASSERT(rop.ll.is_type_array(dest_type));
+
+                // The type is an array type,
+                // which means our pointer is to an array type,
+                // not to &[0] (the address of the 1st element).
+                // IE:  typedef int Int5Array[5];
+                //      Int5Array *dest_base;
+                // in contrast to what most of use might consider an array pointer
+                //      int *dest_base;
+                // So if we dereference Int5Array* to with [4], the 5th element,
+                // we would be really be pointing to ((int *)dest_base)[5*4] which is
+                // out of bounds of our 1 instance of Int5Array.
+                // Thus when dealing with arrays we will just dereference it and get the address to get
+                // the address of the 1st element.
+
+                dest_base = rop.ll.GEP(dest_type, dest_base, 0, 0);
+                llvm::Type* element_type = rop.ll.element_type_of(dest_type);
+                dest_type                = element_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);
+                        dest_elem = rop.ll.GEP(dest_type, dest_base, a, c);
+                    else if (num_elements > 1)
+                        dest_elem = rop.ll.GEP(dest_type, dest_base, a);
                     else
                         dest_elem = dest_base;
 

src/liboslexec/llvm_util.cpp

@@ -2728,7 +2728,18 @@ LLVM_Util::type_array(llvm::Type* type, int n)
     return llvm::ArrayType::get(type, n);
 }
 
+bool
+LLVM_Util::is_type_array(llvm::Type* type)
+{
+    return type->isArrayTy();
+}
 
+llvm::Type*
+LLVM_Util::element_type_of(llvm::Type* array_type)
+{
+    OSL_DASSERT(is_type_array(array_type));
+    return array_type->getArrayElementType();
+}
 
 llvm::FunctionType*
 LLVM_Util::type_function(llvm::Type* rettype, cspan<llvm::Type*> params,

src/liboslexec/osogram.y

@@ -27,7 +27,7 @@
 #pragma clang diagnostic ignored "-Wparentheses-equality"
 #endif
 
-#if OSL_CLANG_VERSION >= 150000
+#if (OSL_CLANG_VERSION >= 150000) || (OSL_INTEL_CLANG_VERSION >= 140000)
 #pragma clang diagnostic ignored "-Wunused-but-set-variable"
 #endif
 

src/testrender/shading.cpp

@@ -154,10 +154,11 @@ struct MxConductorParams : public MxMicrofacetBaseParams {
 
     Color3 evalT(float cos_theta) const { return Color3(0.0f); }
 
-    float get_ior() const
-    {
-        return 0;  // no transmission possible
-    }
+    // Avoid function was declared but never referenced
+    // float get_ior() const
+    // {
+    //     return 0;  // no transmission possible
+    // }
 };
 
 struct MxGeneralizedSchlickParams : public MxMicrofacetBaseParams {

src/testshade/simplerend.cpp

@@ -521,7 +521,7 @@ SimpleRenderer::get_userdata(bool derivatives, ustring name, TypeDesc type,
         size_t size = p->type().size();
         memcpy(val, p->data(), size);
         if (derivatives)
-            memcpy(val, (char*)p->data() + size, 2 * size);
+            memset((char*)val + size, 0, 2 * size);
         return true;
     }