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

Clarify SPIRV Translator dependency and remove custom SPIRV_TRANSLATOR_DIR #48

Open
dvrogozh opened this issue Mar 12, 2019 · 2 comments

Comments

@dvrogozh
Copy link
Contributor

We touched these 2 topics in #45 (comment).

  1. SPIRV Translator is embedded into LLVM or not?

The default path of common_clang build assumes that https://github.com/KhronosGroup/SPIRV-LLVM-Translator/ will be built and packaged within LLVM build which could result either in separate static library named libLLVMSPIRVLib.a or in a unified LLVM shared library which will include translator library inside.

However, it seems that distributions may incline to build SPIRV Translator as a separate package rather than within LLVM. For example: https://repology.org/project/spirv-llvm-translator/versions - that's what Debian and Ubuntu does.

I would like to use this Issue to collect community feedback on this. @tjaalton, please, comment.

  1. How to detect SPIRV Translator on the system?

As of now common_clang already has a path to assure that it works with SPIRV translator as a separate package on the system. If I understand correctly, for that we need to specify 2 options:
-DLLVMSPIRV_INCLUDED_IN_LLVM=OFF -DSPIRV_TRANSLATOR_DIR=/path/to/installed/spirv/translator

While 1st option seems reasonable (though we may wish to consider changing the default after feedback on the 1st question), then 2nd option seems excessive. SPIRV translator provides lib/pkgconfig/LLVMSPIRVLib.pc, thus we can use standard mechanism to detect it on the system rather than introduce custom SPIRV_TRANSLATOR_DIR. Can this change be introduced?

@AlexeySachkov
Copy link
Contributor

SPIRV Translator is embedded into LLVM or not?

It depends on how it was built. I agree that Linux distributives most likely will package it separately. I think that by default we need to assume that SPIRV-LLVM-Translator is not bundled together with LLVM

How to detect SPIRV Translator on the system?

I think we need to state that SPIRV-LLVM-Translator is the same pre-requisite for opencl-clang building as LLVM & Clang and it should be installed on the system. In this case SPIRV_TRANSLATOR_DIR is not needed at all.

However, I would like to leave this option in place, because you may want to build opencl-clang with custom SPIRV-LLVM-Translator or in an environment where it is not installed to the default location on a system (for example there is a similar option for llvm: LLVM_DIR). This option should be optional instead of mandatory.

@dvrogozh
Copy link
Contributor Author

Yep, that's what I would do. We are on the same page here. I would advice though to avoid overcomplicating build system. It might be reasonable to have SPIRV_TRANSLATOR_DIR, maybe on Windows that would be the right way to do (as an example), but I would double check.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants