-
Notifications
You must be signed in to change notification settings - Fork 773
{tools}[GCCcore/14.2.0] SPIRV-tools v2025.1 #23174
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
Micket
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.
Some obversations from what i had separately started with for SPIRV-tools and friends
| easyblock = 'CMakeMake' | ||
|
|
||
| name = 'SPIRV-tools' | ||
| version = '2024.3' |
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 this the correct version to use?
https://github.com/KhronosGroup/SPIRV-Tools/releases has 2025.1
though, strangely, they also do mark 2024.4 as the latest.
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.
@Crivella any reason we're not going with latest here?
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 think this was the version i had used originally that i was at some point able to stack with glslang before adding testing, but in the end I preferred going to the monolitic build.
Moved to the latest version ced2504
There is also the idea in #23171 (comment) to be considered to ship SPIRV-tools and glslang as a single build.
| { | ||
| "download_filename": "vulkan-sdk-%s.tar.gz" % local_vulkan_sdk_version, | ||
| "filename": "SPIRV-headers-vulkan-sdk-%s.tar.gz" % local_vulkan_sdk_version, | ||
| "extract_cmd": "mkdir -p %(builddir)s/spirv-tools/external/spirv-headers && tar xzvf %s --strip-components=1 -C $_", | ||
| "source_urls": ["https://github.com/KhronosGroup/SPIRV-Headers/archive/refs/tags"], | ||
| }, |
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.
the version i started with 2025.1 allowed for -DSPIRV-Headers_SOURCE_DIR=$EBROOTSPIRVMINHEADERS for keeping this separate.
I had just created a SPIRV-Headers-1.4.309.0-GCCcore-14.2.0 as a builddep:
easyblock = 'CMakeMake'
name = 'SPIRV-Headers'
version = '1.4.313.0'
homepage = 'https://github.com/KhronosGroup/SPIRV-Headers'
description = 'Khronos-reference front end for GLSL/ESSL, partial front end for HLSL, and a SPIR-V generator. '
toolchain = {'name': 'GCCcore', 'version': '14.2.0'}
source_urls = ['https://github.com/KhronosGroup/SPIRV-Headers/archive/refs/tags/']
sources = ['vulkan-sdk-%(version)s.tar.gz']
checksums = ['f68be549d74afb61600a1e3a7d1da1e6b7437758c8e77d664909f88f302c5ac1']
builddependencies = [
('binutils', '2.42'),
('CMake', '3.31.3'),
]
sanity_check_paths = {
'files': [f'include/spirv/{x}/spirv.h' for x in ['1.0', '1.1', '1.2']],
'dirs': ['include/spirv/'],
}
moduleclass = 'tools| dependencies = [] | ||
|
|
||
| configopts = ' '.join([ | ||
| '-DSPIRV_TOOLS_USE_MIMALLOC=ON', # Use mimalloc for memory management |
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.
This does not seem like it's supported on 2024.3, or even 2025.1. mimalloc support was added only yesterday to the main branch KhronosGroup/SPIRV-Tools@422150b
(and fortunately, it was added with support for custom external paths, so we could just use a normal external builddep)
|
|
||
| local_vulkan_sdk_version = '1.4.309.0' | ||
| local_googletest_version = '1.17.0' | ||
| local_google_effcee_version = '2019.1' |
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.
looks like they stopped bothering tagging, but there is much newer code.
| # Running with googletest also requires effcee -> re2 | ||
| { | ||
| "source_urls": ["https://github.com/google/googletest/releases/download/v%s" % local_googletest_version], | ||
| "filename": "googletest-%s.tar.gz" % local_googletest_version, | ||
| "extract_cmd": "mkdir -p %(builddir)s/spirv-tools/external/googletest && tar xzvf %s --strip-components=1 -C $_", | ||
| }, | ||
| { | ||
| "source_urls": ["https://github.com/google/effcee/archive/refs/tags/"], | ||
| "download_filename": "v%s.tar.gz" % local_google_effcee_version, | ||
| "filename": "effcee-%s.tar.gz" % local_google_effcee_version, | ||
| "extract_cmd": "mkdir -p %(builddir)s/spirv-tools/external/effcee && tar xzvf %s --strip-components=1 -C $_", | ||
| }, | ||
| { | ||
| "source_urls": ["https://github.com/google/re2/archive/refs/tags/"], | ||
| "download_filename": "%s.tar.gz" % local_google_re2_version, | ||
| "filename": "re2-%s.tar.gz" % local_google_re2_version, | ||
| "extract_cmd": "mkdir -p %(builddir)s/spirv-tools/external/re2 && tar xzvf %s --strip-components=1 -C $_", | ||
| }, |
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.
Hm, a quick test I think a very simple patch, like adding
find_package(GTest REQUIRED GLOBAL)
add_library(gmock ALIAS GTest::gmock)
add_library(gmock_main ALIAS GTest::gmock_main)
add_library(gtest ALIAS GTest::gtest)
add_library(gtest_main ALIAS GTest::gtest_main)
would allow for these to be separate easyconfigs as well, we already have googletest, so, a simple re2 and effcee build should maybe just work fine here. Still finishing up those standalone easyconfigs for testing so I can't be sure how well that will work out, but it looks straight forward enough as long as the gmock and other targets are defined.
|
Umh I have reused something i started making a few months ago to try SPIRV with LLVM. |
|
Test report by @Crivella |
|
At some point the downloaded HASH for the same version changed and the build was not working anymore.
In general also for the other dependencies, as this is a suite of tools, i do not expect problems down the line in using a specific version of statically linked dependencies |
|
@boegelbot please test @ jsc-zen3 |
|
@Crivella: Request for testing this PR well received on jsczen3l1.int.jsc-zen3.fz-juelich.de PR test command '
Test results coming soon (I hope)... Details- notification for comment with ID 3008731392 processed Message to humans: this is just bookkeeping information for me, |
|
Test report by @boegelbot |
|
@Micket Considering this are not libraries that we are gonna reuse after, i have the feeling it would be a huge increase of effort on our part to customize their build to make the various packages able to be separated. Especially considering i do not see an effort upstream in making major version of the releases compatible between each other, but only tying them together by a list of commit hashes |
|
I agree that spending work on using our EasyConfigs just for the tests might be too much effort, especially considering that it doesn't look like they're particularly interested in using releases. As far as I can see, all of these dependencies are only used for testing. As long as these binaries don't end up in our install folder, I think it is fine to just use these internal submodules. What might be important is making sure that the installation works offline, i.e. that we're able to fetch all sources beforehand. |
easybuild/easyconfigs/s/SPIRV-tools/SPIRV-tools-2024.3-GCCcore-14.2.0.eb
Outdated
Show resolved
Hide resolved
|
Test report by @Thyre |
- Added build of `spirv-fuzz` - Replace double quotes with single quotes
|
@boegelbot please test @ jsc-zen3 |
|
Test report by @Crivella |
|
@Crivella: Request for testing this PR well received on jsczen3l1.int.jsc-zen3.fz-juelich.de PR test command '
Test results coming soon (I hope)... Details- notification for comment with ID 3244381937 processed Message to humans: this is just bookkeeping information for me, |
|
Also tried to use |
|
Weird to see this error, but since the conditions stay the same and only the order is switched, I would consider this okay. |
|
Test report by @boegelbot |
SPIRV-tools 2024.3|
@boegelbot please test @ jsc-zen3 |
|
@Crivella: Request for testing this PR well received on jsczen3l1.int.jsc-zen3.fz-juelich.de PR test command '
Test results coming soon (I hope)... Details- notification for comment with ID 3248781117 processed Message to humans: this is just bookkeeping information for me, |
|
Test report by @Crivella |
|
Test report by @boegelbot |
|
Keeping this open for now as here we are also running the NOTES: the additional extras packages here (w.r.t. #23171) are for
|
|
Closing this in favor of The PR text/discussion will still be available for future reference in case we decide to also have a version of |
Added EC and patches for SPIRV-tools
Requires: