Skip to content

Commit

Permalink
More comprehensive warnings about things that don't optimize away
Browse files Browse the repository at this point in the history 
New classes of warnings triggered when enabling option "opt_warnings":

* Creation of new strings mid-shader.
* Access to the characters of strings mid-shader.

We already warned about texture access by name that could not be
turned into a handle.

In all of these cases, we only warn *after* runtime optimization, so that shader
idioms in the source code that appear to do these forbidden things will not warn
as long as they can be reduced to safe operations in the course of the runtime
optimization step.

Signed-off-by: Larry Gritz <[email protected]>
  • Loading branch information
@lgritz
lgritz committed May 2, 2022
1 parent b7a8728 commit 54f2755
Show file tree
Hide file tree
Showing 8 changed files with 133 additions and 47 deletions.
11 changes: 10 additions & 1 deletion src/include/OSL/oslconfig.h.in
View file
Original file line number Diff line number Diff line change
Expand Up @@ -106,7 +106,6 @@ using OIIO::ustringHash;
using OIIO::string_view;
using OIIO::span;
using OIIO::cspan;
using OIIO::Strutil::print;

using OIIO::TypeDesc;
using OIIO::TypeUnknown;
Expand All @@ -123,6 +122,16 @@ using OIIO::TypeFloat2;
using OIIO::TypeVector2;
using OIIO::TypeVector4;

using OIIO::Strutil::print;

template<typename Str, typename... Args>
OSL_NODISCARD inline std::string
fmtformat(const Str& fmt, Args&&... args)
{
return OIIO::Strutil::fmt::format(fmt, std::forward<Args>(args)...);
}



// N.B. SymArena is not really "configuration", but we cram it here for
// lack of a better home.
Expand Down
9 changes: 5 additions & 4 deletions src/include/OSL/oslexec.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -137,10 +137,11 @@ class OSLEXECPUBLIC ShadingSystem
/// int no_noise Replace noise with constant value. (0)
/// int no_pointcloud Skip pointcloud lookups. (0)
/// int exec_repeat How many times to run each group (1).
/// int opt_warnings Warn on certain failure to runtime-optimize
/// certain shader constructs. (0)
/// int gpu_opt_error Consider a hard error if certain shader
/// constructs cannot be optimized away. (0)
/// int opt_warnings Warn on failure to runtime-optimize certain
/// shader constructs. (0)
/// int gpu_opt_error Issue a hard error if certain shader
/// constructs cannot be optimized away, which
/// have no way to run on GPU. (0)
/// 2. Attributes that should be set by applications/renderers that
/// incorporate OSL:
/// string commonspace Name of "common" coord system ("world")
Expand Down
10 changes: 9 additions & 1 deletion src/liboslexec/oslexec_pvt.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -145,7 +145,15 @@ struct OpDescriptor {
simple_assign(simple), flags(flags)
{}

enum FlagValues { None=0, Tex=1, SideEffects=2 };
enum FlagValues {
None = 0,
SideEffects = 1,
Tex = 2,
StrCreate = 4, // creates strings
StrChars = 8, // accesses string characters
PoliceMisc = 16, // misc opcodes to be policed
Police = Tex | StrCreate | StrChars | PoliceMisc
};
};


Expand Down
56 changes: 35 additions & 21 deletions src/liboslexec/runtimeoptimize.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -3310,25 +3310,25 @@ RuntimeOptimizer::run ()


bool
RuntimeOptimizer::police(const Opcode& op, string_view msg, int type)
RuntimeOptimizer::police_(int type, const Opcode& op, string_view msg)
{
if ((type & police_gpu_err) && shadingsys().m_gpu_opt_error) {
shadingcontext()->errorf("Optimization error for GPUs:\n"
" group: %s\n"
" layer: %s\n"
" source: %s:%d\n"
" issue: %s",
group().name(), inst()->layername(),
op.sourcefile(), op.sourceline(), msg);
return true;
} else if ((type & police_opt_warn) && shadingsys().m_opt_warnings) {
shadingcontext()->warningf("Optimization warning:\n"
" group: %s\n"
" layer: %s\n"
" source: %s:%d\n"
" issue: %s",
shadingcontext()->errorfmt("Optimization error for GPUs:\n"
" group: {}\n"
" layer: {}\n"
" source: {}:{}\n"
" issue: {}",
group().name(), inst()->layername(),
op.sourcefile(), op.sourceline(), msg);
return true;
} else if ((type & police_opt_warn) && shadingsys().m_opt_warnings) {
shadingcontext()->warningfmt("Optimization warning:\n"
" group: {}\n"
" layer: {}\n"
" source: {}:{}\n"
" issue: {}",
group().name(), inst()->layername(),
op.sourcefile(), op.sourceline(), msg);
}
return false;
}
Expand All @@ -3354,8 +3354,7 @@ RuntimeOptimizer::check_for_error_calls(bool warn)
if (op.opname() == Strings::error) {
inst()->has_error_op(true);
if (warn)
err |= police (op, "error() call present in optimized shader.",
police_opt_warn);
err |= police(op, "error() call present in optimized shader.");
}
}
}
Expand All @@ -3368,7 +3367,7 @@ bool
RuntimeOptimizer::police_failed_optimizations()
{
bool err = false;
bool do_warn = shadingsys().m_opt_warnings;
int do_warn = shadingsys().m_opt_warnings;
bool do_gpu_err = shadingsys().m_gpu_opt_error;
if (!do_warn && !do_gpu_err)
return false; // no need for any of this expense
Expand All @@ -3382,15 +3381,30 @@ RuntimeOptimizer::police_failed_optimizations()
const OpDescriptor *opd = shadingsys().op_descriptor (op.opname());
if (! opd)
continue;
if (! (opd->flags & OpDescriptor::Police))
continue;
if (opd->flags & OpDescriptor::Tex) {
Symbol *sym = opargsym (op, 1); // arg 1 is texture name
OSL_DASSERT(sym && sym->typespec().is_string());
if (! sym->is_constant()) {
err |= police (op, OIIO::Strutil::sprintf("%s(): texture name cannot be reduced to a constant.",
op.opname()),
police_gpu_err);
err |= police(
police_gpu_err, op,
"{}(): texture name cannot be converted to a handle.",
op.opname());
}
}
if (opd->flags & OpDescriptor::StrChars) {
err |= police(
op,
"{}(): need for string characters couldn't be optimized away.",
op.opname());
}
if (opd->flags & OpDescriptor::StrCreate) {
err |= police(
op,
"{}(): new string creation couldn't be optimized to a constant.",
op.opname());
}
// FIXME: Will add more tests and warnings as we go
}
}
Expand Down
23 changes: 21 additions & 2 deletions src/liboslexec/runtimeoptimize.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -405,8 +405,27 @@ class RuntimeOptimizer final : public OSOProcessorBase {
/// After optimization, check for things that should not be left
/// unoptimized.
bool police_failed_optimizations ();
enum { police_opt_warn = 1, police_gpu_err = 3, police_gpu_err_only = 2 }; // bit field
bool police(const Opcode& op, string_view msg, int type = police_opt_warn);
enum { // bit field
police_opt_warn = 1,
police_gpu_err_only = 2,
police_gpu_err = 3,
police_string_create_only = 4,
police_string_create = 5,
police_string_chars_only = 8,
police_string_chars = 9
};
bool police_(int type, const Opcode& op, string_view msg);

template<typename Str, typename... Args>
bool police(int type, const Opcode& op, const Str& fmt, Args&&... args) {
return police_(type, op, fmtformat(fmt, std::forward<Args>(args)...));
}

template<typename Str, typename... Args>
bool police(const Opcode& op, const Str& fmt, Args&&... args) {
return police_(police_opt_warn, op,
fmtformat(fmt, std::forward<Args>(args)...));
}

private:
int m_optimize; ///< Current optimization level
Expand Down
30 changes: 17 additions & 13 deletions src/liboslexec/shadingsys.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1150,6 +1150,8 @@ shading_system_setup_op_descriptors (ShadingSystemImpl::OpDescriptorMap& op_desc
#define OP(name,ll,fold,simp,flag) OP2(name,name,ll,fold,simp,flag)
#define TEX OpDescriptor::Tex
#define SIDE OpDescriptor::SideEffects
#define STRCHARS OpDescriptor::StrChars
#define STRCREATE OpDescriptor::StrCreate

// name llvmgen folder simple flags
OP (aassign, aassign, aassign, false, 0);
Expand Down Expand Up @@ -1180,7 +1182,7 @@ shading_system_setup_op_descriptors (ShadingSystemImpl::OpDescriptorMap& op_desc
OP (compassign, compassign, compassign, false, 0);
OP (compl, unary_op, compl, true, 0);
OP (compref, compref, compref, true, 0);
OP (concat, generic, concat, true, 0);
OP (concat, generic, concat, true, STRCREATE);
OP (continue, loopmod_op, none, false, 0);
OP (cos, generic, cos, true, 0);
OP (cosh, generic, none, true, 0);
Expand All @@ -1198,7 +1200,7 @@ shading_system_setup_op_descriptors (ShadingSystemImpl::OpDescriptorMap& op_desc
OP (Dz, Dz, deriv, true, 0);
OP (dowhile, loop_op, none, false, 0);
OP (end, end, none, false, 0);
OP (endswith, generic, endswith, true, 0);
OP (endswith, generic, endswith, true, STRCHARS);
OP (environment, environment, none, true, TEX);
OP (eq, compare_op, eq, true, 0);
OP (erf, generic, erf, true, 0);
Expand All @@ -1213,13 +1215,13 @@ shading_system_setup_op_descriptors (ShadingSystemImpl::OpDescriptorMap& op_desc
OP (floor, generic, floor, true, 0);
OP (fmod, modulus, none, true, 0);
OP (for, loop_op, none, false, 0);
OP (format, printf, format, true, 0);
OP (format, printf, format, true, STRCREATE);
OP (fprintf, printf, none, false, SIDE);
OP (functioncall, functioncall, functioncall, false, 0);
OP (functioncall_nr,functioncall_nr, none, false, 0);
OP (ge, compare_op, ge, true, 0);
OP (getattribute, getattribute, getattribute, false, 0);
OP (getchar, generic, getchar, true, 0);
OP (getchar, generic, getchar, true, STRCHARS);
OP (getmatrix, getmatrix, getmatrix, false, 0);
OP (getmessage, getmessage, getmessage, false, 0);
OP (gettextureinfo, gettextureinfo, gettextureinfo,false, TEX);
Expand Down Expand Up @@ -1267,8 +1269,8 @@ shading_system_setup_op_descriptors (ShadingSystemImpl::OpDescriptorMap& op_desc
OP (psnoise, noise, noise, true, 0);
OP (radians, generic, radians, true, 0);
OP (raytype, raytype, raytype, true, 0);
OP (regex_match, regex, none, false, 0);
OP (regex_search, regex, regex_search, false, 0);
OP (regex_match, regex, none, false, STRCHARS);
OP (regex_search, regex, regex_search, false, STRCHARS);
OP (return, return, none, false, 0);
OP (round, generic, none, true, 0);
OP (select, select, select, true, 0);
Expand All @@ -1285,15 +1287,15 @@ shading_system_setup_op_descriptors (ShadingSystemImpl::OpDescriptorMap& op_desc
OP (splineinverse, spline, none, true, 0);
OP (split, split, split, false, 0);
OP (sqrt, generic, sqrt, true, 0);
OP (startswith, generic, none, true, 0);
OP (startswith, generic, none, true, STRCHARS);
OP (step, generic, none, true, 0);
OP (stof, generic, stof, true, 0);
OP (stoi, generic, stoi, true, 0);
OP (strlen, generic, strlen, true, 0);
OP2(strtof,stof, generic, stof, true, 0);
OP2(strtoi,stoi, generic, stoi, true, 0);
OP (stof, generic, stof, true, STRCHARS);
OP (stoi, generic, stoi, true, STRCHARS);
OP (strlen, generic, strlen, true, STRCHARS);
OP2(strtof,stof, generic, stof, true, STRCHARS);
OP2(strtoi,stoi, generic, stoi, true, STRCHARS);
OP (sub, sub, sub, true, 0);
OP (substr, generic, substr, true, 0);
OP (substr, generic, substr, true, STRCHARS | STRCREATE);
OP (surfacearea, get_simple_SG_field, none, true, 0);
OP (tan, generic, none, true, 0);
OP (tanh, generic, none, true, 0);
Expand All @@ -1315,6 +1317,8 @@ shading_system_setup_op_descriptors (ShadingSystemImpl::OpDescriptorMap& op_desc
#undef OP
#undef TEX
#undef SIDE
#undef STRCHARS
#undef STRCREATE
}


Expand Down
26 changes: 22 additions & 4 deletions testsuite/opt-warnings/ref/out.txt
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2,12 +2,30 @@ Compiled test.osl -> test.oso
WARNING: Optimization warning:
group: MyGroup
layer: TestLayer
source: test.osl:17
issue: texture(): texture name cannot be reduced to a constant.
source: test.osl:14
issue: format(): new string creation couldn't be optimized to a constant.
WARNING: Optimization warning:
group: MyGroup
layer: TestLayer
source: test.osl:24
issue: gettextureinfo(): texture name cannot be reduced to a constant.
source: test.osl:18
issue: texture(): texture name cannot be converted to a handle.
WARNING: Optimization warning:
group: MyGroup
layer: TestLayer
source: test.osl:25
issue: gettextureinfo(): texture name cannot be converted to a handle.
WARNING: Optimization warning:
group: MyGroup
layer: TestLayer
source: test.osl:31
issue: concat(): new string creation couldn't be optimized to a constant.
WARNING: Optimization warning:
group: MyGroup
layer: TestLayer
source: test.osl:37
issue: endswith(): need for string characters couldn't be optimized away.
C = 1 0 0
exists = 0
"foo" + bar = foobar
endswith(bar,"help") = 0

15 changes: 14 additions & 1 deletion testsuite/opt-warnings/test.osl
View file
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@
shader test (
// Purposefully make a param that can't be constant-folded
int frame = 1 [[ int lockgeom = 0 ]],
string bar = "bar" [[ int lockgeom = 0 ]],
output color Cout = 0)
{
string varying_name = format("foo%04d.tif", frame);
Expand All @@ -22,6 +23,18 @@ shader test (
{
int exists = 0;
gettextureinfo (varying_name, "exists", exists);
printf ("exists = %d", exists);
printf ("exists = %d\n", exists);
}

// Creation of strings mid-shader
{
string s = concat("foo", bar);
printf ("\"foo\" + bar = %s\n",s);
}

// Access to string characters mid-shader
{
int e = endswith(bar, "help");
printf ("endswith(bar,\"help\") = %d\n", e);
}
}

0 comments on commit 54f2755

Please sign in to comment.