Fix OptiX + clang14 issue with duplicate symbols #1561
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
aconty
approved these changes
Aug 27, 2022
|
static should have the same effect, don't know if it is being drop for a legitimate reason (bad device declaration) or a bug in the front/backend. Anyway I'm glad it helped. |
We had been using clang/llvm 12 and when trying out 14 I started getting test failures with OptiX compliaining about duplicate symbols. They all seemed to be the things we declared as OSL_CONSTANT_DATA. It looks like there's a difference in llvm 12 vs 14 in how the generated PTX marks its symbol visibility. The solution (thanks to a clever suggestion by Alex Conty) was just to enclose those declarations in anonymous namespaces. Works like a charm! Signed-off-by: Larry Gritz <[email protected]>
lgritz
added a commit
to lgritz/OpenShadingLanguage
that referenced
this pull request
Aug 29, 2022
…dation#1561) We had been using clang/llvm 12 and when trying out 14 I started getting test failures with OptiX complaining about duplicate symbols. They all seemed to be the things we declared as OSL_CONSTANT_DATA. It looks like there's a difference in llvm 12 vs 14 in how the generated PTX marks its symbol visibility. The solution (thanks to a clever suggestion by Alex Conty) was just to enclose those declarations in anonymous namespaces. Works like a charm! Signed-off-by: Larry Gritz <[email protected]>
lgritz
added a commit
to lgritz/OpenShadingLanguage
that referenced
this pull request
Aug 29, 2022
…dation#1561) We had been using clang/llvm 12 and when trying out 14 I started getting test failures with OptiX complaining about duplicate symbols. They all seemed to be the things we declared as OSL_CONSTANT_DATA. It looks like there's a difference in llvm 12 vs 14 in how the generated PTX marks its symbol visibility. The solution (thanks to a clever suggestion by Alex Conty) was just to enclose those declarations in anonymous namespaces. Works like a charm! Signed-off-by: Larry Gritz <[email protected]>
chellmuth
added a commit
to chellmuth/OpenShadingLanguage
that referenced
this pull request
Oct 17, 2022
LLVM 14 introduced two ptx regressions to our file-scoped static variables: 1) They gained the .visible directive 2) They stopped being optimized out when unused This lead to duplicate symbol errors when building an OptiX pipeline with multiple shaders. We attempted a fix using anonymous namespaces in AcademySoftwareFoundation#1561, but kept the static keyword, resulting in a partial solution. Removing static generates ptx that matches LLVM 12 for the affected variable definitions. Signed-off-by: Chris Hellmuth <[email protected]>
lgritz
pushed a commit
that referenced
this pull request
Oct 17, 2022
* Address -Winconsistent-missing-override clang warning * Fix ptx definitions of OSL_CONSTANT_DATA arrays LLVM 14 introduced two ptx regressions to our file-scoped static variables: 1) They gained the .visible directive 2) They stopped being optimized out when unused This lead to duplicate symbol errors when building an OptiX pipeline with multiple shaders. We attempted a fix using anonymous namespaces in #1561, but kept the static keyword, resulting in a partial solution. Removing static generates ptx that matches LLVM 12 for the affected variable definitions. Signed-off-by: Chris Hellmuth <[email protected]>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
We had been using clang/llvm 12 and when trying out 14 I started
getting test failures with OptiX compliaining about duplicate
symbols. They all seemed to be the things we declared as
OSL_CONSTANT_DATA. It looks like there's a difference in llvm 12 vs 14
in how the generated PTX marks its symbol visibility.
The solution (thanks to a clever suggestion by Alex Conty) was just to
enclose those declarations in anonymous namespaces. Works like a
charm!
Signed-off-by: Larry Gritz [email protected]