intel-graphics-compiler: move opencl-clang into the package#440534
intel-graphics-compiler: move opencl-clang into the package#440534emilazy merged 3 commits intoNixOS:masterfrom
Conversation
Darwin was broken for a long time and I do not plan to support it in the future.
nixpkgs-ci
bot
added
10.rebuild-linux: 1-10
SuperSandro2000
force-pushed
the
opencl-clang-standalone-from-llvm-15
branch
from
95c6457 to
f9b5e9f
Compare
nixpkgs-ci
bot
added
the
9.needs: reviewer
emilazy
left a comment
emilazy
left a comment
There was a problem hiding this comment.
This is super excessive – you can do an LLVM + Clang build in a dozen lines if you don’t need the split packages, platform support, and Nixpkgs‐related functionality that leads to most of the complexity of llvmPackages_*. See Cling and Swift for examples.
This could be a single inline let that uses the whole monorepo as src builds the necessary components in one short derivation. Or, potentially better if it works, you can simply skip setting CCLANG_BUILD_PREBUILDS in the intel-graphics-compiler build and let it handle building its own LLVM/Clang with CCLANG_BUILD_INTREE_LLVM. That is likely to be far less code and maintenance burden.
There was a problem hiding this comment.
Does this actually require an LLVM 15 LLD? Since it was never using a patched LLD, I’m guessing it would work with the regular llvmPackages.lld. (Unless it’s consuming LLD as a library?)
Similarly, does it actually need a wrapped patched Clang, or just the patched libclang and some wrapped Clang (or even GCC?) to compile its own code with? I’m not sure we can commit to keeping wrapCCWith working for dropped Clangs – we have conditionals in there that can be cleaned up after the drops, although I don’t think any of them would be immediately relevant to the use case here.
There was a problem hiding this comment.
Unlikely to be relevant in a GPU compiler library context.
There was a problem hiding this comment.
Only relevant because of split packages, I think, which are unnecessary here.
There was a problem hiding this comment.
Per the filename, this is only used for LLVM ≥ 16, so this is an unreferenced file.
There was a problem hiding this comment.
I don’t think it makes sense to use overrideAttrs to inject patches when you’re taking ownership of the whole packaging (especially since we should be avoiding it in Nixpkgs anyway). They can just be applied directly in the relevant derivations (probably only a single derivation, given what I said in another comment).
There was a problem hiding this comment.
This would be solved by doing a simpler single‐derivation LLVM build rather than copying the complicated split‐packages stuff.
There was a problem hiding this comment.
This should probably have an identifier to distinguish it from upstream LLVM and include the opencl-clang version. (Is it intended that 15.0.3 ≠ 15.0.7, though?)
There was a problem hiding this comment.
This seems like a NOP override now? (But it probably makes sense to just drop it in favour of not using a custom wrapped Clang here.)
There was a problem hiding this comment.
Since you said opencl-clang is only used for intel-graphics-compiler, maybe it should be inlined into that package, too? I think it’s best to avoid exposing this single‐purpose LLVM build accessibly outside of passthru.tests, and the two derivations seem quite coupled, from what you’ve said.
But as I said in the initial comment, it’s likely you don’t even need a separate package here…
nixpkgs-ci
bot
removed
the
9.needs: reviewer
|
emilazy
left a comment
emilazy
left a comment
There was a problem hiding this comment.
The following patch is net negative lines of code, and successfully compiles up to intel-compute-runtime; feel free to use it with Co-authored-by: Emily <[email protected]>. I have not tested using it in practice due to lack of applicable hardware, but it seems to have all the correct things in its output:
diff --git a/pkgs/by-name/in/intel-graphics-compiler/package.nix b/pkgs/by-name/in/intel-graphics-compiler/package.nix
index 00ff791d94..b61c963a83 100644
--- a/pkgs/by-name/in/intel-graphics-compiler/package.nix
+++ b/pkgs/by-name/in/intel-graphics-compiler/package.nix
@@ -2,82 +2,110 @@
lib,
stdenv,
fetchFromGitHub,
- bash,
cmake,
- runCommandLocal,
+ ninja,
+ pkg-config,
+ git,
bison,
flex,
+ zlib,
intel-compute-runtime,
- llvmPackages_15,
- opencl-clang,
python3,
spirv-tools,
spirv-headers,
- spirv-llvm-translator,
-
- buildWithPatches ? true,
}:
-let
- vc_intrinsics_src = fetchFromGitHub {
- owner = "intel";
- repo = "vc-intrinsics";
- rev = "v0.23.1";
- hash = "sha256-7coQegLcgIKiqnonZmgrKlw6FCB3ltSh6oMMvdopeQc=";
- };
-
- inherit (llvmPackages_15) lld llvm;
- inherit (if buildWithPatches then opencl-clang else llvmPackages_15) clang libclang;
- spirv-llvm-translator' = spirv-llvm-translator.override { inherit llvm; };
-
- # Handholding the braindead build script
- # cmake requires an absolute path
- prebuilds = runCommandLocal "igc-cclang-prebuilds" { } ''
- mkdir $out
- ln -s ${clang}/bin/clang $out/
- ln -s ${opencl-clang}/lib/* $out/
- ln -s ${lib.getLib libclang}/lib/clang/${lib.getVersion clang}/include/opencl-c.h $out/
- ln -s ${lib.getLib libclang}/lib/clang/${lib.getVersion clang}/include/opencl-c-base.h $out/
- '';
-in
stdenv.mkDerivation rec {
pname = "intel-graphics-compiler";
version = "2.16.0";
- src = fetchFromGitHub {
- owner = "intel";
- repo = "intel-graphics-compiler";
- tag = "v${version}";
- hash = "sha256-vtVktc77OT7OANVXnLvEQx+NEQBPrTE5FFynXhpsK7o=";
- };
+ # See the repository for expected versions:
+ # <https://github.com/intel/intel-graphics-compiler/blob/v2.16.0/documentation/build_ubuntu.md#revision-table>
+ srcs = [
+ (fetchFromGitHub {
+ name = "igc";
+ owner = "intel";
+ repo = "intel-graphics-compiler";
+ tag = "v${version}";
+ hash = "sha256-vtVktc77OT7OANVXnLvEQx+NEQBPrTE5FFynXhpsK7o=";
+ })
+ (fetchFromGitHub {
+ name = "llvm-project";
+ owner = "llvm";
+ repo = "llvm-project";
+ tag = "llvmorg-15.0.7";
+ hash = "sha256-wjuZQyXQ/jsmvy6y1aksCcEDXGBjuhpgngF3XQJ/T4s=";
+ })
+ (fetchFromGitHub {
+ name = "vc-intrinsics";
+ owner = "intel";
+ repo = "vc-intrinsics";
+ rev = "v0.23.1";
+ hash = "sha256-7coQegLcgIKiqnonZmgrKlw6FCB3ltSh6oMMvdopeQc=";
+ })
+ (fetchFromGitHub {
+ name = "opencl-clang";
+ owner = "intel";
+ repo = "opencl-clang";
+ tag = "v15.0.3";
+ hash = "sha256-JkYFmnDh7Ot3Br/818aLN33COEG7+xyOf8OhdoJX9Cw==";
+ })
+ (fetchFromGitHub {
+ name = "llvm-spirv";
+ owner = "KhronosGroup";
+ repo = "SPIRV-LLVM-Translator";
+ tag = "v15.0.15";
+ hash = "sha256-kFVDS+qwoG1AXrZ8LytoiLVbZkTGR9sO+Wrq3VGgWNQ=";
+ })
+ ];
+
+ sourceRoot = ".";
+
+ cmakeDir = "../igc";
+
+ postUnpack = ''
+ chmod -R +w .
+ mv opencl-clang llvm-spirv llvm-project/llvm/projects/
+ '';
postPatch = ''
- substituteInPlace IGC/AdaptorOCL/igc-opencl.pc.in \
+ substituteInPlace igc/IGC/AdaptorOCL/igc-opencl.pc.in \
--replace-fail '/@CMAKE_INSTALL_INCLUDEDIR@' "/include" \
--replace-fail '/@CMAKE_INSTALL_LIBDIR@' "/lib"
- chmod +x IGC/Scripts/igc_create_linker_script.sh
- patchShebangs --build IGC/Scripts/igc_create_linker_script.sh
+ chmod +x igc/IGC/Scripts/igc_create_linker_script.sh
+ patchShebangs --build igc/IGC/Scripts/igc_create_linker_script.sh
+
+ # The build system only applies patches when the sources are in a
+ # Git repository.
+ git -C llvm-project init
+ git -C llvm-project \
+ -c user.name=nixbld -c user.email= \
+ commit --allow-empty -m stub
+ substituteInPlace \
+ llvm-project/llvm/projects/opencl-clang/cmake/modules/CMakeFunctions.cmake \
+ --replace-fail \
+ 'COMMAND ''${GIT_EXECUTABLE} am --3way --ignore-whitespace -C0 ' \
+ 'COMMAND patch -p1 --ignore-whitespace -i '
'';
nativeBuildInputs = [
- bash
bison
cmake
flex
+ git
+ ninja
(python3.withPackages (
ps: with ps; [
mako
pyyaml
]
))
+ zlib
];
buildInputs = [
- lld
- llvm
spirv-headers
- spirv-llvm-translator'
spirv-tools
];
@@ -87,11 +115,10 @@
doCheck = false;
cmakeFlags = [
- "-DVC_INTRINSICS_SRC=${vc_intrinsics_src}"
- "-DCCLANG_BUILD_PREBUILDS=ON"
- "-DCCLANG_BUILD_PREBUILDS_DIR=${prebuilds}"
"-DIGC_OPTION__SPIRV_TOOLS_MODE=Prebuilds"
- "-DIGC_OPTION__VC_INTRINSICS_MODE=Source"
+ "-DIGC_OPTION__USE_PREINSTALLED_SPIRV_HEADERS=ON"
+ "-DSPIRV-Headers_INCLUDE_DIR=${spirv-headers}/include"
+ "-DLLVM_EXTERNAL_SPIRV_HEADERS_SOURCE_DIR=${spirv-headers.src}"
"-Wno-dev"
];
@@ -102,7 +129,7 @@
meta = with lib; {
description = "LLVM-based compiler for OpenCL targeting Intel Gen graphics hardware";
homepage = "https://github.com/intel/intel-graphics-compiler";
- changelog = "https://github.com/intel/intel-graphics-compiler/releases/tag/${src.rev}";
+ changelog = "https://github.com/intel/intel-graphics-compiler/releases/tag/v${version}";
license = licenses.mit;
platforms = platforms.linux;
maintainers = with maintainers; [ SuperSandro2000 ];
diff --git a/pkgs/by-name/op/opencl-clang/opencl-headers-dir.patch b/pkgs/by-name/op/opencl-clang/opencl-headers-dir.patch
deleted file mode 100644
index 70343b8ee1..0000000000
--- a/pkgs/by-name/op/opencl-clang/opencl-headers-dir.patch
+++ /dev/null
@@ -1,25 +0,0 @@
-diff --git a/cl_headers/CMakeLists.txt b/cl_headers/CMakeLists.txt
-index 3dd2ea4..aeae6e9 100644
---- a/cl_headers/CMakeLists.txt
-+++ b/cl_headers/CMakeLists.txt
-@@ -11,12 +11,14 @@ add_custom_command(
- )
- endfunction(copy_file)
-
--if(USE_PREBUILT_LLVM)
-- set(OPENCL_HEADERS_DIR
-- "${LLVM_LIBRARY_DIRS}/clang/${LLVM_VERSION_MAJOR}.${LLVM_VERSION_MINOR}.${LLVM_VERSION_PATCH}/include/")
--else(USE_PREBUILT_LLVM)
-- set(OPENCL_HEADERS_DIR "${CLANG_SOURCE_DIR}/lib/Headers")
--endif(USE_PREBUILT_LLVM)
-+if(NOT DEFINED OPENCL_HEADERS_DIR)
-+ if(USE_PREBUILT_LLVM)
-+ set(OPENCL_HEADERS_DIR
-+ "${LLVM_LIBRARY_DIRS}/clang/${LLVM_VERSION_MAJOR}.${LLVM_VERSION_MINOR}.${LLVM_VERSION_PATCH}/include/")
-+ else(USE_PREBUILT_LLVM)
-+ set(OPENCL_HEADERS_DIR "${CLANG_SOURCE_DIR}/lib/Headers")
-+ endif(USE_PREBUILT_LLVM)
-+endif()
- copy_file(${OPENCL_HEADERS_DIR}/opencl-c.h opencl-c.h)
-
- add_custom_target (
diff --git a/pkgs/by-name/op/opencl-clang/package.nix b/pkgs/by-name/op/opencl-clang/package.nix
deleted file mode 100644
index 3b9157221e..0000000000
--- a/pkgs/by-name/op/opencl-clang/package.nix
+++ /dev/null
@@ -1,123 +0,0 @@
-{
- lib,
- stdenv,
- applyPatches,
- fetchFromGitHub,
- cmake,
- git,
- llvmPackages_15,
- spirv-llvm-translator,
- buildWithPatches ? true,
-}:
-
-let
- addPatches =
- component: pkg:
- pkg.overrideAttrs (oldAttrs: {
- postPatch = oldAttrs.postPatch or "" + ''
- for p in ${passthru.patchesOut}/${component}/*; do
- patch -p1 -i "$p"
- done
- '';
- });
-
- llvmPkgs = llvmPackages_15;
- inherit (llvmPkgs) llvm;
- spirv-llvm-translator' = spirv-llvm-translator.override { inherit llvm; };
- libclang = if buildWithPatches then passthru.libclang else llvmPkgs.libclang;
-
- passthru = rec {
- spirv-llvm-translator = spirv-llvm-translator';
- llvm = addPatches "llvm" llvmPkgs.llvm;
- libclang = addPatches "clang" llvmPkgs.libclang;
-
- clang-unwrapped = libclang.out;
- clang = llvmPkgs.clang.override {
- cc = clang-unwrapped;
- };
-
- patchesOut = stdenv.mkDerivation {
- pname = "opencl-clang-patches";
- inherit version src;
- # Clang patches assume the root is the llvm root dir
- # but clang root in nixpkgs is the clang sub-directory
- postPatch = ''
- for filename in patches/clang/*.patch; do
- substituteInPlace "$filename" \
- --replace-fail "a/clang/" "a/" \
- --replace-fail "b/clang/" "b/"
- done
- '';
-
- installPhase = ''
- [ -d patches ] && cp -r patches/ $out || mkdir $out
- mkdir -p $out/clang $out/llvm
- '';
- };
- };
-
- version = "15.0.3";
- src = applyPatches {
- src = fetchFromGitHub {
- owner = "intel";
- repo = "opencl-clang";
- tag = "v${version}";
- hash = "sha256-JkYFmnDh7Ot3Br/818aLN33COEG7+xyOf8OhdoJX9Cw=";
- };
-
- patches = [
- # Build script tries to find Clang OpenCL headers under ${llvm}
- # Work around it by specifying that directory manually.
- ./opencl-headers-dir.patch
- ];
-
- postPatch = ''
- # fix not be able to find clang from PATH
- substituteInPlace cl_headers/CMakeLists.txt \
- --replace-fail " NO_DEFAULT_PATH" ""
- ''
- + lib.optionalString stdenv.hostPlatform.isDarwin ''
- # Uses linker flags that are not supported on Darwin.
- sed -i -e '/SET_LINUX_EXPORTS_FILE/d' CMakeLists.txt
- substituteInPlace CMakeLists.txt \
- --replace-fail '-Wl,--no-undefined' ""
- '';
- };
-in
-
-stdenv.mkDerivation {
- pname = "opencl-clang";
- inherit version src;
-
- nativeBuildInputs = [
- cmake
- git
- llvm.dev
- ];
-
- buildInputs = [
- libclang
- llvm
- spirv-llvm-translator'
- ];
-
- cmakeFlags = [
- "-DPREFERRED_LLVM_VERSION=${lib.getVersion llvm}"
- "-DOPENCL_HEADERS_DIR=${lib.getLib libclang}/lib/clang/${lib.getVersion libclang}/include/"
-
- "-DLLVMSPIRV_INCLUDED_IN_LLVM=OFF"
- "-DSPIRV_TRANSLATOR_DIR=${spirv-llvm-translator'}"
- ];
-
- inherit passthru;
-
- meta = with lib; {
- homepage = "https://github.com/intel/opencl-clang/";
- description = "Clang wrapper library with an OpenCL-oriented API and the ability to compile OpenCL C kernels to SPIR-V modules";
- license = licenses.ncsa;
- maintainers = [ ];
- platforms = platforms.all;
- # error: invalid value 'CL3.0' in '-cl-std=CL3.0'
- broken = stdenv.hostPlatform.isDarwin;
- };
-}
diff --git a/pkgs/top-level/aliases.nix b/pkgs/top-level/aliases.nix
index afd79f028d..9b84949ae0 100644
--- a/pkgs/top-level/aliases.nix
+++ b/pkgs/top-level/aliases.nix
@@ -1794,6 +1794,7 @@
opencv2 = throw "opencv2 has been removed as it is obsolete and was not used by any other package; please migrate to OpenCV 4"; # Added 2024-08-20
opencv3 = throw "opencv3 has been removed as it is obsolete and was not used by any other package; please migrate to OpenCV 4"; # Added 2024-08-20
openafs_1_8 = openafs; # Added 2022-08-22
+ opencl-clang = throw "opencl-clang has been merged into intel-graphics-compiler"; # Added 2025-09-06
opencl-info = throw "opencl-info has been removed, as the upstream is unmaintained; consider using 'clinfo' instead"; # Added 2024-06-12
opencomposite-helper = throw "opencomposite-helper has been removed from nixpkgs as it causes issues with some applications. See https://wiki.nixos.org/wiki/VR#OpenComposite for the recommended setup"; # Added 2024-09-07
openconnect_gnutls = openconnect; # Added 2022-03-29I followed the upstream build instructions for this. Letting the intel-graphics-compiler build system drive the LLVM build, as I suggested in my previous review, results in a build that should be smaller and quicker than the fully‐featured LLVM package set this pull request vendors, and of course a far more maintainable expression than vendoring 2,000 lines of obsolete code.
More importantly though, I missed on first reading that the approach in this PR won’t actually fix the issue with the LLVM drops; it depends on our spirv-llvm-translator package supporting LLVM versions that will be removed, and so would be broken by 0d225f3. Letting the upstream build system handle the bootstrap of LLVM and SPIRV-LLVM-Translator resolves that.
|
Thanks! I will take a look at this next week, not sure if I can do it on Monday or throughout the week though. |
|
I just successfully tested the intel-compute-runtime-legacy1 on a laptop with clinfo and a hash brute forcing tool and it works. I may have access to my device supporting intel-compute-runtime in the next days where I can test there, too. |
emilazy
left a comment
emilazy
left a comment
There was a problem hiding this comment.
LGTM of course, thanks!
I believe that if the compiler works with one runtime it should work with the other, from what I’ve seen about the architecture, or at least I’d expect if it breaks it’ll be the other way around (new compiler violates expectations of the old runtime).
nixpkgs-ci
bot
added
the
12.approvals: 1
|
Anything I can do to help move this along or is it just waiting on testing? It’s the one remaining thing for #440273 to be ready for merge, and I have further LLVM work on top of that that it’d be good to start pushing out for the next |
|
I think also that the none legacy driver should work and if you think this is good to go we can merge it. |
|
Sounds good to me. Can you squash the commits since the first six aren’t relevant now? |
SuperSandro2000
force-pushed
the
opencl-clang-standalone-from-llvm-15
branch
from
4b1c182 to
33e3319
Compare
SuperSandro2000
changed the title
SuperSandro2000
force-pushed
the
opencl-clang-standalone-from-llvm-15
branch
from
33e3319 to
92c3937
Compare
|
I left the two commits which removed darwin support and the option to apply patches and the rest of the refactor is the 3rd commit. |
3 participants
Tested by commenting out llvmPackages_15 and by not triggering a rebuild for intel-compute-runtime for the vendoring commit.
Closes #440272
Things done
passthru.tests.nixpkgs-reviewon this PR. See nixpkgs-review usage../result/bin/.Add a 👍 reaction to pull requests you find important.