From c3eba39110fd26c5d77f5448f387400cc2920021 Mon Sep 17 00:00:00 2001 From: "Alex M. Wells" Date: Mon, 24 Feb 2025 12:37:08 -0800 Subject: [PATCH 1/3] Fix false positives when utilizing OSL's "debug_uninit" feature. It was checking a "while" loops condition variable before the condition had executed causing false positive. It was checking all elements of arrays causing false positives when only a subset of the array is actually being read from in "pointcloud_get", "spline", "splineinverse" calls. Implementation detail: in batched mode, to handle a varying count or knot count parameters a new version of osl_uninit_check_values_offset was added Fix message formatting for batched debug_uninit messages which was printing out pointer to the string instead of the strings themselves. Signed-off-by: Alex M. Wells --- src/liboslexec/batched_llvm_instance.cpp | 114 +++++++++++++++++------ src/liboslexec/builtindecl_wide_xmacro.h | 1 + src/liboslexec/llvm_instance.cpp | 23 ++++- src/liboslexec/wide/wide_shadingsys.cpp | 86 +++++++++++++++-- testsuite/debug-uninit/ref/out-opt2.txt | 2 + testsuite/debug-uninit/ref/out.txt | 2 + testsuite/debug-uninit/test.osl | 24 +++++ 7 files changed, 213 insertions(+), 39 deletions(-) diff --git a/src/liboslexec/batched_llvm_instance.cpp b/src/liboslexec/batched_llvm_instance.cpp index b9bf7423a..4788d706d 100644 --- a/src/liboslexec/batched_llvm_instance.cpp +++ b/src/liboslexec/batched_llvm_instance.cpp @@ -138,6 +138,9 @@ static ustring op_aref("aref"); static ustring op_compref("compref"); static ustring op_mxcompref("mxcompref"); static ustring op_useparam("useparam"); +static ustring op_pointcloud_get("pointcloud_get"); +static ustring op_spline("spline"); +static ustring op_splineinverse("splineinverse"); static ustring unknown_shader_group_name(""); @@ -1620,6 +1623,7 @@ BatchedBackendLLVM::llvm_generate_debug_uninit(const Opcode& op) } llvm::Value* ncheck = ll.constant(int(t.numelements() * t.aggregate)); + llvm::Value* varying_nchecks = nullptr; llvm::Value* offset = ll.constant(0); BatchedBackendLLVM::TempScope temp_scope(*this); llvm::Value* loc_varying_offsets = nullptr; @@ -1701,6 +1705,31 @@ BatchedBackendLLVM::llvm_generate_debug_uninit(const Opcode& op) ll.op_store(comp, loc_varying_offsets); } ncheck = ll.constant(1); + } else if (op.opname() == op_pointcloud_get && i == 2) { + // int pointcloud_get (string ptcname, int indices[], int count, string attr, type data[]) + // will only read indices[0..count-1], so limit the check to count + OSL_ASSERT(3 < op.nargs()); + Symbol& count_sym = *opargsym(op, 3); + if (count_sym.is_uniform()) { + ncheck = llvm_load_value(count_sym); + } else { + // Don't think we can have a uniform indices array that + // has a varying index count + if(!sym.is_uniform()) { + varying_nchecks = ll.void_ptr(llvm_get_pointer(count_sym)); + } + } + } else if (((op.opname() == op_spline) || + (op.opname() == op_splineinverse)) + && i == 4) { + // If an explicit knot count was provided to spline|splineinverse we should + // limit our check of knot values to that count + bool has_knot_count = (op.nargs() == 5); + if (has_knot_count) { + Symbol& knot_count_sym = *opargsym(op, 3); + OSL_ASSERT(knot_count_sym.is_uniform()); + ncheck = llvm_load_value(knot_count_sym); + } } if (loc_varying_offsets != nullptr) { @@ -1724,15 +1753,15 @@ BatchedBackendLLVM::llvm_generate_debug_uninit(const Opcode& op) if (sym.is_uniform()) { ll.call_function(build_name( FuncSpec("uninit_check_values_offset") - .arg_uniform(TypeDesc::PTR) - .arg_varying(TypeInt) + .arg_uniform(TypeDesc::PTR) // vals + .arg_varying(TypeInt) // firstcheck .mask()), args); } else { ll.call_function(build_name( FuncSpec("uninit_check_values_offset") - .arg_varying(TypeDesc::PTR) - .arg_varying(TypeInt) + .arg_varying(TypeDesc::PTR) // vals + .arg_varying(TypeInt) // firstcheck .mask()), args); } @@ -1756,33 +1785,60 @@ BatchedBackendLLVM::llvm_generate_debug_uninit(const Opcode& op) ncheck }; ll.call_function(build_name( FuncSpec("uninit_check_values_offset") - .arg_uniform(TypeDesc::PTR) - .arg_uniform(TypeInt)), + .arg_uniform(TypeDesc::PTR) // vals + .arg_uniform(TypeInt)), // firstcheck args); } else { - llvm::Value* args[] - = { ll.mask_as_int(ll.current_mask()), - ll.constant(t), - llvm_void_ptr(sym), - sg_void_ptr(), - ll.constant(op.sourcefile()), - ll.constant(op.sourceline()), - ll.constant(group().name()), - ll.constant(layer()), - ll.constant(inst()->layername()), - ll.constant(inst()->shadername().c_str()), - ll.constant(int(&op - &inst()->ops()[0])), - ll.constant(op.opname()), - ll.constant(i), - ll.constant(sym.unmangled()), - offset, - ncheck }; - ll.call_function(build_name( - FuncSpec("uninit_check_values_offset") - .arg_varying(TypeDesc::PTR) - .arg_uniform(TypeInt) - .mask()), - args); + if (varying_nchecks != nullptr) { + llvm::Value* args[] + = { ll.mask_as_int(ll.current_mask()), + ll.constant(t), + llvm_void_ptr(sym), + sg_void_ptr(), + ll.constant(op.sourcefile()), + ll.constant(op.sourceline()), + ll.constant(group().name()), + ll.constant(layer()), + ll.constant(inst()->layername()), + ll.constant(inst()->shadername().c_str()), + ll.constant(int(&op - &inst()->ops()[0])), + ll.constant(op.opname()), + ll.constant(i), + ll.constant(sym.unmangled()), + offset, + varying_nchecks }; + ll.call_function(build_name( + FuncSpec("uninit_check_values_offset") + .arg_varying(TypeDesc::PTR) // vals + .arg_uniform(TypeInt) // firstcheck + .arg_varying(TypeInt) // nchecks + .mask()), + args); + } else { + llvm::Value* args[] + = { ll.mask_as_int(ll.current_mask()), + ll.constant(t), + llvm_void_ptr(sym), + sg_void_ptr(), + ll.constant(op.sourcefile()), + ll.constant(op.sourceline()), + ll.constant(group().name()), + ll.constant(layer()), + ll.constant(inst()->layername()), + ll.constant(inst()->shadername().c_str()), + ll.constant(int(&op - &inst()->ops()[0])), + ll.constant(op.opname()), + ll.constant(i), + ll.constant(sym.unmangled()), + offset, + ncheck }; + ll.call_function(build_name( + FuncSpec("uninit_check_values_offset") + .arg_varying(TypeDesc::PTR) // vals + .arg_uniform(TypeInt) // firstcheck + .mask()), + args); + } } } } diff --git a/src/liboslexec/builtindecl_wide_xmacro.h b/src/liboslexec/builtindecl_wide_xmacro.h index f35889bf5..388d20ae2 100644 --- a/src/liboslexec/builtindecl_wide_xmacro.h +++ b/src/liboslexec/builtindecl_wide_xmacro.h @@ -373,6 +373,7 @@ DECL(__OSL_MASKED_OP(range_check), "xXiisXsisiss") DECL(__OSL_OP2(uninit_check_values_offset, X, i), "xLXXsisissisisii") DECL(__OSL_MASKED_OP2(uninit_check_values_offset, X, Wi), "xiLXXsisissisisXi") DECL(__OSL_MASKED_OP2(uninit_check_values_offset, WX, i), "xiLXXsisissisisii") +DECL(__OSL_MASKED_OP3(uninit_check_values_offset, WX, i, Wi), "xiLXXsisissisisiX") DECL(__OSL_MASKED_OP2(uninit_check_values_offset, WX, Wi), "xiLXXsisissisisXi") DECL(__OSL_OP1(get_attribute, s), "iXissiiXXi") diff --git a/src/liboslexec/llvm_instance.cpp b/src/liboslexec/llvm_instance.cpp index f9d51b349..80e852e6d 100644 --- a/src/liboslexec/llvm_instance.cpp +++ b/src/liboslexec/llvm_instance.cpp @@ -143,6 +143,9 @@ static ustring op_aref("aref"); static ustring op_compref("compref"); static ustring op_mxcompref("mxcompref"); static ustring op_useparam("useparam"); +static ustring op_pointcloud_get("pointcloud_get"); +static ustring op_spline("spline"); +static ustring op_splineinverse("splineinverse"); static ustring unknown_shader_group_name(""); @@ -1135,8 +1138,10 @@ BackendLLVM::llvm_generate_debug_uninit(const Opcode& op) // don't generate uninit test code for it. continue; } - if (op.opname() == Strings::op_dowhile && i == 0) { - // The first argument of 'dowhile' is the condition temp, but + if (((op.opname() == Strings::op_dowhile) || + (op.opname() == Strings::op_while)) + && i == 0) { + // The first argument of 'dowhile' or "while" is the condition temp, but // most likely its initializer has not run yet. Unless there is // no "condition" code block, in that case we should still test // it for uninit. @@ -1185,6 +1190,20 @@ BackendLLVM::llvm_generate_debug_uninit(const Opcode& op) comp = ll.op_add(comp, col_ind); offset = comp; ncheck = ll.constant(1); + } else if (op.opname() == op_pointcloud_get && i == 2) { + // int pointcloud_get (string ptcname, int indices[], int count, string attr, type data[]) + // will only read indices[0..count-1], so limit the check to count + OSL_ASSERT(3 < op.nargs()); + ncheck = llvm_load_value(*opargsym(op, 3)); + } else if (((op.opname() == op_spline) || + (op.opname() == op_splineinverse)) + && i == 4) { + // If an explicit knot count was provided to spline|splineinverse we should + // limit our check of knot values to that count + bool has_knot_count = (op.nargs() == 5); + if (has_knot_count) { + ncheck = llvm_load_value(*opargsym(op, 3)); + } } llvm::Value* args[] = { ll.constant(t), diff --git a/src/liboslexec/wide/wide_shadingsys.cpp b/src/liboslexec/wide/wide_shadingsys.cpp index 847705fb2..8d873d75f 100644 --- a/src/liboslexec/wide/wide_shadingsys.cpp +++ b/src/liboslexec/wide/wide_shadingsys.cpp @@ -169,10 +169,10 @@ __OSL_OP2(uninit_check_values_offset, X, ustring layername = USTR(layername_); ctx->errorfmt( "Detected possible use of uninitialized value in {} {} at {}:{} (group {}, layer {} {}, shader {}, op {} '{}', arg {})", - typedesc, symbolname, sourcefile, sourceline, + typedesc, USTR(symbolname), USTR(sourcefile), sourceline, groupname.empty() ? "" : groupname.c_str(), layer, layername.empty() ? "" : layername.c_str(), - shadername, opnum, opname, argnum); + USTR(shadername), opnum, USTR(opname), argnum); } } @@ -233,10 +233,80 @@ __OSL_MASKED_OP2(uninit_check_values_offset, WX, ustring layername = USTR(layername_); ctx->errorfmt( "Detected possible use of uninitialized value in {} {} at {}:{} (group {}, layer {} {}, shader {}, op {} '{}', arg {}) for lanes({:x}) of batch", - typedesc, symbolname, sourcefile, sourceline, + typedesc, USTR(symbolname), USTR(sourcefile), sourceline, groupname.empty() ? "" : groupname.c_str(), layer, layername.empty() ? "" : layername.c_str(), - shadername, opnum, opname, argnum, lanes_uninit.value()); + USTR(shadername), opnum, USTR(opname), argnum, lanes_uninit.value()); + } +} + + + +// Many parameters, but the 3 parameters used in the function name +// correspond to: "vals", "firstcheck", and "nchecks" +OSL_BATCHOP void +__OSL_MASKED_OP3(uninit_check_values_offset, WX, + i, Wi)(int mask_value, long long typedesc_, void* vals_, + void* bsg_, ustring_pod sourcefile, int sourceline, + ustring_pod groupname_, int layer, ustring_pod layername_, + ustring_pod shadername, int opnum, ustring_pod opname, + int argnum, ustring_pod symbolname, int firstcheck, + int *nchecks_) +{ + TypeDesc typedesc = TYPEDESC(typedesc_); + auto* bsg = reinterpret_cast(bsg_); + ShadingContext* ctx = bsg->uniform.context; + const Mask mask(mask_value); + + Mask lanes_uninit(false); + + if (typedesc.basetype == TypeDesc::FLOAT) { + float* vals = (float*)vals_; + mask.foreach ([=, &lanes_uninit](ActiveLane lane) -> void { + int nchecks = nchecks_[lane]; + for (int c = firstcheck, e = firstcheck + nchecks; c < e; ++c) { + if (!std::isfinite(vals[c * __OSL_WIDTH + lane])) { + lanes_uninit.set_on(lane); + vals[c * __OSL_WIDTH + lane] = 0; + } + } + }); + } + if (typedesc.basetype == TypeDesc::INT) { + int* vals = (int*)vals_; + mask.foreach ([=, &lanes_uninit](ActiveLane lane) -> void { + int nchecks = nchecks_[lane]; + for (int c = firstcheck, e = firstcheck + nchecks; c < e; ++c) { + if (vals[c * __OSL_WIDTH + lane] + == std::numeric_limits::min()) { + lanes_uninit.set_on(lane); + vals[c * __OSL_WIDTH + lane] = 0; + } + } + }); + } + if (typedesc.basetype == TypeDesc::STRING) { + ustring* vals = (ustring*)vals_; + mask.foreach ([=, &lanes_uninit](ActiveLane lane) -> void { + int nchecks = nchecks_[lane]; + for (int c = firstcheck, e = firstcheck + nchecks; c < e; ++c) { + if (vals[c * __OSL_WIDTH + lane] + == Strings::uninitialized_string) { + lanes_uninit.set_on(lane); + vals[c * __OSL_WIDTH + lane] = ustring(); + } + } + }); + } + if (lanes_uninit.any_on()) { + ustring groupname = USTR(groupname_); + ustring layername = USTR(layername_); + ctx->errorfmt( + "Detected possible use of uninitialized value in {} {} at {}:{} (group {}, layer {} {}, shader {}, op {} '{}', arg {}) for lanes({:x}) of batch", + typedesc, USTR(symbolname), USTR(sourcefile), sourceline, + groupname.empty() ? "" : groupname.c_str(), layer, + layername.empty() ? "" : layername.c_str(), + USTR(shadername), opnum, USTR(opname), argnum, lanes_uninit.value()); } } @@ -299,10 +369,10 @@ __OSL_MASKED_OP2(uninit_check_values_offset, X, ustring layername = USTR(layername_); ctx->errorfmt( "Detected possible use of uninitialized value in {} {} at {}:{} (group {}, layer {} {}, shader {}, op {} '{}', arg {}) for lanes({:x}) of batch", - typedesc, symbolname, sourcefile, sourceline, + typedesc, USTR(symbolname), USTR(sourcefile), sourceline, groupname.empty() ? "" : groupname.c_str(), layer, layername.empty() ? "" : layername.c_str(), - shadername, opnum, opname, argnum, lanes_uninit.value()); + USTR(shadername), opnum, USTR(opname), argnum, lanes_uninit.value()); } } @@ -367,10 +437,10 @@ __OSL_MASKED_OP2(uninit_check_values_offset, WX, ustring layername = USTR(layername_); ctx->errorfmt( "Detected possible use of uninitialized value in {} {} at {}:{} (group {}, layer {} {}, shader {}, op {} '{}', arg {}) for lanes({:x}) of batch", - typedesc, symbolname, sourcefile, sourceline, + typedesc, USTR(symbolname), USTR(sourcefile), sourceline, groupname.empty() ? "" : groupname.c_str(), layer, layername.empty() ? "" : layername.c_str(), - shadername, opnum, opname, argnum, lanes_uninit.value()); + USTR(shadername), opnum, USTR(opname), argnum, lanes_uninit.value()); } } diff --git a/testsuite/debug-uninit/ref/out-opt2.txt b/testsuite/debug-uninit/ref/out-opt2.txt index 25d3b5860..fdf901da6 100644 --- a/testsuite/debug-uninit/ref/out-opt2.txt +++ b/testsuite/debug-uninit/ref/out-opt2.txt @@ -8,3 +8,5 @@ ERROR: [RendererServices::texture] ImageInput::create() called with no filename ERROR: Detected possible use of uninitialized value in float[3] A at test.osl:22 (group unnamed_group_1, layer 0 test_0, shader test, op 7 'aref', arg 1) ERROR: Detected possible use of uninitialized value in color C at test.osl:28 (group unnamed_group_1, layer 0 test_0, shader test, op 12 'compref', arg 1) ERROR: Detected possible use of uninitialized value in matrix M at test.osl:34 (group unnamed_group_1, layer 0 test_0, shader test, op 17 'mxcompref', arg 1) +ERROR: Detected possible use of uninitialized value in float[20] knots3 at test.osl:54 (group unnamed_group_1, layer 0 test_0, shader test, op 44 'spline', arg 4) +ERROR: Detected possible use of uninitialized value in float[20] knots4 at test.osl:55 (group unnamed_group_1, layer 0 test_0, shader test, op 46 'splineinverse', arg 4) diff --git a/testsuite/debug-uninit/ref/out.txt b/testsuite/debug-uninit/ref/out.txt index b549b7390..a0908b811 100644 --- a/testsuite/debug-uninit/ref/out.txt +++ b/testsuite/debug-uninit/ref/out.txt @@ -8,3 +8,5 @@ ERROR: [RendererServices::texture] ImageInput::create() called with no filename ERROR: Detected possible use of uninitialized value in float[3] A at test.osl:22 (group unnamed_group_1, layer 0 test_0, shader test, op 12 'aref', arg 1) ERROR: Detected possible use of uninitialized value in color C at test.osl:28 (group unnamed_group_1, layer 0 test_0, shader test, op 17 'compref', arg 1) ERROR: Detected possible use of uninitialized value in matrix M at test.osl:34 (group unnamed_group_1, layer 0 test_0, shader test, op 22 'mxcompref', arg 1) +ERROR: Detected possible use of uninitialized value in float[20] knots3 at test.osl:54 (group unnamed_group_1, layer 0 test_0, shader test, op 49 'spline', arg 4) +ERROR: Detected possible use of uninitialized value in float[20] knots4 at test.osl:55 (group unnamed_group_1, layer 0 test_0, shader test, op 51 'splineinverse', arg 4) diff --git a/testsuite/debug-uninit/test.osl b/testsuite/debug-uninit/test.osl index 1bb06aec0..92773383d 100644 --- a/testsuite/debug-uninit/test.osl +++ b/testsuite/debug-uninit/test.osl @@ -33,5 +33,29 @@ test (output color Cout = 0) x += M[1][2]; // NOT an error x += M[0][0]; // An error } + float knots[20]; + float knots2[20]; + float knots3[20]; + float knots4[20]; + int index=0; + // Ensure no false positive for temporary condition variable + while (index < 10) { + knots[index] = x*index; + knots2[index] = (1.0-x)*index; + knots3[index] = 2*x*index; + knots4[index] = 2*(1.0-x)*index; + ++index; + } + // Ensure no false positive for accessing initialized portion of a partially initialized array + x += spline ("linear", x, 10, knots); + x += splineinverse ("linear", x, 10, knots2); + + // Accessing uninitialized portion of a partially initialized array + x += spline ("linear", x, 15, knots3); + x += splineinverse ("linear", x, 15, knots4); + + // pointcloud_get could also have false postive and read partially initialized array + // leaving pointcloud_get out, as not all builds are configured for pointcloud support + Cout[0] += x; // force x results to not be optimized away } From a9bccb529f28814bfafc39818caf9898730c1b11 Mon Sep 17 00:00:00 2001 From: "Alex M. Wells" Date: Mon, 24 Feb 2025 13:13:13 -0800 Subject: [PATCH 2/3] clang formatting changes Signed-off-by: Alex M. Wells --- src/liboslexec/batched_llvm_instance.cpp | 46 ++++++++++++------------ src/liboslexec/builtindecl_wide_xmacro.h | 3 +- src/liboslexec/llvm_instance.cpp | 8 ++--- src/liboslexec/wide/wide_shadingsys.cpp | 26 ++++++++------ 4 files changed, 44 insertions(+), 39 deletions(-) diff --git a/src/liboslexec/batched_llvm_instance.cpp b/src/liboslexec/batched_llvm_instance.cpp index 4788d706d..f1816b3bb 100644 --- a/src/liboslexec/batched_llvm_instance.cpp +++ b/src/liboslexec/batched_llvm_instance.cpp @@ -1624,7 +1624,7 @@ BatchedBackendLLVM::llvm_generate_debug_uninit(const Opcode& op) llvm::Value* ncheck = ll.constant(int(t.numelements() * t.aggregate)); llvm::Value* varying_nchecks = nullptr; - llvm::Value* offset = ll.constant(0); + llvm::Value* offset = ll.constant(0); BatchedBackendLLVM::TempScope temp_scope(*this); llvm::Value* loc_varying_offsets = nullptr; @@ -1709,13 +1709,13 @@ BatchedBackendLLVM::llvm_generate_debug_uninit(const Opcode& op) // int pointcloud_get (string ptcname, int indices[], int count, string attr, type data[]) // will only read indices[0..count-1], so limit the check to count OSL_ASSERT(3 < op.nargs()); - Symbol& count_sym = *opargsym(op, 3); + Symbol& count_sym = *opargsym(op, 3); if (count_sym.is_uniform()) { ncheck = llvm_load_value(count_sym); } else { - // Don't think we can have a uniform indices array that + // Don't think we can have a uniform indices array that // has a varying index count - if(!sym.is_uniform()) { + if (!sym.is_uniform()) { varying_nchecks = ll.void_ptr(llvm_get_pointer(count_sym)); } } @@ -1753,15 +1753,15 @@ BatchedBackendLLVM::llvm_generate_debug_uninit(const Opcode& op) if (sym.is_uniform()) { ll.call_function(build_name( FuncSpec("uninit_check_values_offset") - .arg_uniform(TypeDesc::PTR) // vals - .arg_varying(TypeInt) // firstcheck + .arg_uniform(TypeDesc::PTR) // vals + .arg_varying(TypeInt) // firstcheck .mask()), args); } else { ll.call_function(build_name( FuncSpec("uninit_check_values_offset") - .arg_varying(TypeDesc::PTR) // vals - .arg_varying(TypeInt) // firstcheck + .arg_varying(TypeDesc::PTR) // vals + .arg_varying(TypeInt) // firstcheck .mask()), args); } @@ -1785,8 +1785,8 @@ BatchedBackendLLVM::llvm_generate_debug_uninit(const Opcode& op) ncheck }; ll.call_function(build_name( FuncSpec("uninit_check_values_offset") - .arg_uniform(TypeDesc::PTR) // vals - .arg_uniform(TypeInt)), // firstcheck + .arg_uniform(TypeDesc::PTR) // vals + .arg_uniform(TypeInt)), // firstcheck args); } else { if (varying_nchecks != nullptr) { @@ -1807,13 +1807,13 @@ BatchedBackendLLVM::llvm_generate_debug_uninit(const Opcode& op) ll.constant(sym.unmangled()), offset, varying_nchecks }; - ll.call_function(build_name( - FuncSpec("uninit_check_values_offset") - .arg_varying(TypeDesc::PTR) // vals - .arg_uniform(TypeInt) // firstcheck - .arg_varying(TypeInt) // nchecks - .mask()), - args); + ll.call_function( + build_name(FuncSpec("uninit_check_values_offset") + .arg_varying(TypeDesc::PTR) // vals + .arg_uniform(TypeInt) // firstcheck + .arg_varying(TypeInt) // nchecks + .mask()), + args); } else { llvm::Value* args[] = { ll.mask_as_int(ll.current_mask()), @@ -1832,12 +1832,12 @@ BatchedBackendLLVM::llvm_generate_debug_uninit(const Opcode& op) ll.constant(sym.unmangled()), offset, ncheck }; - ll.call_function(build_name( - FuncSpec("uninit_check_values_offset") - .arg_varying(TypeDesc::PTR) // vals - .arg_uniform(TypeInt) // firstcheck - .mask()), - args); + ll.call_function( + build_name(FuncSpec("uninit_check_values_offset") + .arg_varying(TypeDesc::PTR) // vals + .arg_uniform(TypeInt) // firstcheck + .mask()), + args); } } } diff --git a/src/liboslexec/builtindecl_wide_xmacro.h b/src/liboslexec/builtindecl_wide_xmacro.h index 388d20ae2..1a2314a4c 100644 --- a/src/liboslexec/builtindecl_wide_xmacro.h +++ b/src/liboslexec/builtindecl_wide_xmacro.h @@ -373,7 +373,8 @@ DECL(__OSL_MASKED_OP(range_check), "xXiisXsisiss") DECL(__OSL_OP2(uninit_check_values_offset, X, i), "xLXXsisissisisii") DECL(__OSL_MASKED_OP2(uninit_check_values_offset, X, Wi), "xiLXXsisissisisXi") DECL(__OSL_MASKED_OP2(uninit_check_values_offset, WX, i), "xiLXXsisissisisii") -DECL(__OSL_MASKED_OP3(uninit_check_values_offset, WX, i, Wi), "xiLXXsisissisisiX") +DECL(__OSL_MASKED_OP3(uninit_check_values_offset, WX, i, Wi), + "xiLXXsisissisisiX") DECL(__OSL_MASKED_OP2(uninit_check_values_offset, WX, Wi), "xiLXXsisissisisXi") DECL(__OSL_OP1(get_attribute, s), "iXissiiXXi") diff --git a/src/liboslexec/llvm_instance.cpp b/src/liboslexec/llvm_instance.cpp index 80e852e6d..5c9bbaa81 100644 --- a/src/liboslexec/llvm_instance.cpp +++ b/src/liboslexec/llvm_instance.cpp @@ -1138,8 +1138,8 @@ BackendLLVM::llvm_generate_debug_uninit(const Opcode& op) // don't generate uninit test code for it. continue; } - if (((op.opname() == Strings::op_dowhile) || - (op.opname() == Strings::op_while)) + if (((op.opname() == Strings::op_dowhile) + || (op.opname() == Strings::op_while)) && i == 0) { // The first argument of 'dowhile' or "while" is the condition temp, but // most likely its initializer has not run yet. Unless there is @@ -1195,8 +1195,8 @@ BackendLLVM::llvm_generate_debug_uninit(const Opcode& op) // will only read indices[0..count-1], so limit the check to count OSL_ASSERT(3 < op.nargs()); ncheck = llvm_load_value(*opargsym(op, 3)); - } else if (((op.opname() == op_spline) || - (op.opname() == op_splineinverse)) + } else if (((op.opname() == op_spline) + || (op.opname() == op_splineinverse)) && i == 4) { // If an explicit knot count was provided to spline|splineinverse we should // limit our check of knot values to that count diff --git a/src/liboslexec/wide/wide_shadingsys.cpp b/src/liboslexec/wide/wide_shadingsys.cpp index 8d873d75f..e25c665c5 100644 --- a/src/liboslexec/wide/wide_shadingsys.cpp +++ b/src/liboslexec/wide/wide_shadingsys.cpp @@ -236,7 +236,8 @@ __OSL_MASKED_OP2(uninit_check_values_offset, WX, typedesc, USTR(symbolname), USTR(sourcefile), sourceline, groupname.empty() ? "" : groupname.c_str(), layer, layername.empty() ? "" : layername.c_str(), - USTR(shadername), opnum, USTR(opname), argnum, lanes_uninit.value()); + USTR(shadername), opnum, USTR(opname), argnum, + lanes_uninit.value()); } } @@ -245,13 +246,13 @@ __OSL_MASKED_OP2(uninit_check_values_offset, WX, // Many parameters, but the 3 parameters used in the function name // correspond to: "vals", "firstcheck", and "nchecks" OSL_BATCHOP void -__OSL_MASKED_OP3(uninit_check_values_offset, WX, - i, Wi)(int mask_value, long long typedesc_, void* vals_, - void* bsg_, ustring_pod sourcefile, int sourceline, - ustring_pod groupname_, int layer, ustring_pod layername_, - ustring_pod shadername, int opnum, ustring_pod opname, - int argnum, ustring_pod symbolname, int firstcheck, - int *nchecks_) +__OSL_MASKED_OP3(uninit_check_values_offset, WX, i, + Wi)(int mask_value, long long typedesc_, void* vals_, + void* bsg_, ustring_pod sourcefile, int sourceline, + ustring_pod groupname_, int layer, ustring_pod layername_, + ustring_pod shadername, int opnum, ustring_pod opname, + int argnum, ustring_pod symbolname, int firstcheck, + int* nchecks_) { TypeDesc typedesc = TYPEDESC(typedesc_); auto* bsg = reinterpret_cast(bsg_); @@ -306,7 +307,8 @@ __OSL_MASKED_OP3(uninit_check_values_offset, WX, typedesc, USTR(symbolname), USTR(sourcefile), sourceline, groupname.empty() ? "" : groupname.c_str(), layer, layername.empty() ? "" : layername.c_str(), - USTR(shadername), opnum, USTR(opname), argnum, lanes_uninit.value()); + USTR(shadername), opnum, USTR(opname), argnum, + lanes_uninit.value()); } } @@ -372,7 +374,8 @@ __OSL_MASKED_OP2(uninit_check_values_offset, X, typedesc, USTR(symbolname), USTR(sourcefile), sourceline, groupname.empty() ? "" : groupname.c_str(), layer, layername.empty() ? "" : layername.c_str(), - USTR(shadername), opnum, USTR(opname), argnum, lanes_uninit.value()); + USTR(shadername), opnum, USTR(opname), argnum, + lanes_uninit.value()); } } @@ -440,7 +443,8 @@ __OSL_MASKED_OP2(uninit_check_values_offset, WX, typedesc, USTR(symbolname), USTR(sourcefile), sourceline, groupname.empty() ? "" : groupname.c_str(), layer, layername.empty() ? "" : layername.c_str(), - USTR(shadername), opnum, USTR(opname), argnum, lanes_uninit.value()); + USTR(shadername), opnum, USTR(opname), argnum, + lanes_uninit.value()); } } From 778c97f2289fc2121251e7528ecf34c6e57968da Mon Sep 17 00:00:00 2001 From: "Alex M. Wells" Date: Mon, 24 Feb 2025 15:16:14 -0800 Subject: [PATCH 3/3] more clang formatting Signed-off-by: Alex M. Wells --- src/liboslexec/batched_llvm_instance.cpp | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/liboslexec/batched_llvm_instance.cpp b/src/liboslexec/batched_llvm_instance.cpp index f1816b3bb..5d81ec018 100644 --- a/src/liboslexec/batched_llvm_instance.cpp +++ b/src/liboslexec/batched_llvm_instance.cpp @@ -1719,8 +1719,8 @@ BatchedBackendLLVM::llvm_generate_debug_uninit(const Opcode& op) varying_nchecks = ll.void_ptr(llvm_get_pointer(count_sym)); } } - } else if (((op.opname() == op_spline) || - (op.opname() == op_splineinverse)) + } else if (((op.opname() == op_spline) + || (op.opname() == op_splineinverse)) && i == 4) { // If an explicit knot count was provided to spline|splineinverse we should // limit our check of knot values to that count