SPEC: https://download.copr.fedorainfracloud.org/results/frantisekz/intel-media-driver-free/fedora-rawhide-x86_64/03086654-intel-compute-runtime/intel-compute-runtime.spec SRPM: https://download.copr.fedorainfracloud.org/results/frantisekz/intel-media-driver-free/fedora-rawhide-x86_64/03086654-intel-compute-runtime/intel-compute-runtime-21.50.21939-1.fc36.src.rpm
I'm interested in OpenCL support for newer Intel GPUs and I was going to package this, but it's great you beat me to it. I'll help with reviews.
Initial review: > BuildRequires: g++ Shouldn't this be: gcc-g++ ? > BuildRequires: intel-gmmlib > BuildRequires: intel-gmmlib-devel > BuildRequires: libva-devel > BuildRequires: intel-igc > BuildRequires: intel-igc-devel I'd expect the -devel packages to have Requires: on their non-devel parts, so are the non-devel ones (intel-gmmlib and intel-igc) strictly necessary? > Version: 21.50.21939 ... > -DNEO_OCL_VERSION_MAJOR=21 \ > -DNEO_OCL_VERSION_MINOR=50 \ > -DNEO_VERSION_BUILD=21939 \ This begs for automation, otherwise it's prone to missed updates in both places. Either construct Version: from these three components or extract the right component from Version when passing to cmake. > -DCMAKE_BUILD_TYPE=Release \ > -DBUILD_SHARED_LIBS:BOOL=OFF \ > -DCMAKE_BUILD_TYPE=Release \ You're passing CMAKE_BUOLD_TYPE twice. > %if 0%{?__isa_bits} == 64 > -DCMAKE_INSTALL_LIBDIR=lib64 \ > %else > -DCMAKE_INSTALL_LIBDIR=lib \ > %endif %cmake macro contains this: -DLIB_INSTALL_DIR:PATH=/usr/lib64 \ ... %if "lib64" == "lib64" -DLIB_SUFFIX=64 \ %endif so the above should not be necessary and if it is, it's a bug in CMakeLists. > -DSKIP_UNIT_TESTS=1 \ How hard would it be to build and run the tests? > %{_libdir}/libocloc.so Unversioned .so in %{_libdir} is questionable[1]. Please explain why it's OK. [1] https://docs.fedoraproject.org/en-US/packaging-guidelines/#_devel_packages > %global optflags %{optflags} -Wno-error=odr ... > -Wno-dev \ Why are you passing one flag in optflags and another in %cmake? This seems inconsistent.
SPEC: https://download.copr.fedorainfracloud.org/results/frantisekz/intel-media-driver-free/fedora-rawhide-x86_64/03191612-intel-compute-runtime/intel-compute-runtime.spec SRPM: https://download.copr.fedorainfracloud.org/results/frantisekz/intel-media-driver-free/fedora-rawhide-x86_64/03191612-intel-compute-runtime/intel-compute-runtime-22.02.22151-1.fc36.src.rpm
(In reply to Dominik 'Rathann' Mierzejewski from comment #2) > Initial review: > > > BuildRequires: g++ > > Shouldn't this be: gcc-g++ ? Fixed > > > BuildRequires: intel-gmmlib > > BuildRequires: intel-gmmlib-devel > > BuildRequires: libva-devel > > BuildRequires: intel-igc > > BuildRequires: intel-igc-devel > > I'd expect the -devel packages to have Requires: on their non-devel parts, > so are the non-devel ones (intel-gmmlib and intel-igc) strictly necessary? > Fixed > > Version: 21.50.21939 > ... > > -DNEO_OCL_VERSION_MAJOR=21 \ > > -DNEO_OCL_VERSION_MINOR=50 \ > > -DNEO_VERSION_BUILD=21939 \ > > This begs for automation, otherwise it's prone to missed updates in both > places. > Either construct Version: from these three components or extract the right > component from Version when passing to cmake. Fixed > > > -DCMAKE_BUILD_TYPE=Release \ > > -DBUILD_SHARED_LIBS:BOOL=OFF \ > > -DCMAKE_BUILD_TYPE=Release \ > > You're passing CMAKE_BUOLD_TYPE twice. > Fixed > > %if 0%{?__isa_bits} == 64 > > -DCMAKE_INSTALL_LIBDIR=lib64 \ > > %else > > -DCMAKE_INSTALL_LIBDIR=lib \ > > %endif > > %cmake macro contains this: > -DLIB_INSTALL_DIR:PATH=/usr/lib64 \ > ... > %if "lib64" == "lib64" > -DLIB_SUFFIX=64 \ > %endif > so the above should not be necessary and if it is, it's a bug in CMakeLists. > Fixed > > -DSKIP_UNIT_TESTS=1 \ > > How hard would it be to build and run the tests? > Tests are currently failing on multiple places on copy commands, which makes it seem like they're not maintained like they should be (or it may be caused by DNEO_DISABLE_BUILTINS_COMPILATION), I'll report upstream. > > %{_libdir}/libocloc.so > > Unversioned .so in %{_libdir} is questionable[1]. Please explain why it's OK. > > [1] > https://docs.fedoraproject.org/en-US/packaging-guidelines/#_devel_packages > Upstream doesn't version .so files by default. I'll ask about it, shall I go on with plain mv & ln ? > > %global optflags %{optflags} -Wno-error=odr > ... > > -Wno-dev \ > > Why are you passing one flag in optflags and another in %cmake? This seems > inconsistent. Fixed. Also, bumped the package to the latest tag, fixed gcc12 build.
It looks like it needs a runtime dependency on intel-igc. Without intel-igc installed, it doesn't do anything under clpeak: $ strace -f clpeak ... [pid 17433] openat(AT_FDCWD, "/lib64/libigdfcl.so.1", O_RDONLY|O_CLOEXEC) = -1 ENOENT (No such file or directory) [pid 17433] openat(AT_FDCWD, "/usr/lib64/libigdfcl.so.1", O_RDONLY|O_CLOEXEC) = -1 ENOENT (No such file or directory) [pid 17433] munmap(0x7fabf564d000, 53975) = 0 [pid 17433] openat(AT_FDCWD, "/etc/ld.so.cache", O_RDONLY|O_CLOEXEC) = 3 [pid 17433] newfstatat(3, "", {st_mode=S_IFREG|0644, st_size=53975, ...}, AT_EMPTY_PATH) = 0 [pid 17433] mmap(NULL, 53975, PROT_READ, MAP_PRIVATE, 3, 0) = 0x7fabf564d000 [pid 17433] close(3) = 0 [pid 17433] openat(AT_FDCWD, "/lib64/libigc.so.1", O_RDONLY|O_CLOEXEC) = -1 ENOENT (No such file or directory) [pid 17433] openat(AT_FDCWD, "/usr/lib64/libigc.so.1", O_RDONLY|O_CLOEXEC) = -1 ENOENT (No such file or directory) However, once added, it segfaults. clinfo segfaults as well. Now, I'm doing this on F35 with your RPMs from COPR installed, so it's possible there's some library mismatch.
(In reply to Dominik 'Rathann' Mierzejewski from comment #5) > It looks like it needs a runtime dependency on intel-igc. I'll add that. > > However, once added, it segfaults. clinfo segfaults as well. > Yeah, this is kinda expected right now. It's due to https://github.com/intel/intel-graphics-compiler/issues/204 (or, by me working around that by -DNEO_DISABLE_BUILTINS_COMPILATION=ON, at least if I understood it right). Sorry, I somehow must've assumed you've seen this ticket. According to https://github.com/intel/intel-graphics-compiler/issues/225 , it might not take too long before newer llvm get properly supported, but before that, the compute-runtime package will be useful only for vaapi driver compilation. The other possibility to get this working is to get one commit reverted from llvm, or try https://github.com/intel/intel-graphics-compiler/issues/204#issuecomment-1008240911 , but I'd rather try waiting, hopefully it'll get resolved sooner than later upstream.
(In reply to František Zatloukal from comment #4) > (In reply to Dominik 'Rathann' Mierzejewski from comment #2) > > Initial review: > > > > > BuildRequires: g++ > > > > Shouldn't this be: gcc-g++ ? > > Fixed This should be gcc-c++.
SPEC: https://download.copr.fedorainfracloud.org/results/frantisekz/intel-media-driver-free/fedora-rawhide-x86_64/03196768-intel-compute-runtime/intel-compute-runtime.spec SRPM: https://download.copr.fedorainfracloud.org/results/frantisekz/intel-media-driver-free/fedora-rawhide-x86_64/03196768-intel-compute-runtime/intel-compute-runtime-22.02.22151-1.fc36.src.rpm
(In reply to Neal Gompa from comment #7) > This should be gcc-c++. Addressed, intel-igc dependency added.
You forgot to add the changelog. :)
There's lots of bundled stuff in third_party/. In particular: third_party/opencl_headers third_party/opengl_headers can be removed and system ones used instead. Add Requires: libglvnd-devel Requires: ocl-icd-devel and patch appropriately. third_party/gtest It'd be nice to use system gtest once the tests are fixed to work, but not critical. # https://github.com/DLTcollab/sse2neon is bundled in third_party/sse2neon Provides: bundled(sse2neon) third_party/uapi Seems to bundle three different versions of kernel drm headers. Not sure if Fedora kernel-headers can be used instead. Please check. DirectX stuff can be ignored, I guess it's not used for build. I'm not sure what aub_stream headers are or where they come from. third_party/source_level_debugger contains one header under BSD license. Not sure where that came from, either. Included spec file (scripts/packaging/opencl/rhel_8.4/SPECS/opencl.spec) puts the built files into two subpackages: intel-ocloc and intel-opencl. Perhaps it makes sense to do the same in Fedora.
The uapi drm headers seem to be MIT licensed as well, but it looks like a variant that's not detected by licensecheck. Licensing seems to be fine, then.
(In reply to Dominik 'Rathann' Mierzejewski from comment #11) > There's lots of bundled stuff in third_party/. In particular: > > third_party/opencl_headers > third_party/opengl_headers > can be removed and system ones used instead. Add Unfortunately, the bundled versions are prehistoric, I've tried, ended up with reporting it upstream: https://github.com/intel/compute-runtime/issues/496 I guess the way forward here is to mark those two as bundlet too for now? > third_party/uapi Will take a look! > Perhaps it makes sense to do the same in Fedora. Mhm, seems like a good idea. I think we can keep intel-compute-runtime as a metapackage which would then pull in both intel-ocloc and intel-opencl. This also makes sense in case we decide sometime in the future that we want to have the OpenCL driver in default package set for some Fedora images.
SPEC: https://download.copr.fedorainfracloud.org/results/frantisekz/intel-media-driver-free/fedora-rawhide-x86_64/03259722-intel-compute-runtime/intel-compute-runtime.spec SRPM: https://download.copr.fedorainfracloud.org/results/frantisekz/intel-media-driver-free/fedora-rawhide-x86_64/03259722-intel-compute-runtime/intel-compute-runtime-22.03.22192-1.fc36.src.rpm
Hopefully, I didn't forget anything, it's currently not possible to use the upstream drm.h, I've added commented out code to change that if/when that becomes a case in the future (by copying all the headers into builddir, it seemed easier than patching up all the files, there're lots of them). Package split into intel-ocloc and intel-opencl.
It'd be nice to have more useful Summary: and description of the subpackages. ocloc has this in help: ocloc is a tool for managing Intel Compute GPU device binary format. It can be used for generation (as part of 'compile' command) as well as manipulation (decoding/modifying - as part of 'disasm'/'asm' commands) of such binary files. Intel Compute GPU device binary is a format used by Intel Compute GPU runtime (aka NEO). Intel Compute GPU runtime will return this binary format when queried using clGetProgramInfo(..., CL_PROGRAM_BINARIES, ...). It will also honor this format as input to clCreateProgramWithBinary function call. ocloc does not require Intel GPU device to be present in the system nor does it depend on Intel Compute GPU runtime driver to be installed. It does however rely on the same set of compilers (IGC, common_clang) as the runtime driver. For the -opencl subpackage, maybe take a look at the unmaintained beignet package description, e.g. https://src.fedoraproject.org/rpms/beignet/blob/f29/f/beignet.spec . I think this package is good to go and you can improve the above when importing. Please open a bug to track the crashes. We do want the OpenCL driver functional as soon as possible.
And once the crashes are fixed, please backport the package to F35.
Thanks for the review! I've updated descriptions, should be better, probably not perfect :) > Please open a bug to track the crashes. We do want the OpenCL driver functional as soon as possible. Yeah, those would be https://github.com/intel/intel-graphics-compiler/issues/204 and https://github.com/intel/intel-graphics-compiler/issues/224 , or did you mean buzgilla tracker? Also, I've added rm -v for sse2neon (and removed bundled provide for it) and ExclusiveArch: x86_64 i686 as intel-gmmlib is built only for those and intel-igc now too. The support for other arches seems non-existent/incomplete throughout the entire stack. I've requested repo for the package: https://pagure.io/releng/fedora-scm-requests/issue/41679 > And once the crashes are fixed, please backport the package to F35. I wasn't planning to do that originally, but I'll take a look/do my best. I'll have to find/use some older combination of intel-igc and this package as intel-gmmlib in f35 is too old and new release breaks api/abi. Also, I'd be glad if you'd want to co-maintain this (and/or any other part of the stack)? Thanks again!
(In reply to František Zatloukal from comment #18) > Thanks for the review! > > I've updated descriptions, should be better, probably not perfect :) > > > Please open a bug to track the crashes. We do want the OpenCL driver functional as soon as possible. > > Yeah, those would be > https://github.com/intel/intel-graphics-compiler/issues/204 and > https://github.com/intel/intel-graphics-compiler/issues/224 , or did you > mean buzgilla tracker? I'd open a bugzilla tracker once the repo is created. When the package is out, people will find it, sooner or later, and will report crashes in bugzilla or at least look there first. Though hopefully the issue may get fixed before F36 is released. > Also, I've added rm -v for sse2neon (and removed bundled provide for it) and > ExclusiveArch: x86_64 i686 as intel-gmmlib is built only for those and > intel-igc now too. The support for other arches seems > non-existent/incomplete throughout the entire stack. Good point, thanks. > I've requested repo for the package: > https://pagure.io/releng/fedora-scm-requests/issue/41679 > > > And once the crashes are fixed, please backport the package to F35. > > I wasn't planning to do that originally, but I'll take a look/do my best. > I'll have to find/use some older combination of intel-igc and this package > as intel-gmmlib in f35 is too old and new release breaks api/abi. Alright. Let's keep this F36+ only, then. > Also, I'd be glad if you'd want to co-maintain this (and/or any other part > of the stack)? Sure. Please add me as co-maintainer to this and to intel-media-driver-free. I'll be an active user for the latter and in position to test the former. > Thanks again! You're welcome. Thanks for your contribution!
(fedscm-admin): The Pagure repository was created at https://src.fedoraproject.org/rpms/intel-compute-runtime
Built in rawhide: intel-compute-runtime-22.04.22286-1.fc36 > Sure. Please add me as co-maintainer to this and to intel-media-driver-free. Added > I'd open a bugzilla tracker once the repo is created. Good point, will do in the morning :)