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

Remove dependency on OptiX SDK for building core OSL with OptiX support #1627

Merged

Conversation

pmoursnv
Copy link
Contributor

@pmoursnv pmoursnv commented Dec 8, 2022

OSL currently includes the OptiX header in several places, but without actually making any use of its contents. OptiX support is implemented soly using LLVM and its NVPTX backend, without any types or functions used from the OptiX SDK, making these includes superfluous. The only part of the codebase that does need them are the tests, so this patch makes finding the SDK dependent on whether tests are enabled or not.
Avoiding including the OptiX headers in the core makes it possible still to build OSL with OptiX support (USE_OPTIX=ON;OSL_BUILD_TESTS=OFF;), but without needing the OptiX SDK in the process, simplifying the build process on build systems where having the OptiX SDK installed is a challenge.

Additionally this patch also makes it possible to use OSL in OptiX without pre-built LLVM bitcode containing OSL intrinsics (USE_LLVM_BITCODE=OFF;). Building that bitcode module requires the CUDA toolkit, which too can be a challenge to have available on build systems, but it's not technically needed if the renderer implementing OSL does provide an implementation for all the OSL intrinsics declared in builtindecl.h itself (and it already had to do so for some of them right now anyway). In that case only have to initialize the now empty LLVM module OSL creates with a proper target and data layout (which would normally be inherited from the pre-built bitcode), otherwise compiling shader groups will later fail, as the NVPTX backend is looked up based on the target triple set on the module (see implementation of LLVM_Util::ptx_compile_group()).

CMake previously errored when attempting to build with OptiX support and without the LLVM bitcode, but due to the previous change it only does that when tests are enabled too now, making the following a valid combination of build options when no CUDA/OptiX SDK is available, but OptiX support is still desired: USE_OPTIX=ON;USE_LLVM_BITCODE=OFF;OSL_BUILD_TESTS=OFF;. This is e.g. the case with Blender (more discussion regarding this on https://developer.blender.org/D15902).

Note: The change to externalpackages.cmake looks larger than it is due whitespace changes caused by the new if clause.

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). [Existing tests build and run]
  • My code follows the prevailing code style of this project.

Signed-off-by: Patrick Mours <pmours@nvidia.com>
@linux-foundation-easycla
Copy link

linux-foundation-easycla bot commented Dec 8, 2022

CLA Signed

The committers listed above are authorized under a signed CLA.

  • login: pmoursnv / name: Patrick Mours (a05363c)

@brechtvl
Copy link
Contributor

The changes look good to me.

The failing VFX2022 clang-format check can be fixed by reordering the includes in src/liboslexec/oslexec_pvt.h.

The other 3 failures look like preexisting issues not caused by this pull request.

@lgritz
Copy link
Collaborator

lgritz commented Dec 15, 2022

I know why the mac tests are failing, it's on me to fix. (I've been on vacation.)

I'll look into that other failure, too, which happens when setting up dependencies, so I don't think it's at all related to this PR, just ignore it.

Please fix the formatting for the clang-format failure, and then I'll merge this. Thanks for the fix!

Signed-off-by: Patrick Mours <pmours@nvidia.com>
@lgritz lgritz merged commit fae61e6 into AcademySoftwareFoundation:main Dec 16, 2022
18 of 21 checks passed
lgritz pushed a commit to lgritz/OpenShadingLanguage that referenced this pull request Dec 20, 2022
lgritz pushed a commit to lgritz/OpenShadingLanguage that referenced this pull request Jan 2, 2023
lgritz pushed a commit to lgritz/OpenShadingLanguage that referenced this pull request Jan 3, 2023
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.

None yet

3 participants