Bug fix: optimizer interaction with output param that also is userdata

CMakeLists.txt

@@ -472,7 +472,7 @@ TESTSUITE ( aastep allowconnect-err and-or-not-synonyms arithmetic
             transitive-assign
             transform transformc trig typecast
             unknown-instruction
-            userdata
+            userdata userdata-passthrough
             vararray-connect vararray-default
             vararray-deserialize vararray-param
             vecctr vector

src/liboslexec/runtimeoptimize.cpp

@@ -656,7 +656,8 @@ RuntimeOptimizer::add_useparam (SymbolPtrVec &allsyms)
     for (int i = 0;  i < (int)inst()->symbols().size();  ++i) {
         Symbol *s = inst()->symbol(i);
         if (s->symtype() == SymTypeOutputParam &&
-            (s->connected() || (s->valuesource() == Symbol::DefaultVal && s->has_init_ops()))) {
+            (s->connected() || s->connected_down() || s->renderer_output() ||
+             (s->valuesource() == Symbol::DefaultVal && s->has_init_ops()))) {
             outputparams.push_back (i);
             s->initialized (true);
         }
@@ -3173,8 +3174,17 @@ RuntimeOptimizer::run ()
             const OpDescriptor *opd = shadingsys().op_descriptor (op.opname());
             if (! opd)
                 continue;
+            // a non-unused layer with a nontrivial op does something
             if (op.opname() != Strings::end && op.opname() != Strings::useparam)
-                does_nothing = false;  // a non-unused layer with a nontrivial op
+                does_nothing = false;
+            // Useparam of a down-connected or renderer output does something
+            if (op.opname() == Strings::useparam) {
+                for (int i = 0, e = op.nargs(); i < e; ++i) {
+                    Symbol *sym = opargsym (op, i);
+                    if (sym->connected_down() || sym->renderer_output())
+                        does_nothing = false;
+                }
+            }
             if (opd->flags & OpDescriptor::Tex) {
                 // for all the texture ops, arg 1 is the texture name
                 Symbol *sym = opargsym (op, 1);

src/testshade/simplerend.cpp

@@ -395,6 +395,14 @@ SimpleRenderer::get_userdata (bool derivatives, ustring name, TypeDesc type,
         return true;
     }
 
+    if (const OIIO::ParamValue* p = userdata.find_pv(name, type)) {
+        size_t size = p->type().size();
+        memcpy(val, p->data(), size);
+        if (derivatives)
+            memcpy(val, (char*)p->data() + size, 2 * size);
+        return true;
+    }
+
     return false;
 }
 

src/testshade/simplerend.h

@@ -109,6 +109,7 @@ class SimpleRenderer : public RendererServices
 
     ShadingSystem *shadingsys = nullptr;
     OIIO::ParamValueList options;
+    OIIO::ParamValueList userdata;
 
 protected:
     // Camera parameters

src/testshade/testshade.cpp

@@ -101,6 +101,7 @@ static float uscale = 1, vscale = 1;
 static float uoffset = 0, voffset = 0;
 static std::vector<const char*> shader_setup_args;
 static std::string localename = OIIO::Sysutil::getenv("TESTSHADE_LOCALE");
+static OIIO::ParamValueList userdata;
 
 
 
@@ -339,18 +340,11 @@ specify_expr (int argc OSL_MAYBE_UNUSED, const char *argv[])
 
 
 
+// Utility: Add {paramname, stringval} to the given parameter list.
 static void
-action_param (int /*argc*/, const char *argv[])
+add_param (ParamValueList& params, string_view command,
+           string_view paramname, string_view stringval)
 {
-    std::string command = argv[0];
-    bool use_reparam = false;
-    if (OIIO::Strutil::istarts_with(command, "--reparam") ||
-        OIIO::Strutil::istarts_with(command, "-reparam"))
-        use_reparam = true;
-    ParamValueList &params (use_reparam ? reparams : (::params));
-
-    string_view paramname (argv[1]);
-    string_view stringval (argv[2]);
     TypeDesc type = TypeDesc::UNKNOWN;
     bool unlockgeom = false;
     float f[16];
@@ -459,6 +453,21 @@ action_param (int /*argc*/, const char *argv[])
 
 
 
+static void
+action_param(int /*argc*/, const char *argv[])
+{
+    std::string command = argv[0];
+    bool use_reparam = false;
+    if (OIIO::Strutil::istarts_with(command, "--reparam") ||
+        OIIO::Strutil::istarts_with(command, "-reparam"))
+        use_reparam = true;
+    ParamValueList &params (use_reparam ? reparams : (::params));
+
+    add_param(params, command, argv[1], argv[2]);
+}
+
+
+
 // reparam -- just set reparam_layer and then let action_param do all the
 // hard work.
 static void
@@ -499,6 +508,14 @@ stash_shader_arg (int argc, const char* argv[])
 
 
 
+static void
+stash_userdata(int argc, const char* argv[])
+{
+    add_param(userdata, argv[0], argv[1], argv[2]);
+}
+
+
+
 void
 print_info()
 {
@@ -601,6 +618,8 @@ getargs (int argc, const char *argv[])
                 "--offsetst %f %f", &uoffset, &voffset, "", // old name
                 "--scaleuv %f %f", &uscale, &vscale, "Scale s & t texture lookups (default: 1, 1)",
                 "--scalest %f %f", &uscale, &vscale, "", // old name
+                "--userdata %@ %s %s", stash_userdata, nullptr, nullptr,
+                        "Add userdata (args: name value) (options: type=%s)",
                 "--userdata_isconnected", &userdata_isconnected, "Consider lockgeom=0 to be isconnected()",
                 "--locale %s", &localename, "Set a different locale",
                 NULL);
@@ -1531,6 +1550,9 @@ test_shade (int argc, const char *argv[])
         rend->errhandler().verbosity (ErrorHandler::VERBOSE);
     rend->attribute("saveptx", (int)saveptx);
 
+    // Hand the userdata options from the command line over to the renderer
+    rend->userdata.merge(userdata);
+
     // Request a TextureSystem (by default it will be the global shared
     // one). This isn't strictly necessary, if you pass nullptr to
     // ShadingSystem ctr, it will ask for the shared one internally.

testsuite/debug-uninit/ref/out-opt2.txt

@@ -1,10 +1,10 @@
 Compiled test.osl -> test.oso
 
 Output Cout to Cout.tif
-ERROR: Detected possible use of uninitialized value in int i_uninit at test.osl:14 (group unnamed_group_1, layer 0 test_0, shader test, op 0 'assign', arg 1)
-ERROR: Detected possible use of uninitialized value in float f_uninit at test.osl:14 (group unnamed_group_1, layer 0 test_0, shader test, op 1 'color', arg 1)
-ERROR: Detected possible use of uninitialized value in string s_uninit at test.osl:15 (group unnamed_group_1, layer 0 test_0, shader test, op 2 'texture', arg 1)
+ERROR: Detected possible use of uninitialized value in int i_uninit at test.osl:14 (group unnamed_group_1, layer 0 test_0, shader test, op 1 'assign', arg 1)
+ERROR: Detected possible use of uninitialized value in float f_uninit at test.osl:14 (group unnamed_group_1, layer 0 test_0, shader test, op 2 'color', arg 1)
+ERROR: Detected possible use of uninitialized value in string s_uninit at test.osl:15 (group unnamed_group_1, layer 0 test_0, shader test, op 3 'texture', arg 1)
 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 6 '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 11 '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 16 'mxcompref', arg 1)
+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)

testsuite/debug-uninit/ref/out.txt

@@ -1,10 +1,10 @@
 Compiled test.osl -> test.oso
 
 Output Cout to Cout.tif
-ERROR: Detected possible use of uninitialized value in int i_uninit at test.osl:14 (group unnamed_group_1, layer 0 test_0, shader test, op 3 'assign', arg 1)
-ERROR: Detected possible use of uninitialized value in float f_uninit at test.osl:14 (group unnamed_group_1, layer 0 test_0, shader test, op 4 'color', arg 1)
-ERROR: Detected possible use of uninitialized value in string s_uninit at test.osl:15 (group unnamed_group_1, layer 0 test_0, shader test, op 5 'texture', arg 1)
+ERROR: Detected possible use of uninitialized value in int i_uninit at test.osl:14 (group unnamed_group_1, layer 0 test_0, shader test, op 4 'assign', arg 1)
+ERROR: Detected possible use of uninitialized value in float f_uninit at test.osl:14 (group unnamed_group_1, layer 0 test_0, shader test, op 5 'color', arg 1)
+ERROR: Detected possible use of uninitialized value in string s_uninit at test.osl:15 (group unnamed_group_1, layer 0 test_0, shader test, op 6 'texture', arg 1)
 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 11 '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 16 '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 21 'mxcompref', arg 1)
+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)

testsuite/userdata-passthrough/ref/out.txt

@@ -0,0 +1,5 @@
+Compiled test.osl -> test.oso
+
+Output Cd to Cd.tif
+Pixel (0, 0):
+  Cd : 1 1 1

testsuite/userdata-passthrough/run.py

@@ -0,0 +1,7 @@
+#!/usr/bin/env python
+
+# Copyright Contributors to the Open Shading Language project.
+# SPDX-License-Identifier: BSD-3-Clause
+# https://github.com/imageworks/OpenShadingLanguage
+
+command = testshade("--userdata:type=vector Cd 1,1,1 -o Cd Cd.tif --print test")

testsuite/userdata-passthrough/test.osl

@@ -0,0 +1,26 @@
+// Copyright Contributors to the Open Shading Language project.
+// SPDX-License-Identifier: BSD-3-Clause
+// https://github.com/imageworks/OpenShadingLanguage
+
+
+// This is testing the specific combination of an output parameter (which
+// is presumed to be a renderer output or otherwise definitely used) which
+// gets its value from userdata (lockgeom=0).
+//
+// Cd is what we call a "pass-through" value: its initial value is from an
+// input, it gets modified, but it's also an output.
+//
+// Here's the interesting bug, prior to 1.12: If scale is non-1, all is
+// fine, the output will be scale*input_Cd. But when scale is exactly 1.0,
+// the `Cd *= scale` will be optimized away, leaving no ops in the shader
+// that read Cd, and therefore get_userdata will never be called to retrieve
+// the value, and so the output value of Cd will be its default here, 0. But
+// in fact, it should still hold whatever value it has from teh userdata.
+
+
+shader test (float scale = 1,
+             output vector Cd = 0 [[ int lockgeom=0 ]]
+    )
+{
+    Cd *= scale;
+}