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.

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

Add interface to ShadingSystem for Batched Execution #1272

Merged

Conversation

AlexMWells
Copy link
Contributor

Introduce template struct BatchedShaderGlobals to store up to WidthT instances of ShaderGlobal data. The data for a batch is divided into underlying uniform and varying data structures. Uniform section contains a fixed set of shader global values that are shared between all instances. Varying has Block<DataT, WidthT> members so each instance can hold a different value. The Block data structure is specialized to hold the underlying data type in a SIMD friendly Structure of Arrays (SOA) data layout but provides an array subscript interface to a proxy that can import/export the DataT in its orignal data layout.

Introduce template class ShadingSystem::BatchedExecutor as an interface to execute batches of shader globals through a shader network. BatchedExecutor provides similar set of execute methods, but they accept a BatchedShaderGlobals and batchSize instead of individual ShaderGlobal(s). This is just the interface, batched backend implemention to follow in subsequent pull requests.

Introduce template<typename DataT, int WidthT> Wide to hold a reference to Block<DataT, WidthT> and provide access to the underlying SOA data layout. Wide can handle array types and const correctness. The Wide adapter is lightweight and can be copied by value, used as function parameter. Future pull requests will add SIMD versions of OSL library functions and Wide will be the primary was to access SOA data.

Once batched execution finishes, the resulting symbol address can be passed to a Wide<DataT, Width> to access the resulting data in a type safe manner.

Updated testshade with command line option "--batched" causing batched execution of 16 or 8 instances through the shader network and to extract the results. A later pull request will add batched interface to renderer services.

Replaced use of std::bind in testshade in favor of c++11 lambda function.

to store upto WidthT instances of ShaderGlobal data.  The data for a batch is broken into underlying uniform and varying data structures.  Uniform contains a fixed set of shader global values that are shared between all instances.  Varying has Block<DataT, WidthT> members so each instance can hold a different value.  The Block data structure is specialized to hold the underlying data type in a SIMD friendly Structure of Arrays (SOA) data layout but provides an array subscript interface to a proxy that can import/export the DataT in its orignal data layout.

Introduce template<int WidthT> class ShadingSystem::BatchedExecutor as an interface to execute batches of shader globals through a shader network.  BatchedExecutor provides similar execute methods that accept a BatchedShaderGlobals and batchSize instead of individual ShaderGlobal(s). This is just the interface, batched backend implemention to follow in subsequent pull requests.

Introduce template<typename DataT, int WidthT> Wide to hold a reference to Block<DataT, WidthT> and provide access to the underlying SOA data layout.  Wide can handle array types and const correctness.  The Wide adapter is lightweight and can be copied by value, used as function parameter.

Once batched execution finishes, the resulting symbol address can be passed to a Wide<DataT, Width> to access the resulting data in a type safe manner.

Updated testshade with command line option "--batched" causing batched execution of 16 or 8 instances through the shader network and to extract the results.  A later pull request will add batched interface to renderer services.
Copy link
Collaborator

@lgritz lgritz left a comment

Choose a reason for hiding this comment

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

I think this is all fine. I'll double check that it doesn't break the build at work (CI looks fine), then merge.

@@ -171,7 +171,7 @@ class DoubleSpinBox final : public QDoubleSpinBox {
double ss = m_fixed_step;
if (m_variable_step_digits) {
ss = m_variable_min_step;
double av = fabs(valsize);
double av = ::fabs(valsize);
Copy link
Collaborator

Choose a reason for hiding this comment

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

I think this would be more clear as std::fabs

bool
ShadingSystem::supports_batch_execution_at(int width)
{
auto requestedISA = LLVM_Util::lookup_isa_by_name(m_impl->llvm_jit_target().c_str());
Copy link
Collaborator

Choose a reason for hiding this comment

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

lookup_isa_by_name takes a string_view, so the .c_str() is unnecessary here.

@lgritz
Copy link
Collaborator

lgritz commented Oct 19, 2020

If you don't object to the minor fix comments I made, I can add those changes in the process of doing the merge (and probably a teensy bit of formatting fixes as well).

@AlexMWells
Copy link
Contributor Author

Both those changes and any comments you want are fine, go ahead.

@lgritz
Copy link
Collaborator

lgritz commented Oct 19, 2020

Yeah, sure, can you update the PR with those two small changes, then I'll just merge.

@lgritz lgritz merged commit ab37128 into AcademySoftwareFoundation:master Oct 20, 2020
@lgritz
Copy link
Collaborator

lgritz commented Oct 20, 2020

merged!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants