diff --git a/src/cmake/testing.cmake b/src/cmake/testing.cmake index fc5e956b0..b69c6cf1f 100644 --- a/src/cmake/testing.cmake +++ b/src/cmake/testing.cmake @@ -398,6 +398,7 @@ macro (osl_add_all_tests) userdata userdata-defaults userdata-partial userdata-custom userdata-passthrough vararray-connect vararray-default vararray-deserialize vararray-param + vararray-scalar-connect vecctr vector vector-reg wavelength_color wavelength_color-reg Werror xml xml-reg ) diff --git a/src/liboslexec/backendllvm.cpp b/src/liboslexec/backendllvm.cpp index a72854152..89882fcfa 100644 --- a/src/liboslexec/backendllvm.cpp +++ b/src/liboslexec/backendllvm.cpp @@ -742,8 +742,8 @@ BackendLLVM::llvm_test_nonzero(Symbol& val, bool test_derivs) bool -BackendLLVM::llvm_assign_impl(Symbol& Result, Symbol& Src, int arrayindex, - int srccomp, int dstcomp) +BackendLLVM::llvm_assign_impl(Symbol& Result, Symbol& Src, int srcarrayindex, + int dstarrayindex, int srccomp, int dstcomp) { OSL_DASSERT(!Result.typespec().is_structure()); OSL_DASSERT(!Src.typespec().is_structure()); @@ -751,28 +751,31 @@ BackendLLVM::llvm_assign_impl(Symbol& Result, Symbol& Src, int arrayindex, const TypeSpec& result_t(Result.typespec()); const TypeSpec& src_t(Src.typespec()); - llvm::Value* arrind = arrayindex >= 0 ? ll.constant(arrayindex) : NULL; + llvm::Value* sarrind = srcarrayindex >= 0 ? ll.constant(srcarrayindex) + : NULL; + llvm::Value* darrind = dstarrayindex >= 0 ? ll.constant(dstarrayindex) + : NULL; if (Result.typespec().is_closure() || Src.typespec().is_closure()) { if (Src.typespec().is_closure()) { - llvm::Value* srcval = llvm_load_value(Src, 0, arrind, 0); - llvm_store_value(srcval, Result, 0, arrind, 0); + llvm::Value* srcval = llvm_load_value(Src, 0, sarrind, 0); + llvm_store_value(srcval, Result, 0, darrind, 0); } else { llvm::Value* null = ll.constant_ptr(NULL, ll.type_void_ptr()); - llvm_store_value(null, Result, 0, arrind, 0); + llvm_store_value(null, Result, 0, darrind, 0); } return true; } if (Result.typespec().is_matrix() && Src.typespec().is_int_or_float()) { // Handle m=f, m=i separately - llvm::Value* src = llvm_load_value(Src, 0, arrind, 0, + llvm::Value* src = llvm_load_value(Src, 0, sarrind, 0, TypeDesc::FLOAT /*cast*/); // m=f sets the diagonal components to f, the others to zero llvm::Value* zero = ll.constant(0.0f); for (int i = 0; i < 4; ++i) for (int j = 0; j < 4; ++j) - llvm_store_value(i == j ? src : zero, Result, 0, arrind, + llvm_store_value(i == j ? src : zero, Result, 0, darrind, i * 4 + j); llvm_zero_derivs(Result); // matrices don't have derivs currently return true; @@ -786,7 +789,7 @@ BackendLLVM::llvm_assign_impl(Symbol& Result, Symbol& Src, int arrayindex, // will ensure they are the same size, except for certain cases where // the size difference is intended (by the optimizer). if (result_t.is_array() && !Src.is_constant() && src_t.is_array() - && arrayindex == -1) { + && srcarrayindex == -1 && dstarrayindex == -1) { OSL_DASSERT(assignable(result_t.elementtype(), src_t.elementtype())); llvm::Value* resultptr = llvm_get_pointer(Result); llvm::Value* srcptr = llvm_get_pointer(Src); @@ -806,12 +809,14 @@ BackendLLVM::llvm_assign_impl(Symbol& Result, Symbol& Src, int arrayindex, // The following code handles f=f, f=i, v=v, v=f, v=i, m=m, s=s. // Remember that llvm_load_value will automatically convert scalar->triple. - TypeDesc rt = Result.typespec().simpletype(); - TypeDesc basetype = TypeDesc::BASETYPE(rt.basetype); - const int num_components = rt.aggregate; - const bool singlechan = (srccomp != -1) || (dstcomp != -1); - if (!singlechan) { - if (rt.is_array() && arrayindex == -1) { + TypeDesc rt = Result.typespec().simpletype(); + TypeDesc basetype = TypeDesc::BASETYPE(rt.basetype); + const int num_components = rt.aggregate; + const bool singlechan = (srccomp != -1) || (dstcomp != -1); + const bool scalar_to_array_elem = (Src.arraylen() == 0 + && dstarrayindex != -1); + if (!singlechan && !scalar_to_array_elem) { + if (rt.is_array() && srcarrayindex == -1 && dstarrayindex == -1) { // Initialize entire array const int num_elements = std::min(rt.numelements(), src_t.simpletype().numelements()); @@ -836,14 +841,24 @@ BackendLLVM::llvm_assign_impl(Symbol& Result, Symbol& Src, int arrayindex, for (int i = 0; i < num_components; ++i) { llvm::Value* src_val = Src.is_constant() - ? llvm_load_constant_value(Src, arrayindex, i, + ? llvm_load_constant_value(Src, srcarrayindex, i, basetype) - : llvm_load_value(Src, 0, arrind, i, basetype); + : llvm_load_value(Src, 0, sarrind, i, basetype); if (!src_val) return false; - llvm_store_value(src_val, Result, 0, arrind, i); + llvm_store_value(src_val, Result, 0, darrind, i); } } + } else if (scalar_to_array_elem) { + for (int i = 0; i < num_components; ++i) { + llvm::Value* src_val + = Src.is_constant() + ? llvm_load_constant_value(Src, -1, i, basetype) + : llvm_load_value(Src, 0, NULL, i, basetype); + if (!src_val) + return false; + llvm_store_value(src_val, Result, 0, darrind, i); + } } else { // connect individual component of an aggregate type // set srccomp to 0 for case when src is actually a float @@ -851,17 +866,18 @@ BackendLLVM::llvm_assign_impl(Symbol& Result, Symbol& Src, int arrayindex, srccomp = 0; llvm::Value* src_val = Src.is_constant() - ? llvm_load_constant_value(Src, arrayindex, srccomp, basetype) - : llvm_load_value(Src, 0, arrind, srccomp, basetype); + ? llvm_load_constant_value(Src, srcarrayindex, srccomp, + basetype) + : llvm_load_value(Src, 0, sarrind, srccomp, basetype); if (!src_val) return false; // write source float into all components when dstcomp == -1, otherwise // the single element requested. if (dstcomp == -1) { for (int i = 0; i < num_components; ++i) - llvm_store_value(src_val, Result, 0, arrind, i); + llvm_store_value(src_val, Result, 0, darrind, i); } else - llvm_store_value(src_val, Result, 0, arrind, dstcomp); + llvm_store_value(src_val, Result, 0, darrind, dstcomp); } // Handle derivatives @@ -871,18 +887,19 @@ BackendLLVM::llvm_assign_impl(Symbol& Result, Symbol& Src, int arrayindex, if (!singlechan) { for (int d = 1; d <= 2; ++d) { for (int i = 0; i < num_components; ++i) { - llvm::Value* val = llvm_load_value(Src, d, arrind, i); - llvm_store_value(val, Result, d, arrind, i); + llvm::Value* val = llvm_load_value(Src, d, sarrind, i); + llvm_store_value(val, Result, d, darrind, i); } } } else { for (int d = 1; d <= 2; ++d) { - llvm::Value* val = llvm_load_value(Src, d, arrind, srccomp); + llvm::Value* val = llvm_load_value(Src, d, sarrind, + srccomp); if (dstcomp == -1) { for (int i = 0; i < num_components; ++i) - llvm_store_value(val, Result, d, arrind, i); + llvm_store_value(val, Result, d, darrind, i); } else - llvm_store_value(val, Result, d, arrind, dstcomp); + llvm_store_value(val, Result, d, darrind, dstcomp); } } } else { diff --git a/src/liboslexec/backendllvm.h b/src/liboslexec/backendllvm.h index 9c0dbaf9c..7cc64df89 100644 --- a/src/liboslexec/backendllvm.h +++ b/src/liboslexec/backendllvm.h @@ -75,7 +75,8 @@ class BackendLLVM final : public OSOProcessorBase { void llvm_create_constant(const Symbol& sym); - void llvm_assign_initial_value(const Symbol& sym, bool force = false); + void llvm_assign_initial_value(const Symbol& sym, bool force = false, + int arrayindex = -1); llvm::LLVMContext& llvm_context() const { return ll.context(); } AllocationMap& named_values() { return m_named_values; } @@ -256,8 +257,9 @@ class BackendLLVM final : public OSOProcessorBase { /// Implementation of Simple assignment. If arrayindex >= 0, in /// designates a particular array index to assign. - bool llvm_assign_impl(Symbol& Result, Symbol& Src, int arrayindex = -1, - int srccomp = -1, int dstcomp = -1); + bool llvm_assign_impl(Symbol& Result, Symbol& Src, int srcarrayindex = -1, + int dstarrayindex = -1, int srccomp = -1, + int dstcomp = -1); /// Convert the name of a global (and its derivative index) into the diff --git a/src/liboslexec/batched_backendllvm.cpp b/src/liboslexec/batched_backendllvm.cpp index e94122ef4..70a72fec3 100644 --- a/src/liboslexec/batched_backendllvm.cpp +++ b/src/liboslexec/batched_backendllvm.cpp @@ -1894,7 +1894,8 @@ BatchedBackendLLVM::llvm_test_nonzero(const Symbol& val, bool test_derivs) bool BatchedBackendLLVM::llvm_assign_impl(const Symbol& Result, const Symbol& Src, - int arrayindex, int srccomp, int dstcomp) + int srcarrayindex, int dstarrayindex, + int srccomp, int dstcomp) { OSL_DEV_ONLY(std::cout << "llvm_assign_impl arrayindex=" << arrayindex << " Result(" << Result.name() @@ -1914,25 +1915,28 @@ BatchedBackendLLVM::llvm_assign_impl(const Symbol& Result, const Symbol& Src, const TypeSpec& result_t(Result.typespec()); const TypeSpec& src_t(Src.typespec()); - llvm::Value* arrind = arrayindex >= 0 ? ll.constant(arrayindex) : NULL; + llvm::Value* sarrind = srcarrayindex >= 0 ? ll.constant(srcarrayindex) + : NULL; + llvm::Value* darrind = dstarrayindex >= 0 ? ll.constant(dstarrayindex) + : NULL; if (Result.typespec().is_closure() || Src.typespec().is_closure()) { if (Src.typespec().is_closure()) { - llvm::Value* srcval = llvm_load_value(Src, 0, arrind, 0, + llvm::Value* srcval = llvm_load_value(Src, 0, sarrind, 0, TypeUnknown, op_is_uniform); - llvm_store_value(srcval, Result, 0, arrind, 0); + llvm_store_value(srcval, Result, 0, darrind, 0); } else { llvm::Value* null_value = ll.constant_ptr(NULL, ll.type_void_ptr()); if (!op_is_uniform) null_value = ll.widen_value(null_value); - llvm_store_value(null_value, Result, 0, arrind, 0); + llvm_store_value(null_value, Result, 0, darrind, 0); } return true; } if (Result.typespec().is_matrix() && Src.typespec().is_int_or_float()) { // Handle m=f, m=i separately - llvm::Value* src = llvm_load_value(Src, 0, arrind, 0, + llvm::Value* src = llvm_load_value(Src, 0, sarrind, 0, TypeDesc::FLOAT /*cast*/, op_is_uniform); // m=f sets the diagonal components to f, the others to zero @@ -1944,7 +1948,7 @@ BatchedBackendLLVM::llvm_assign_impl(const Symbol& Result, const Symbol& Src, for (int i = 0; i < 4; ++i) for (int j = 0; j < 4; ++j) - llvm_store_value(i == j ? src : zero, Result, 0, arrind, + llvm_store_value(i == j ? src : zero, Result, 0, darrind, i * 4 + j); llvm_zero_derivs(Result); // matrices don't have derivs currently return true; @@ -1954,34 +1958,36 @@ BatchedBackendLLVM::llvm_assign_impl(const Symbol& Result, const Symbol& Src, // The following code handles f=f, f=i, v=v, v=f, v=i, m=m, s=s. // Remember that llvm_load_value will automatically convert scalar->triple. - TypeDesc rt = Result.typespec().simpletype(); - TypeDesc basetype = TypeDesc::BASETYPE(rt.basetype); - const int num_components = rt.aggregate; - const bool singlechan = (srccomp != -1) || (dstcomp != -1); + TypeDesc rt = Result.typespec().simpletype(); + TypeDesc basetype = TypeDesc::BASETYPE(rt.basetype); + const int num_components = rt.aggregate; + const bool singlechan = (srccomp != -1) || (dstcomp != -1); + const bool scalar_to_array_elem = (Src.arraylen() == 0 + && dstarrayindex != -1); // Because we are not mem-copying arrays wholesale, // We will add an outer array index loop to copy 1 element or entire array - int start_array_index = arrayindex; + int start_array_index = srcarrayindex; int end_array_index = start_array_index + 1; - if (start_array_index == -1) { + if (srcarrayindex == -1 && dstarrayindex == -1) { if (result_t.is_array() && src_t.is_array()) { - start_array_index = 0; - end_array_index = std::min(result_t.arraylength(), - src_t.arraylength()); + start_array_index = dstarrayindex = srcarrayindex = 0; + end_array_index = std::min(result_t.arraylength(), + src_t.arraylength()); } } - for (arrayindex = start_array_index; arrayindex < end_array_index; - ++arrayindex) { - arrind = arrayindex >= 0 ? ll.constant(arrayindex) : NULL; - - if (!singlechan) { + for (srcarrayindex = start_array_index; srcarrayindex < end_array_index; + ++srcarrayindex, ++dstarrayindex) { + sarrind = srcarrayindex >= 0 ? ll.constant(srcarrayindex) : NULL; + darrind = dstarrayindex >= 0 ? ll.constant(dstarrayindex) : NULL; + if (!singlechan && !scalar_to_array_elem) { for (int i = 0; i < num_components; ++i) { // Automatically handle widening the source value to match the destination's llvm::Value* src_val = Src.is_constant() - ? llvm_load_constant_value(Src, arrayindex, i, + ? llvm_load_constant_value(Src, srcarrayindex, i, basetype, op_is_uniform) - : llvm_load_value(Src, 0, arrind, i, basetype, + : llvm_load_value(Src, 0, sarrind, i, basetype, op_is_uniform); if (!src_val) return false; @@ -1991,7 +1997,19 @@ BatchedBackendLLVM::llvm_assign_impl(const Symbol& Result, const Symbol& Src, << ll.llvm_typenameof(src_val) << std::endl); // The llvm_load_value above should have handled bool to int conversions // when the basetype == Typedesc::INT - llvm_store_value(src_val, Result, 0, arrind, i); + llvm_store_value(src_val, Result, 0, darrind, i); + } + } else if (scalar_to_array_elem) { + for (int i = 0; i < num_components; ++i) { + llvm::Value* src_val + = Src.is_constant() + ? llvm_load_constant_value(Src, -1, i, basetype, + op_is_uniform) + : llvm_load_value(Src, 0, NULL, i, basetype, + op_is_uniform); + if (!src_val) + return false; + llvm_store_value(src_val, Result, 0, darrind, i); } } else { // connect individual component of an aggregate type @@ -2001,9 +2019,9 @@ BatchedBackendLLVM::llvm_assign_impl(const Symbol& Result, const Symbol& Src, // Automatically handle widening the source value to match the destination's llvm::Value* src_val = Src.is_constant() - ? llvm_load_constant_value(Src, arrayindex, srccomp, + ? llvm_load_constant_value(Src, srcarrayindex, srccomp, basetype, op_is_uniform) - : llvm_load_value(Src, 0, arrind, srccomp, basetype, + : llvm_load_value(Src, 0, sarrind, srccomp, basetype, op_is_uniform); if (!src_val) return false; @@ -2015,9 +2033,9 @@ BatchedBackendLLVM::llvm_assign_impl(const Symbol& Result, const Symbol& Src, // the single element requested. if (dstcomp == -1) { for (int i = 0; i < num_components; ++i) - llvm_store_value(src_val, Result, 0, arrind, i); + llvm_store_value(src_val, Result, 0, darrind, i); } else - llvm_store_value(src_val, Result, 0, arrind, dstcomp); + llvm_store_value(src_val, Result, 0, darrind, dstcomp); } // Handle derivatives @@ -2036,25 +2054,25 @@ BatchedBackendLLVM::llvm_assign_impl(const Symbol& Result, const Symbol& Src, if (Src.has_derivs()) { // src and result both have derivs -- copy them // allow a uniform Src to store to a varying Result, - val = llvm_load_value(Src, d, arrind, i, + val = llvm_load_value(Src, d, sarrind, i, TypeUnknown, op_is_uniform); } - llvm_store_value(val, Result, d, arrind, i); + llvm_store_value(val, Result, d, darrind, i); } } else { llvm::Value* val = zero; if (Src.has_derivs()) { // src and result both have derivs -- copy them // allow a uniform Src to store to a varying Result, - val = llvm_load_value(Src, d, arrind, srccomp, + val = llvm_load_value(Src, d, sarrind, srccomp, TypeUnknown, op_is_uniform); } if (dstcomp == -1) { for (int i = 0; i < num_components; ++i) - llvm_store_value(val, Result, d, arrind, i); + llvm_store_value(val, Result, d, darrind, i); } else - llvm_store_value(val, Result, d, arrind, dstcomp); + llvm_store_value(val, Result, d, darrind, dstcomp); } } } diff --git a/src/liboslexec/batched_backendllvm.h b/src/liboslexec/batched_backendllvm.h index 9bafa7767..fce47a577 100644 --- a/src/liboslexec/batched_backendllvm.h +++ b/src/liboslexec/batched_backendllvm.h @@ -74,7 +74,7 @@ class BatchedBackendLLVM : public OSOProcessorBase { void llvm_assign_initial_value(const Symbol& sym, llvm::Value* llvm_initial_shader_mask_value, - bool force = false); + bool force = false, int arrayindex = -1); llvm::LLVMContext& llvm_context() const { return ll.context(); } AllocationMap& named_values() { return m_named_values; } @@ -335,8 +335,8 @@ class BatchedBackendLLVM : public OSOProcessorBase { /// Implementation of Simple assignment. If arrayindex >= 0, in /// designates a particular array index to assign. bool llvm_assign_impl(const Symbol& Result, const Symbol& Src, - int arrayindex = -1, int srccomp = -1, - int dstcomp = -1); + int srcarrayindex = -1, int dstarrayindex = -1, + int srccomp = -1, int dstcomp = -1); /// Convert the name of a global (and its derivative index) into the diff --git a/src/liboslexec/batched_llvm_instance.cpp b/src/liboslexec/batched_llvm_instance.cpp index 8e6ff0a76..8bda1d7ee 100644 --- a/src/liboslexec/batched_llvm_instance.cpp +++ b/src/liboslexec/batched_llvm_instance.cpp @@ -994,8 +994,14 @@ BatchedBackendLLVM::llvm_type_closure_component_wide_ptr() void BatchedBackendLLVM::llvm_assign_initial_value( - const Symbol& sym, llvm::Value* llvm_initial_shader_mask_value, bool force) + const Symbol& sym, llvm::Value* llvm_initial_shader_mask_value, bool force, + int arrayindex) { + // the array-index is intended for sparse param connections and we use + // force to explicitly initialize them, so we expect that anything with a + // valid array index will be a param and is forced. + OSL_DASSERT((sym.symtype() == SymTypeParam && force) || arrayindex == -1); + // Don't write over connections! Connection values are written into // our layer when the earlier layer is run, as part of its code. So // we just don't need to initialize it here at all. @@ -1263,8 +1269,15 @@ BatchedBackendLLVM::llvm_assign_initial_value( OSL_ASSERT((!sym.is_uniform() || !sym.renderer_output()) && "All render outputs should be varying"); - - for (int a = 0, c = 0; a < arraylen; ++a) { + int a = 0, c = 0; + if (arrayindex != -1) { + // If we have an array index, setup the loop variables to only + // execute that index + a = arrayindex; + c = num_components * arrayindex; + arraylen = arrayindex + 1; + } + for (; a < arraylen; ++a) { llvm::Value* arrind = sym.typespec().is_array() ? ll.constant(a) : NULL; if (sym.typespec().is_closure_based()) @@ -2132,6 +2145,78 @@ BatchedBackendLLVM::build_llvm_instance(bool groupentry) llvm_assign_initial_value(s, llvm_initial_shader_mask_value); } + // Track all symbols who needed 'partial' initialization + tsl::robin_set initedsyms; + + // Make a third pass for sparsely connected array parameters. + // Only init the unconnected elements, upstream shaders are responsible for + // initing and overwritting any connected elements. + for (int c = 0, Nc = inst()->nconnections(); c < Nc; ++c) { + const Connection& con(inst()->connection(c)); + + + Symbol* dstsym(inst()->symbol(con.dst.param)); + // Find any sparse connections, and then find any remaining + // unconnected entries to initialize their values here, upstream + // will have handled all the connected ones. + if (con.dst.arrayindex != -1 && con.src.arrayindex == -1 + && initedsyms.count(dstsym) == 0) { + initedsyms.insert(dstsym); + int arraylen = 0; + TypeSpec dstType = dstsym->typespec(); + if (dstType.is_unsized_array()) { + const ShaderInstance::SymOverrideInfo* sym_override + = inst()->instoverride(con.dst.param); + + OSL_DASSERT(sym_override); + + if (sym_override + && (sym_override->valuesource() == Symbol::InstanceVal + || sym_override->valuesource() + == Symbol::ConnectedVal)) { + arraylen = sym_override->arraylen(); + } else { + arraylen = 0; + } + + } else { + arraylen = dstType.numelements(); + } + + std::vector inited(arraylen, false); + unsigned ninit = arraylen; + + if (con.dst.arrayindex < arraylen) { + inited[con.dst.arrayindex] = true; + ninit--; + } + for (int rc = c + 1; rc < Nc && ninit; ++rc) { + const Connection& next(inst()->connection(rc)); + // Allow redundant/overwriting connections, i.e: + // 1. connect layer.value[i] connect layer.value[j] + // 2. connect layer.value connect layer.value + if (inst()->symbol(next.dst.param) == dstsym) { + if (next.dst.arrayindex != -1) { + if (next.dst.arrayindex < arraylen + && !inited[next.dst.arrayindex]) { + inited[next.dst.arrayindex] = true; + --ninit; + } + } else + ninit = 0; + } + } + if (ninit) { + for (int e = 0; e < arraylen; ++e) { + if (!inited[e]) { + llvm_assign_initial_value(*dstsym, initial_shader_mask, + true, e); + } + } + } + } + } + // All the symbols are stack allocated now. if (groupentry) { @@ -2157,9 +2242,6 @@ BatchedBackendLLVM::build_llvm_instance(bool groupentry) ll.op_branch(m_exit_instance_block); // also sets insert point } - // Track all symbols who needed 'partial' initialization - tsl::robin_set initedsyms; - { // The current mask could be altered by early returns or exit // But for copying output parameters to connected shaders, @@ -2183,9 +2265,6 @@ BatchedBackendLLVM::build_llvm_instance(bool groupentry) for (int c = 0, Nc = child->nconnections(); c < Nc; ++c) { const Connection& con(child->connection(c)); if (con.srclayer == this->layer()) { - OSL_ASSERT( - con.src.arrayindex == -1 && con.dst.arrayindex == -1 - && "no support for individual array element connections"); // Validate unsupported connection vecSrc -> vecDst[j] OSL_ASSERT( (con.dst.channel == -1 @@ -2238,8 +2317,11 @@ BatchedBackendLLVM::build_llvm_instance(bool groupentry) // so no need to do it here as well llvm_run_connected_layers(*srcsym, con.src.param); - // Perform actual copy assignment from src to dest sym - llvm_assign_impl(*dstsym, *srcsym, -1, con.src.channel, + // The upstream run will write the value to it's local output + // parameter location, now we just have to copy it to the + // destination layer's input location. + llvm_assign_impl(*dstsym, *srcsym, con.src.arrayindex, + con.dst.arrayindex, con.src.channel, con.dst.channel); } } diff --git a/src/liboslexec/llvm_instance.cpp b/src/liboslexec/llvm_instance.cpp index 368ed4c38..a61eecebd 100644 --- a/src/liboslexec/llvm_instance.cpp +++ b/src/liboslexec/llvm_instance.cpp @@ -537,13 +537,19 @@ BackendLLVM::llvm_create_constant(const Symbol& sym) } void -BackendLLVM::llvm_assign_initial_value(const Symbol& sym, bool force) +BackendLLVM::llvm_assign_initial_value(const Symbol& sym, bool force, + int arrayindex) { + // the array-index is intended for sparse param connections and we use + // force to explicitly initialize them, so we expect that anything with a + // valid array index will be a param and is forced. + OSL_DASSERT((sym.symtype() == SymTypeParam && force) || arrayindex == -1); + // Don't write over connections! Connection values are written into // our layer when the earlier layer is run, as part of its code. So // we just don't need to initialize it here at all. if (!force && sym.valuesource() == Symbol::ConnectedVal - && !sym.typespec().is_closure_based()) + && !sym.typespec().is_closure_based() && arrayindex == -1) return; // For "globals" that are closures, there is nothing to initialize. if (sym.typespec().is_closure_based() && sym.symtype() == SymTypeGlobal) @@ -553,7 +559,7 @@ BackendLLVM::llvm_assign_initial_value(const Symbol& sym, bool force) // assigned to them. Unless they are params, in which case we took // care of it in the group entry point. if (sym.typespec().is_closure_based() && sym.symtype() != SymTypeParam - && sym.symtype() != SymTypeOutputParam) { + && sym.symtype() != SymTypeOutputParam && arrayindex == -1) { llvm_assign_zero(sym); return; } @@ -800,7 +806,15 @@ BackendLLVM::llvm_assign_initial_value(const Symbol& sym, bool force) int num_components = sym.typespec().simpletype().aggregate; TypeSpec elemtype = sym.typespec().elementtype(); int arraylen = std::max(1, sym.typespec().arraylength()); - for (int a = 0, c = 0; a < arraylen; ++a) { + int a = 0, c = 0; + if (arrayindex != -1) { + // If we have an array index, setup the loop variables to only + // execute that index + a = arrayindex; + c = num_components * arrayindex; + arraylen = arrayindex + 1; + } + for (; a < arraylen; ++a) { llvm::Value* arrind = sym.typespec().is_array() ? ll.constant(a) : NULL; if (sym.typespec().is_closure_based()) @@ -1485,6 +1499,77 @@ BackendLLVM::build_llvm_instance(bool groupentry) llvm_assign_initial_value(s); } + // Track all symbols who needed 'partial' initialization + std::unordered_set initedsyms; + + // Make a third pass for sparsely connected array parameters. + // Only init the unconnected elements, upstream shaders are responsible for + // initing and overwritting any connected elements. + for (int c = 0, Nc = inst()->nconnections(); c < Nc; ++c) { + const Connection& con(inst()->connection(c)); + + + Symbol* dstsym(inst()->symbol(con.dst.param)); + // Find any sparse connections, and then find any remaining + // unconnected entries to initialize their values here, upstream + // will have handled all the connected ones. + if (con.dst.arrayindex != -1 && con.src.arrayindex == -1 + && initedsyms.count(dstsym) == 0) { + initedsyms.insert(dstsym); + int arraylen = 0; + TypeSpec dstType = dstsym->typespec(); + if (dstType.is_unsized_array()) { + const ShaderInstance::SymOverrideInfo* sym_override + = inst()->instoverride(con.dst.param); + + OSL_DASSERT(sym_override); + + if (sym_override + && (sym_override->valuesource() == Symbol::InstanceVal + || sym_override->valuesource() + == Symbol::ConnectedVal)) { + arraylen = sym_override->arraylen(); + } else { + arraylen = 0; + } + + } else { + arraylen = dstType.numelements(); + } + + std::vector inited(arraylen, false); + unsigned ninit = arraylen; + + if (con.dst.arrayindex < arraylen) { + inited[con.dst.arrayindex] = true; + ninit--; + } + for (int rc = c + 1; rc < Nc && ninit; ++rc) { + const Connection& next(inst()->connection(rc)); + // Allow redundant/overwriting connections, i.e: + // 1. connect layer.value[i] connect layer.value[j] + // 2. connect layer.value connect layer.value + if (inst()->symbol(next.dst.param) == dstsym) { + if (next.dst.arrayindex != -1) { + if (next.dst.arrayindex < arraylen + && !inited[next.dst.arrayindex]) { + inited[next.dst.arrayindex] = true; + --ninit; + } + } else + ninit = 0; + } + } + if (ninit) { + for (int e = 0; e < arraylen; ++e) { + if (!inited[e]) { + llvm_assign_initial_value(*dstsym, true, e); + } + } + } + } + } + // All the symbols are stack allocated now. if (groupentry) { @@ -1509,9 +1594,6 @@ BackendLLVM::build_llvm_instance(bool groupentry) if (llvm_has_exit_instance_block()) ll.op_branch(m_exit_instance_block); // also sets insert point - // Track all symbols who needed 'partial' initialization - std::unordered_set initedsyms; - // Transfer all of this layer's outputs into the downstream shader's // inputs. for (int layer = this->layer() + 1; layer < group().nlayers(); ++layer) { @@ -1523,9 +1605,6 @@ BackendLLVM::build_llvm_instance(bool groupentry) for (int c = 0, Nc = child->nconnections(); c < Nc; ++c) { const Connection& con(child->connection(c)); if (con.srclayer == this->layer()) { - OSL_ASSERT( - con.src.arrayindex == -1 && con.dst.arrayindex == -1 - && "no support for individual array element connections"); // Validate unsupported connection vecSrc -> vecDst[j] OSL_ASSERT((con.dst.channel == -1 || con.src.type.aggregate() == TypeDesc::SCALAR @@ -1540,6 +1619,7 @@ BackendLLVM::build_llvm_instance(bool groupentry) if (con.dst.channel != -1 && initedsyms.count(dstsym) == 0) { initedsyms.insert(dstsym); std::bitset<32> inited(0); // Only need to be 16 (matrix4) + inited[con.dst.channel] = true; OSL_DASSERT(dstsym->typespec().aggregate() <= inited.size()); unsigned ninit = dstsym->typespec().aggregate() - 1; @@ -1572,9 +1652,11 @@ BackendLLVM::build_llvm_instance(bool groupentry) // so no need to do it here as well llvm_run_connected_layers(*srcsym, con.src.param); - // FIXME -- I'm not sure I understand this. Isn't this - // unnecessary if we wrote to the parameter ourself? - llvm_assign_impl(*dstsym, *srcsym, -1, con.src.channel, + // The upstream run will write the value to it's local output + // parameter location, now we just have to copy it to the + // destination layer's input location. + llvm_assign_impl(*dstsym, *srcsym, con.src.arrayindex, + con.dst.arrayindex, con.src.channel, con.dst.channel); } } diff --git a/src/liboslexec/shadingsys.cpp b/src/liboslexec/shadingsys.cpp index 307d57355..ce6c64211 100644 --- a/src/liboslexec/shadingsys.cpp +++ b/src/liboslexec/shadingsys.cpp @@ -3557,7 +3557,26 @@ ShadingSystemImpl::decode_connected_param(string_view connectionname, return c; } c.arrayindex = index; - if (c.arrayindex >= c.type.arraylength()) { + int arraylen = 0; + if (c.type.is_unsized_array()) { + const ShaderInstance::SymOverrideInfo* sym_override + = inst->instoverride(c.param); + OSL_DASSERT(sym_override); + + if (sym_override->valuesource() != Symbol::InstanceVal + && sym_override->valuesource() != Symbol::ConnectedVal) { + errorfmt( + "ConnectShaders: unable to connect to element of dynamic array " + "parameter \"{}\" without an override to indicate size.", + connectionname); + c.param = -1; + return c; + } + arraylen = sym_override->arraylen(); + } else { + arraylen = c.type.arraylength(); + } + if (c.arrayindex >= arraylen) { errorfmt("ConnectShaders: cannot request array element {} from a {}", connectionname, c.type); c.arrayindex = c.type.arraylength() - 1; // clamp it diff --git a/testsuite/vararray-scalar-connect/BATCHED b/testsuite/vararray-scalar-connect/BATCHED new file mode 100644 index 000000000..e69de29bb diff --git a/testsuite/vararray-scalar-connect/ref/out.txt b/testsuite/vararray-scalar-connect/ref/out.txt new file mode 100644 index 000000000..a5310dadc --- /dev/null +++ b/testsuite/vararray-scalar-connect/ref/out.txt @@ -0,0 +1,20 @@ +Compiled test.osl -> test.oso +Compiled upstream.osl -> upstream.oso +Connect u1.fout4 to t.a[0] +Connect u2.fout3 to t.a[2] +Connect u1.fout[1] to t.a[3] +Connect u2.fout[0] to t.a[4] +a array length 6 + [0] = 24 + [1] = 42 + [2] = 7 + [3] = 22 + [4] = 9 + [5] = 46 +b array length 5 + [0] = 3 + [1] = 4 + [2] = 5 + [3] = 6 + [4] = 7 + diff --git a/testsuite/vararray-scalar-connect/run.py b/testsuite/vararray-scalar-connect/run.py new file mode 100755 index 000000000..7f92dbf30 --- /dev/null +++ b/testsuite/vararray-scalar-connect/run.py @@ -0,0 +1,14 @@ +# Copyright Contributors to the Open Shading Language project. +# SPDX-License-Identifier: BSD-3-Clause +# https://github.com/AcademySoftwareFoundation/OpenShadingLanguage +#!/usr/bin/env python + + +command += testshade("-param:type=float[4] fin 21,22,23,24 -layer u1 upstream \ + -layer u2 upstream \ + -param:type=float[6] a 41,42,43,44,45,46 \ + -layer t test \ + -connect u1 fout4 t a[0] \ + -connect u2 fout3 t a[2] \ + -connect u1 fout[1] t a[3] \ + -connect u2 fout[0] t a[4]") diff --git a/testsuite/vararray-scalar-connect/test.osl b/testsuite/vararray-scalar-connect/test.osl new file mode 100644 index 000000000..f0a17f1ee --- /dev/null +++ b/testsuite/vararray-scalar-connect/test.osl @@ -0,0 +1,17 @@ +// Copyright Contributors to the Open Shading Language project. +// SPDX-License-Identifier: BSD-3-Clause +// https://github.com/AcademySoftwareFoundation/OpenShadingLanguage + +void print_array_contents (string name, float f[]) +{ + printf ("%s array length %d\n", name, arraylength(f)); + for (int i = 0; i < arraylength(f); ++i) + printf (" [%d] = %g\n", i, f[i]); +} + + +shader test (float a[] = {0}, float b[] = {3,4,5,6,7}) +{ + print_array_contents ("a", a); + print_array_contents ("b", b); +} diff --git a/testsuite/vararray-scalar-connect/upstream.osl b/testsuite/vararray-scalar-connect/upstream.osl new file mode 100644 index 000000000..c0ea0fb9d --- /dev/null +++ b/testsuite/vararray-scalar-connect/upstream.osl @@ -0,0 +1,30 @@ +// Copyright Contributors to the Open Shading Language project. +// SPDX-License-Identifier: BSD-3-Clause +// https://github.com/AcademySoftwareFoundation/OpenShadingLanguage + +shader upstream (float fin[4] = {9, 8, 7, 6}, + output float fout[4] = { 0, 0, 0, 0 }, + output float fout1 = 0, + output float fout2 = 0, + output float fout3 = 0, + output float fout4 = 0) +{ + if (1) + { + fout[0] = fin[0]; + fout[1] = fin[1]; + fout[2] = fin[2]; + fout[3] = fin[3]; + } + else + { + fout[0] = 2.1; + fout[1] = 2.2; + fout[2] = 2.3; + fout[3] = 2.4; + } + fout1 = fout[0]; + fout2 = fout[1]; + fout3 = fout[2]; + fout4 = fout[3]; +}