Rs fmt specification #1702

steenax86 · 2023-06-22T19:49:53Z

Description

Add new customization points in RendererServices for reporting errors, warnings, printf, and fprintf.

These customization points accept a fmtlib style format specifier (vs. printf style) whose argument types are represented as EncodedTypes (encodedtypes.h) and can be subsequently decoded via a utility function.

Removes dependence on ShadingContext and OSL::ErrorHandler for logging error, warning, printf, and fprintf calls
for use on CPU and device during Shader execution. ShadingContext and OSL::ErrorHandler remain utilized during shader compilation/JIT.

Journal Buffer approach: each thread gets its own chain of pages within the shared journal buffer for recording errors, warnings, etc. By maintaining a separate chain per thread we can record errors (etc.) in a non-blocking fashion. Shade index is recorded for use by error reporter to sort messages but it is not used at present.
Using a Journal buffer is up to a Renderer. The Renderer use the new customization points (rs_errorfmt, rs_filefmt, rs_printfmt, rs_warningfmt or their virtual function equivalents) to interact with a journal::Writer and journal buffer owned by its RenderState. The Renderer would of have to allocate and initialized the journal buffer with the requested page size and number of threads before executing a shader. After a shader is done executing, the contents of the journal buffer can be transfered back to the host and processed with a journal::Reader, which reports them through a journal::Reporter's virtual interface. Intent is for Renderers's to use the Journal buffer and provide their own overriden version of the journal::Reporter. Testshade provides example usage. For legacy purposes, the default virtual RendererServices will route errors to ShadingSystem's OSL::ErrorHandler, but intent is for Renderer's to switch over to using a Journal buffer or their own custom logging.

NOTE: OSL library functions as well use Renderer Service free functions can directly call OSL::filefmt, OSL::printfmt, OSL::errorfmt, OSL::warningfmt which wrappers to the underlying free functions (who may record the messages into a journal buffer).

Added a new hashcode datatype in typespec.cpp; represented as "h" in function arguments in builtindecl.
To print closures added a new OSL function osl_closure_to_ustringhash to return a ustringhash_pod value directly.

Shader execute functions now require a zero-based thread_index to identify the calling thread; used for journalling.
RendererServices::get_texture_info parameter ShadingContext * changed to ShaderGlobals * (aka ExecutionContext).

ShaderGlobals now tracks both shade_index and thread_index.
As we look to hide/remove ShaderGlobals in the future, we still need a context object to replace the role the ShaderGlobals struct is currently serving (just minus the actual shader global variables). As a first step towards this goal, we introduce the concept of an ExecutionContext and OpaqueExecutionContext. These just alias to ShaderGlobals currently, but start to establish the idea of hiding its contents. Instead of direct memory access to the ExecutionContext/ShaderGlobals new getter functions have been added that accept an OpaqueExecutionContext and provide the requested data. When inlined there should be no performance penalty, and reliance on the ShaderGlobals implementation detail will be reduced. IE:
inline const Vec3 & get_P(OpaqueExecContextPtr oec);
template RenderStateT* get_rs(OpaqueExecContextPtr oec);
inline int get_raytype(OpaqueExecContextPtr oec);
Updated free function renderer services and opcolor to use the ExecContext, but left the rest alone to minimize size of this PR.

We introduce opfmt.cpp that provides shim function for llvm_gen to call, where the shim adapts datatypes from llvm to c++ forwarding calls to the free function renderer service error, warning, print, fileprint functions.

Split llvm_gen_printf into llvm_gen_printf_legacy, for string format calls or when using optix, and the new llvm_gen_print_fmt which will convert the printf style specifier to the fmtlib format that the Renderer Service customization points are expecting.

fprintf by default no longer writes out to a file, is routed to journal::Reporter::report_file_print which could be overriden (example in TestShade to maintain existing behavior). However for security reasons, we do not recommend allowing OSL shader's direct file system but instead just an annotated log entry.

To maintain existing behavior of disallowing output of repeated errors/warnings over a history window, we have provided 2 classes that could be used during reporting: TrackRecentlyReported and Report2ErrorHandler. TrackRecentlyReported can turn on/off limitting errors and warnings and control the history capacity. Report2ErrorHandler simply reports errors to an existing OSL::ErrorHandler to allow for easy integration. However we expect Renderers to not use Report2ErrorHandler and provide their own to directly interact with their logging systems.

Tests

All existing unit tests that generate errors, warnings and use printfand/or fileprintf will exercise our new code path
Added rs_bitcode testing pass that duplicates existing tests with --use_rs_bitcode enabled.
Added additional tests to check errorfmt in opnoise, opmatrix, opcolor
Added tests to cover mapping of printf flags to those of fmtlib.

Checklist:

I have read the contribution guidelines.
I have previously submitted a Contributor License Agreement.
I have updated the documentation, if applicable.
I have ensured that the change is tested somewhere in the testsuite (adding new test cases if necessary).
My code follows the prevailing code style of this project.

steenax86 · 2023-06-24T01:11:15Z

All CI builds that last passed are passing for this PR. Please review.

lgritz · 2023-07-19T19:57:01Z

I'm ok with the design and code, but in just verifying that it all works and won't break anybody, for me all the OptiX tests are failing. I think they are all getting tripped up on the assertion at llvm_util.cpp:3655, something function has not been defined on the GPU side. If you don't have a configuration in which you can actually test that, Steena, I can try to figure out how to fix it on my end.

The other thing I'd like to recommend is that as part of this patch you bump the version number in the top level CMakeLists.txt from 1.13.4.x to 1.13.5.0 to reflect that there is an ABI break introduced with this patch.

lgritz · 2023-07-21T22:04:19Z

src/testshade/testshade.cpp

@@ -337,7 +380,7 @@ add_shader(cspan<const char*> argv)

    set_shadingsys_options();

-    if (inbuffer)  // Request to exercise the buffer-based API calls
+    if (inbuffer)  // Request to exercise the jbuffer-based API calls
