Page MenuHome

T61576: Do Not (Re-)Compile OpenCL kernels
ClosedPublic

Authored by Jeroen Bakker (jbakker) on Feb 22 2019, 2:31 PM.

Details

Summary

The goal of this patch is to have limit the number of times
kernels needs to be compiled and are reused as kernels with
different compile directives can lead to identical same
binaries.

The implementation does this by stripping the compile directives.
and reshuffling kernels so the output is more likely to be the
same.

We focussed on the kernels where it was easy to detect and maintain
(bundle, bake, displace, do_volume and background). More optimizations
could be done but they are probably less obvious.

Merged the data_init and state_buffer_size kernels to split_bundle.

This patch will also remove empty kernels for do_volume and bake
when their features are not enabled.

When using the benchmark files there are less background, bake and
do_volume kernels compiled.

Fix: T61576, T61501, T61466

When implementing I was also thinking of using an enum for opencl_program_name, but didn't find a good solution as a lot of the logic is based on the name itself.

Diff Detail

Repository
rB Blender

Event Timeline

intern/cycles/device/opencl/opencl_split.cpp
1790

Add space back

  • Added space back
  • D4390: some small cleanup
intern/cycles/device/opencl/opencl_split.cpp
59

TODO: remove this program. It is bundled in the split_bundle.

171

TODO: only check on split_bundle

Brecht Van Lommel (brecht) requested changes to this revision.Feb 25 2019, 7:35 PM

Seems generally fine.

intern/cycles/device/device_split_kernel.cpp
71–72

Not needed, delete works fine with NULL.

intern/cycles/device/opencl/opencl_split.cpp
114

Why not &= ~NODE_FEATURE_VOLUME?

128

Is this meant to be &= ~NODE_FEATURE_VOLUME?

This revision now requires changes to proceed.Feb 25 2019, 7:35 PM
Jeroen Bakker (jbakker) marked 3 inline comments as done.Feb 26 2019, 8:53 AM
Jeroen Bakker (jbakker) added inline comments.
intern/cycles/device/opencl/opencl_split.cpp
128

My hypothesis was that background didn't had any surface data so didn't need any surface based features like:

  • NODE_FEATURE_HAIR
  • NODE_FEATURE_BUMP
  • NODE_FEATURE_BUMP_STATE

I left NODE_FEATURE_VOLUME for to the point density texture. But even that one needs a surface.

Going into more detail it is possible to use NODE_FEATURE_BUMP and NODE_FEATURE_BUMP_STATE and NODE_FEATURE_HAIR info in the background, expect that they won't do much...
I will change it for now to your proposal. In the future we might want to have a background specific features set to reduce recompilations.

Jeroen Bakker (jbakker) marked an inline comment as done.
  • Added space back
  • D4390: some small cleanup
  • D4390: Reduce OpenCL Kernel Recompilations
This revision is now accepted and ready to land.Feb 26 2019, 12:34 PM
This revision was automatically updated to reflect the committed changes.