Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

More comprehensive warnings about things that don't optimize away #1497

Merged
merged 1 commit into from
May 2, 2022
Merged

More comprehensive warnings about things that don't optimize away #1497

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
11 changes: 10 additions & 1 deletion src/include/OSL/oslconfig.h.in
View file
Open in desktop
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
Open in desktop
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
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can we turn on gpu_opt_error even if we are running on the cpu? (In order to get a shader into shape before trying the GPU)

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That is 100% the point, yes.

/// 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
Open in desktop
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
Open in desktop
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
Open in desktop
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
Open in desktop
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
Open in desktop
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
Open in desktop
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);
}
}