-
Notifications
You must be signed in to change notification settings - Fork 85
Build llvm spirv translator from source for opencl-clang CI test #155
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
Conversation
| - export TARBALL=SPIRV-LLVM-Translator-${TAG}-linux-${BUILD_TYPE}.zip | ||
| - wget https://github.com/KhronosGroup/SPIRV-LLVM-Translator/releases/download/${TAG}/${TARBALL} -O /tmp/${TARBALL} | ||
| - unzip /tmp/${TARBALL} -d spirv-llvm-translator | ||
| - git clone https://github.com/KhronosGroup/SPIRV-LLVM-Translator.git -b llvm_release_80 spirv-llvm-translator |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm not sure that this would be enough
Basically, we are not triggering build of the translator here.
If I understand correctly, we have to git clone the whole llvm, put both the translator and opencl-clang in there, and only then perform in-tree build
Or, we need to build the translator first as separate cmake invocation and then supply it into opencl-clang build
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thank you. I've updated it.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Indeed, you don't have to build llvm. It will take a lot of time, probably travis job will time out.
I think we should build translator as a separate step in cmake. This build should use llvm binaries from deb packages as it is done now.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You're right. Updated.
|
@AlexeySachkov , @AlexeySotkin , could you help review when you're available? Thank you. |
| - unzip /tmp/${TARBALL} -d spirv-llvm-translator | ||
| - git clone https://github.com/KhronosGroup/SPIRV-LLVM-Translator.git -b llvm_release_80 spirv-llvm-translator | ||
| - cd spirv-llvm-translator | ||
| - for patch in `ls ../patches/spirv/*.patch`; do echo $patch; git apply $patch; done |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should we fail the build if we couldn't apply a patch? Probably set -e can help with it,
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Maybe. The build can pass without these patches.
Is it possible to merge this patches to spirv translator, instead of keeping it here and applying it?
I'll merge this pull request firstly. If needed, I'll add "set -e" to stop build if patch cannot be applied.
Thanks.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is it possible to merge this patches to spirv translator, instead of keeping it here and applying it?
It depends on patches from patches/clang directory. Patches for spirv translator will not work without patches to clang.
AlexeySotkin
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM, Thanks!
The source code should be used by travis CI tests, since it has latest changes for spirv translator.