Differential D6391

Cycles/OpenCL: Remove NULL PTR Workaround
ClosedPublic
Actions

Authored by Jeroen Bakker (jbakker) on Dec 11 2019, 9:09 AM.

Details

Reviewers

Sergey Sharybin (sergey)

Maniphest Tasks

T71479: Blender 2.81/2.82 - OpenCL - Significan slow render times

Commits

rC9d022af85aa9: Cycles/OpenCL: Remove NULL PTR Workaround
rBf5e37af5a8fd: Cycles/OpenCL: Remove NULL PTR Workaround

Summary

In the current OpenCL implementation we have a work-around for platforms
that didn't support NULL pointers. We used to replace all NULLs and
empty arrays with a pointer to a single byte on the OpenCL Device.

During investigation of T65924: Cycles Render Crash Windows/AMD RX Vega it was asked to remove this work-around
for testing. This change improves the render times.

 SCENE              | BEFORE | AFTER
--------------------+--------+-------
bmw27               | 108    | 89
barbershop_interior | 867    | 673
classroom           | 270    | 173
fishy_cat           | 244    | 196
koro                | 249    | 207
pavillon_barcelona  | 582    | 414

Before:

After:

Difference:

There are some small noise differences. Could that be related to random noise?

Diff Detail

Repository

rB Blender

Branch

cycles-opencl-nullptr (branched from master)

Build Status

Buildable 5965
Build 5965: arc lint + arc unit

Event Timeline

Jeroen Bakker (jbakker) created this revision.Dec 11 2019, 9:09 AM

Harbormaster completed remote builds in B5965: Diff 20203.Dec 11 2019, 9:09 AM

Jeroen Bakker (jbakker) added a reviewer: Sergey Sharybin (sergey).Dec 11 2019, 9:10 AM

Jeroen Bakker (jbakker) added a project: Render & Cycles.

Jeroen Bakker (jbakker) edited the summary of this revision. (Show Details)Dec 11 2019, 9:39 AM

Jeroen Bakker (jbakker) edited the summary of this revision. (Show Details)Dec 11 2019, 11:01 AM

Wondering if the issue you're linking to was actually caused by kernel being too slow and that this is just masquerading the real issue. But even if so that's a great improvement.

As for the noise: technically, all the random values are seeded to the same value in the beginning, so one would expect that render result matches 1:1. But with parallel samples being rendered maybe we have some concurrency issues? But that's outside of the scope of this change.

I can not really test this change apart from compiling Blender though.

This revision is now accepted and ready to land.Dec 11 2019, 11:17 AM

Just to mention this change does not fix the rendering problem on Windows with Vega, it just improves render times by removing an old hack. If this fix starts failing on other platforms we want to know and might perhaps revert this.

Closed by commit rBf5e37af5a8fd: Cycles/OpenCL: Remove NULL PTR Workaround (authored by Jeroen Bakker (jbakker)). · Explain WhyDec 11 2019, 12:02 PM

This revision was automatically updated to reflect the committed changes.

Jeroen Bakker (jbakker) added a commit: rBf5e37af5a8fd: Cycles/OpenCL: Remove NULL PTR Workaround.

Jeroen Bakker (jbakker) added a task: T71479: Blender 2.81/2.82 - OpenCL - Significan slow render times.Dec 16 2019, 7:47 PM

Nathan Letwory (jesterking) added a commit: rC9d022af85aa9: Cycles/OpenCL: Remove NULL PTR Workaround.Jan 13 2020, 11:29 PM