Looking at the code coverage report and trying to get a higher
coverage amount, employing two strategies: (a) remove unused code
that is not covered because it's... unused; and (b) test things
that are used but aren't in the test run. Details follow.
Arbitrarily pick a couple of render tests to use llvm_opt levels 12
and 13 to verify that they work correctly. (That's a couple hundred
lines of pass setup code that wouldn't otherwise be included in the
coverage analysis, nor tested in the testsuite.) This also required us
to add the --llvm_opt command to testrender, as it is in testshade.
Remove deprecated version of LLVM_Util::make_function.
When OSL_USE_OPTIX is not enabled, exclude compiling contents of
ptx_compile_group.
get_alignment can also be OSL_DEV only as well as simplified
Signed-off-by: Larry Gritz <lg@larrygritz.com>
Disable contents of dump_struct_data_layout when OSL_DEV is not
defined, it will never be called in any other circumstances -- it's
strictly a debugging tool, not part of the shipping library.
Signed-off-by: Larry Gritz <lg@larrygritz.com>
The immintrin.h header is required for the implementation of the
popcount and coutr_zero function only when the Intel compiler is used.
Move the inclusion accordingly. The current state breaks build on
platforms that don't provide the header, but provide the alternative
means (gcc/clang built-ins, etc).
Signed-off-by: Dan Horák <dan@danny.cz>
We compile testshade.cpp twice -- one for a special "does it compile
as a DSO" check but that isn't run from the testsuite. This was
causing the coverage analysis to think nothing in testshade.cpp (at
least one copy of it) was getting run, which is technically true, and
counting all of its lines as untested for the coverage analysis. Just
exclude that funny dso build from the coverage case.
Also exclude anything in src/build-scripts from coverage analysis,
that's not code that ships, so to speak.
We're trying to chip away at untested code bit by bit to get our
coverage to over 80% of lines. We're getting close.
Signed-off-by: Larry Gritz <lg@larrygritz.com>
Define std::hash for ustring and ustringhash, when not using an OIIO
new enough to do so.
Define std::less for ustringhash, when not using an OIIO new enough
to have operator< defined for ustringhash (will be fixed separately).
No need now for ustringHash, which is also unwisely similar to
ustringhash, just use std::hash instead.
Signed-off-by: Larry Gritz <lg@larrygritz.com>
* Reduce dependency on ShadingContext and ShadingSystem objects
for accessing the ShadingSystem and consequently the ColorSystem. In its current form
it is not device-friendly.
We introduce a self-contained pvt::ShadingStateUniform that maintains a proper
state and is independent of the ShadingContext and ShadingSystem. We add an
opaque pointer to ShadingStateUniform in the ShaderGlobals structure.
Update uses of ColorSystem in opcolor.cpp to go through
ShaderGlobals::shadingStateUniform. Also update uses of m_commonspace_synonym
and m_unknown_coordsys_error in opmatrix.cpp to go through
ShaderGlobals::shadingStateUniform.
NOTE: The ShadingStateUniform is private and an OSL implementation detail and
does not change the public interface other than adding an opaque pointer to
ShaderGlobals.
* Fixed LLVM representation to accommodate ShaderGlobals::shadingStateUniform.
* Added missing Copyright Contributors license to shading_state_uniform.h
* Removed m_commonspace_synonym data member
Moved ShadingStateUniform closer to its use in osl_prepend_matrix_from
* Fixed clang-format diffs. Added void* shadingStateUniform to ShaderGlobals in rend_lib.h. Needed for proper working.
* Added shadingStateUniform to wide_opmatrix and batched_shader_globals.h
Adjusted batched struct data layout check in codegen.
* Reordered headers to include shading_state_uniform.h with the other headers
from liboslexec
Signed-off-by: Steena Monteiro <steena.monteiro@intel.com>
Some explicit SIMD loops were technically illegal because each lane was not truly independent, fixed by adding "reduction" clause. Some Mask objects had to be replaced with just "int" to allow it to be specified in the reduction clause.
icx v2022.2.0 does apppear to have some regressions for large amounts of Dual code in a couple use cases, added alternative(improved) Single Statement Assignment (SSA) version of OIIO::fast_sincos which allowed unit tests to pass. Consider updating OIIO fmath.h to use SSA coding style (which would allow this workaround to be removed).
Another regression with transform_normals was avoiding by introducing an OSL_UNLIKELY(condition) to generate a branch vs. masked execution, this change is a general improvement and should be left in.
Avoid flex_bison produced warning in osogram.cpp "variable 'osonerrs' set but not used".
Due to a number of regressions that appear to be fixed in next revision, choose to skip icx 2022.2 and continue CI for the time being with icx 2022.1 until the next release.
Because multiple (possibly newer) versions of oneapi may be installed, use a config file to specific a specific compiler and tbb version NOTE: Individual oneapi components may independent version numbering.
For centos images, use a config file to specificy a specific oneapi compiler and tbb version
Signed-off-by: Alex M. Wells <alex.m.wells@intel.com>
* For analysis, USE_QT=0 will disable osltoy from building (since we
don't/can't test it in the testsuite, it's unexecuted but really
shouldn't count against our code coverage metric).
* Exclude Imath duplicate headers from Sonar analysis.
Signed-off-by: Larry Gritz <lg@larrygritz.com>
Traditionally, OSL has supported building against the current and also
the previous supported version of OIIO, but not necessarily any older.
With OIIO 2.4 released (any day now), and OSL "main" is actually going
to be next year's supported release, it seems reasonable to change the
forward-looking development of OSL to have a minimum of OIIO 2.3,
which means that OSL-next will support OIIO 2.3, 2.4, and the
currently-in-progress 2.5 (or whatever it's called by that point).
The main reason to do this bump is because we'd like to depend on the
ustringhash class, and possibly other things we will probably be able
to easily backport to 2.3 but not to 2.2.
Signed-off-by: Larry Gritz <lg@larrygritz.com>
* A variety of changes to get a clean build and passing tests when
using LLVM 15.0.
* I've noticed problems when using LLVM 15 but building with earlier
clang, so the cmake scripts now print a warning in that case, so if
users encounter trouble they have a hint about what to do to fix it.
* For our CI tests on Mac, force the MacOS-11 test to use llvm 14,
and the MacOS-12 test to use llvm 15.
IMPORTANT TO-DO / CAVEATS:
1. When doing JIT at optlevel 12 or 13, I had to disable a number of
passes that don't seem to exist anymore in LLVM 15. This is enough
to get it working, and to be honest, I don't know if anybody uses
these opt levels. But we need to revisit this, because I don't
know if there these are cases where the names of the passes merely
changed or that new passes take their place (in which case we
should add the new passes, not stop after merely disabling the
deprecated ones). For that matter, optlevel modes 11, 12, 13 are
supposed to match what clang does for -O1, -O2, -O3, and that
changes from one release to the next, so we should probably revisit
this list and make sure it's matching clang's current choices
(which I assume are crafted and periodically revised by clang's
authors).
2. LLVM 15 switches to "opaque pointers". It still supports typed
pointers... for now. But as you can see in the diffs, I had to
disable a variety of deprecation warnings and take some other
actions to put LLVM 15 in the old "opaque ptr" mode to match our
use of LLVM <= 14. But this is only temporary, because the typed
pointer mode is expected to be removed entirely in LLVM 16. So at
some point in the next few months, we are going to need to support
opaque pointers in our use of LLVM. (Also note: for a while, we're
going to have a bunch of ugly `#if` guards or other logic to
support both opaque pointers for users with llvm >= 16 and also
typed pointers for users with llvm <= 14, until such time as our
LLVM minimum is at least 15, which I expect is not a reasonable
assumption for at least a couple years.)
Signed-off-by: Larry Gritz <lg@larrygritz.com>
Several places in leparse.cpp were found by static analysis to have
memory leaks -- results of calling _parse() that, in the case of
early exists in error conditions, never got deleted.
I also did some other modernization: changed NULL to nullptr, changed
some loops to more compact range-for, utilize some Strutil helpers
that will be more efficient than the alternative std::string
manipulation.
Signed-off-by: Larry Gritz <lg@larrygritz.com>
This was meant to work all along, which is why this patch is so small!
But a tiny detail -- not explicitly passing info about the `--help`
argument -- made ArgParse substitute its own default, which exits
after printing the usage info. But, as I apparently meant to do all
along, if you specify it explicitly, you get to handle it, and the way
we handled it alsp printed the extra information.
Anyway, with this patch, `testshade --help` not only prints all the
testshade command options, but also it prints some information at the
end about some of the build settings of OSL, versions of key
dependencies, hardware info, and a run-down of the ShadingSystem
options. This can be helpful for debugging.
Signed-off-by: Larry Gritz <lg@larrygritz.com>
direnv (https://github.com/direnv/direnv) can be used for local developer workflow enhancements. direnv uses .envrc file for configuration. Because .envrc is usually specific to a developer workflow, it is generally not committed.
Signed-off-by: Mark Sisson <5761292+marksisson@users.noreply.github.com>
- There are a handful of tests we disable entirely with because they
are irrelevant or just cannot be made to be clean (and that's ok).
- Tests with sanitizers run slow, so we increase timeouts, and also
set AA to 1 for render tests to speed them up. We rig it so a
non-matching output won't fail, and that's ok because every other CI
test is checking the results; here we have no reason to believe
results will differ from other tests, we just need to run with
sanitizers turned on.
- In testing.cmake, we rig texture tests to disable the leak
sanitizer, because we can't eliminate some completely benign leaks
from the texture system, and because they are called from JITed
code, we don't get a clean stack trace of things that happen in OIIO
and therefore we just can't enumerate the allowed exceptions. So we
trust that OIIO itself tests the texture system adequately and is
able to ignore the benign "leaks."
- We already submitted PRs and merged the little fixes that were
discovered by enabling address and leak sanitizers. This is just the
final bit to turn it on for CI runs from here on out.
Signed-off-by: Larry Gritz <lg@larrygritz.com>
Rather than specify the version in the usual project() statement,
project (OSL VERSION 1.13.0.0 ...)
this patch decouples it
set (OSL_VERSION "1.13.0.0" CACHE STRING "Version")
project (OSL VERSION ${OSLVERSION} ...)
thus allowing the possibility of overriding the version number at
build confuguration time (`cmake -DOSL_VERSION=3.1.4.1`).
This should be used with extreme caution. Probably never used by most
people. I don't need to describe all the ways that things can go
sideways if you make your build of x.y.z.w actually be arbitrarily
different from the x.y.z.w that everyone else uses and that is
documented.
That said, there are two scenarios I have in mind for which this
ability is very useful:
1. Sometimes, we add or deprecate API features "in advance" by having
an `#if` enable or disable code based on the version, essentially
putting in place changes that can't be enabled yet because they
would break compatibility rules, but we want to integrate the code
now and have it become active upon reaching a particular future
version in which that incompatibility will be allowed. The version
number override makes it easy to test this by quickly doing a build
"as if" we had already bumped the version number to see what will
happen when we actually do.
2. Every once in a while we need to do an internal build that, to
conform to version compatibility rules, needs to appear to still be
part of a past version family. This lets individual sites play some
games with version number overrides rather than have to create new
branches and tags for certain one-off internal releases.
Signed-off-by: Larry Gritz <lg@larrygritz.com>
Signed-off-by: Larry Gritz <lg@larrygritz.com>
This is a tricky situation to describe. Consider this shader:
shader test (float A = 0.5,
float B = A)
{
printf("A = %g, B = %g\n", A, B);
}
By default, A is 0.5 and B gets A's value, also 0.5. If you give A a
different instance value, like 1.0, then B will also get the same
value, they'll both be 1.0. If you give B an instance value, that
will be used for B rather than copying A's value, like this:
$ testshade -param B 1.0 -shader test test
A = 0.5, B = 1
But there is a corner case bug: if we assign B an instance value of
0.0, it will incorrectly fall into a special case where it thinks it
gets a default value, only coincidentally because 0.0 is in an unused
slot for the default value (unused because it has no default -- its
default comes from "init ops", that assignment).
$ testshade -param B 0.0 -shader test test
A = 0.5, B = 0.5
!!!
Reading the patch should be self-explanatory for what the bug is and
how we fix it.
Signed-off-by: Larry Gritz <lg@larrygritz.com>
Signed-off-by: Larry Gritz <lg@larrygritz.com>
I've tried to write a complete set of step-by-step instructions for
everything about how to coordinate and deploy a release. This is a
living document, we will amend over time. I'm sure there must be
things I've forgotten.
Signed-off-by: Larry Gritz <lg@larrygritz.com>
The little ReadEvent class had two members that are never set when
OSL_DEV is not defined, nor are they accessible (the accessor
functions also are disabled based on OSL_DEV). It looked to the static
analyzer like the data members were uninitialized, which they
were. Add OSL_DEV guards around the declarations of those members as
well, so they aren't even present in the struct when not needed for
this debugging mode.
Food for thought: another alternative is to keep the members but just
remove the OSL_DEV guards entirely -- just let the two int data
members be used all the time, not just for OSL_DEV. It's too small to
have any perf penalty, and it unclutters the declaration by removing
all those `#ifdef` lines.
For that matter, I also wonder if there's any real point to having
private members and accessor functions for this tiny internal-only
class, and maybe it's better to just let it be compact declaration
of a simple `struct` with three directly accessible data members.
Signed-off-by: Larry Gritz <lg@larrygritz.com>
In testshade, there was a "loop" that wasn't really a loop, and it
was getting flagged by static analysis as being fishy. This patch
rewrites it as an "if".
Also take the opportunity to change from stream output to OSL::print
formatted output.
Signed-off-by: Larry Gritz <lg@larrygritz.com>
We had hard-coded longer allowable timeouts for particular testsuite
tests. But for the static analysis runs, it takes extra long and we
need a way to tell it to increase the timeouts even more, and to
exclude the longest running tests from the analysis coverage run (it's
not necessary to run those tests, they don't increase coverage enough
to justify being an hours long run).
Signed-off-by: Larry Gritz <lg@larrygritz.com>
Signed-off-by: Larry Gritz <lg@larrygritz.com>
In the course of our runtime optimization of shader networks, we
generate lots of new constant symbols (for example, as we optimize an
"a = b + c" into "a = newconst" if b and c can be determined to be
constants). The new constants are named with a numerical suffix doled
out in a sequental manner.
I've always thought that maybe it would be helpful for me when
debugging optimizer output if the name of the symbol also revealed the
value of the constant it held (when possible). Never did anything
about it.
Now it turns out that this has an application for JIT to PTX, where
the PTX cache can save compilation-to-PTX time if it encounters LLVM
IR that is exactly the same as it has seen before and is cache. If
the constants are generated in a different order, it generates
non-functional, purely text differences in the code we generate, and
this means the PTX cache doesn't function as efficiently as possible.
So this change alters the compiler-generated constant naming for two
cases: int constants, and string constants. The int constants append
the value of the integer (using '_' instead of '-' for negatives), and
the string constants append the hexidecimal value of the ustring hash
for that string. In both cases it does this without using or
incrementing the `m_next_newconst` counter that it uses to
sequentially name the other types that can't easily encode the value
to generate a unique name.
It's really to help the PTX cache, but as a side benefit, it will
be slightly easier for me to make sense of the optimized code when
I need to read it with my own eyes while debugging.
Signed-off-by: Larry Gritz <lg@larrygritz.com>
Today's branching of OIIO 2.4 and subsequent changing of its master
branch to think it's "2.5" seems to have triggered an automatic fuse
that annotates a particular function in OIIO as deprecated for version
2.5 and beyond. OSL was still calling the old version, so some
immediate changes are necessary to call the correct new name when
using a new OIIO.
Signed-off-by: Larry Gritz <lg@larrygritz.com>
* Exclude osltoy from coverage analysis, since we don't have a way to
run it from CI
* Run batch shading for analysis to get better code coverage
* llvm_gen_select: extra check that we can't overrun memory of cond[]
Signed-off-by: Larry Gritz <lg@larrygritz.com>
* Change the languagespec to document the new MaterialX-synchronized
closures.
* Document the old ones as "deprecated" -- still supported for 1.x but
assumed to be on the slate for removal in OSL 2.x.
* Change stdosl.h so that the new closures, instead of being seen only
if MATERIALX_CLOSURES is defined, to being enabled by default, but
(for now) can be disabled if NO_MATERIALX_CLOSURES is defined.
Signed-off-by: Larry Gritz <lg@larrygritz.com>
Removing the suffix ids and callseq comments generated by global counters
leads to deterministic ptx in a multithreaded environment.
Signed-off-by: Chris Hellmuth <chellmuth@gmail.com>
Larry Gritz