OpenShadingLanguage
OpenShadingLanguage copied to clipboard
Draft: Remove more unnecessary conditional run layer calls
This PR is a work-in-progress that I've hit a (hopefully) minor stumbling block on.
Overview
@tgrant-nv was looking at some of our shader ptx awhile ago and pointed out that a surprisingly high amount of the total instructions were just conditional layer call checks (due to all the stores and loads surrounding calls). The concern was that this was slowing down OptiX module compilation.
OSL has some basic checks in place to remove unnecessary conditional layer calls, but this patch takes things further. The high level strategy is:
- Mark all our conditional layer calls during llvm generation
- Run a custom llvm optimization pass that associates calls with their basic blocks
- For each call, walk up llvm's dominator tree to find basic blocks guaranteed to have already run
- If a guaranteed basic block already contains the same call as the current one, remove the current one
It doesn't help the compilation times for every shader, but we've seen speedups as high as 5x on some of our most expensive shaders.
Work in progress
The problem I've run into is that the optimization pass introduces quite a bit of overhead to the ptx generation. Most assets take 10-20% longer to generate ptx with this patch. For us, it's probably still a good trade to turn on for GPU (module compilation is more expensive than ptx generation), but I was hoping some more eyes could help improve things. Especially since I've never written a LLVM pass before, maybe I'm doing things particularly inefficiently.
Any feedback or ideas would be appreciated!
Tests
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.
- :x: The email address for the commit (af31c7b9ef73b20c519df61c2eaaf00851493d57) is not linked to the GitHub account, preventing the EasyCLA check. Consult this Help Article and GitHub Help to resolve. (To view the commit's email address, add .patch at the end of this PR page's URL.) For further assistance with EasyCLA, please submit a support request ticket.
This sounds very promising. A 10-20% increase in JIT time isn't so bad, if it increases the shader performance or even reduces the PTX compile time. We'll take a closer look and see what recommendations we can make.
This sounds very promising. A 10-20% increase in JIT time isn't so bad, if it increases the shader performance or even reduces the PTX compile time. We'll take a closer look and see what recommendations we can make.
Thanks! Looking forward to your findings.
Is this ready to be a non-draft? Looks like it might need a rebase as well.
Is this ready to be a non-draft?
It's close. It turned out the speed hit we've been seeing is because removing so many run-layer checks made the remaining ones more palatable for inlining. So LLVM's inlining pass times are growing quite a bit.
Now that it's been merged, I want to test this patch internally on #1680, since that also has inlining ramifications and I want to make sure everything interacts okay.
@chellmuth , as an experiment could you add a no-inline attribute to the layer functions to prevent them from being inlined? Not sure you really want to do that, but could be an interesting data point.