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

Removing race condition when compiling for OptiX 7 #1411

Merged
merged 1 commit into from
Sep 27, 2021

Conversation

cmstein
Copy link
Collaborator

@cmstein cmstein commented Sep 23, 2021

Description

This removes a race condition when compiling PTX device code in a multithreaded environment.

The problems stems from OptiX6 requiring the rend_lib.cu device bitcode to be linked to each compiled shadergroup whereas we only need to link the rend_lib.cu device bitcode to one shadergroup in OptiX7. When I added OptiX7 support, I put a static/global in OSL to ensure that we don't get multiple definitions from rend_lib.cu in the device code for OptiX7. But that's a poor solution for a couple of reasons:

  • it's not thread-safe
  • it doesn't support recompiling shaders in an interactive environment.

This removes the requirement to supply the rend_lib.cu device code when using OptiX7. But that also means the renderer now needs to manually link it in. Both testshade and testrender have been updated to reflect this.

Tests

testsuite runs the same with this fix

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.

requires renderers to manually link in the compiled rend_lib.cu
PTX code when using OptiX 7).

Signed-off-by: Clifford Stein <[email protected]>
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.

LGTM, and it passed testsuite for me by hand as well.

@lgritz lgritz merged commit 8f9206a into AcademySoftwareFoundation:master Sep 27, 2021
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