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.

Sign up for GitHub

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

Jump to bottom

SPIRVError.cpp misses #include "LLVMSPIRVLib.h" #2376

Closed
anbe42 opened this issue Feb 21, 2024 · 2 comments · Fixed by #2377
Closed

SPIRVError.cpp misses #include "LLVMSPIRVLib.h" #2376

anbe42 opened this issue Feb 21, 2024 · 2 comments · Fixed by #2377

Comments

@anbe42
Copy link

anbe42 commented Feb 21, 2024

The prototype for getErrorMessage() is declared in LLVMSPIRVLib.h, the definition is in SPIRVError.cpp.
If I build with -fvisibility=hidden and some visibility "default" modifications (as described in #1963), this does not reach the compilation of SPIRVError.cpp since that does not include the header with the prototype.

Noticed in branch llvm_release_180, so it's probably in main as well. Not sure if older branches are affected.
svenvh added a commit to svenvh/SPIRV-LLVM-Translator that referenced this issue Feb 22, 2024
@svenvh
Include LLVMSPIRVLib.h in SPIRVError.cpp
89305f7 
The prototype for `getErrorMessage()` is declared in `LLVMSPIRVLib.h`,
but `SPIRVError.cpp` defining that function did not include
`LLVMSPIRVLib.h`.  This can be problematic for builds that use
`-fvisibility=hidden`.

Fixes KhronosGroup#2376
@svenvh svenvh mentioned this issue Feb 22, 2024
Merged
@svenvh
Copy link
Member

svenvh commented Feb 22, 2024

Thanks for reporting! Does #2377 fix the issue for you?

svenvh added a commit to svenvh/SPIRV-LLVM-Translator that referenced this issue Feb 22, 2024
@svenvh
Include LLVMSPIRVLib.h in SPIRVError.cpp
7b795f3 
The prototype for `getErrorMessage()` is declared in `LLVMSPIRVLib.h`,
but `SPIRVError.cpp` defining that function did not include
`LLVMSPIRVLib.h`.  This can be problematic for builds that use
`-fvisibility=hidden`.

Fixes KhronosGroup#2376
@anbe42
Copy link
Author

anbe42 commented Feb 22, 2024

Thanks for reporting! Does #2377 fix the issue for you?

Yes.

svenvh added a commit that referenced this issue Feb 23, 2024
@svenvh
Include LLVMSPIRVLib.h in SPIRVError.cpp (#2377)
81f78d2 
The prototype for `getErrorMessage()` is declared in `LLVMSPIRVLib.h`,
but `SPIRVError.cpp` defining that function did not include
`LLVMSPIRVLib.h`.  This can be problematic for builds that use
`-fvisibility=hidden`.

Fixes #2376
svenvh added a commit to svenvh/SPIRV-LLVM-Translator that referenced this issue Feb 23, 2024
@svenvh
[Backport to 18] Include LLVMSPIRVLib.h in SPIRVError.cpp (Khrono…
cc682b4 
…sGroup#2377)

The prototype for `getErrorMessage()` is declared in `LLVMSPIRVLib.h`,
but `SPIRVError.cpp` defining that function did not include
`LLVMSPIRVLib.h`.  This can be problematic for builds that use
`-fvisibility=hidden`.

Fixes KhronosGroup#2376

(cherry picked from commit 81f78d2)
@svenvh svenvh mentioned this issue Feb 23, 2024
Merged
MrSidims pushed a commit that referenced this issue Feb 26, 2024
@svenvh
[Backport to 18] Include LLVMSPIRVLib.h in SPIRVError.cpp (#2377) (…
e6c001c 
…#2380)

The prototype for `getErrorMessage()` is declared in `LLVMSPIRVLib.h`,
but `SPIRVError.cpp` defining that function did not include
`LLVMSPIRVLib.h`.  This can be problematic for builds that use
`-fvisibility=hidden`.

Fixes #2376

(cherry picked from commit 81f78d2)
jsji pushed a commit to intel/llvm that referenced this issue Feb 29, 2024
@svenvh @sys-ce-bb
Include LLVMSPIRVLib.h in SPIRVError.cpp (#2377)
e59ed66 
The prototype for `getErrorMessage()` is declared in `LLVMSPIRVLib.h`,
but `SPIRVError.cpp` defining that function did not include
`LLVMSPIRVLib.h`.  This can be problematic for builds that use
`-fvisibility=hidden`.

Fixes KhronosGroup/SPIRV-LLVM-Translator#2376

Original commit:
KhronosGroup/SPIRV-LLVM-Translator@81f78d24db36622
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

Include LLVMSPIRVLib.h in SPIRVError.cpp svenvh/SPIRV-LLVM-Translator
2 participants
@anbe42 @svenvh