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

miopen: init at 2.18.0-5.3.3 #202476

Merged
merged 2 commits into from
Nov 27, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion pkgs/development/compilers/llvm/rocm/llvm.nix
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,7 @@ in stdenv.mkDerivation rec {
"-DCMAKE_BUILD_TYPE=${if debugVersion then "Debug" else "Release"}"
"-DLLVM_INSTALL_UTILS=ON" # Needed by rustc
"-DLLVM_TARGETS_TO_BUILD=AMDGPU;${llvmNativeTarget}"
"-DLLVM_ENABLE_PROJECTS=clang;lld;compiler-rt"
"-DLLVM_ENABLE_PROJECTS=clang;lld;compiler-rt;clang-tools-extra"
]
++ lib.optionals enableManpages [
"-DLLVM_BINUTILS_INCDIR=${libbfd.dev}/include"
Expand Down
215 changes: 215 additions & 0 deletions pkgs/development/libraries/miopen/default.nix
Original file line number Diff line number Diff line change
@@ -0,0 +1,215 @@
{ lib
, stdenv
, fetchFromGitHub
, writeScript
, pkg-config
, cmake
, rocm-cmake
, rocm-runtime
, rocm-device-libs
, rocm-comgr
, rocm-opencl-runtime
, rocblas
, rocmlir
, hip
, clang
, clang-ocl
, llvm
, miopengemm
, composable_kernel
, half
, boost
, sqlite
, bzip2
, texlive ? null
, doxygen ? null
, sphinx ? null
, python3Packages ? null
, zlib ? null
, fetchurl ? null
, buildDocs ? false
, buildTests ? false
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why are the tests optional?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

They are not needed for use as a dependency or for end-user functionality.
They can also be costly to build.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

They are probably important to check whether the packaging works out alright. Not sure what "can also be costly" means vs. being actually costly.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Most tests don't run within the nix sandbox, also.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's also the reason why #200757 exists.
Tests will need to be run in an impure fashion.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There should not be ? null heres regardless if the tests are run or not, if they are currently broken or not.

# LFS isn't working, so we will manually fetch these
# This isn't strictly required, but is recommended
# https://github.com/ROCmSoftwarePlatform/MIOpen/issues/1373
, fetchKDBs ? true
, useOpenCL ? false
}:

assert buildDocs -> texlive != null;
assert buildDocs -> doxygen != null;
assert buildDocs -> sphinx != null;
assert buildDocs -> python3Packages != null;
assert buildTests -> zlib != null;
assert fetchKDBs -> fetchurl != null;

let
latex = lib.optionalAttrs buildDocs (texlive.combine {
inherit (texlive) scheme-small
latexmk
tex-gyre
fncychap
wrapfig
capt-of
framed
needspace
tabulary
varwidth
titlesec;
});

kdbs = lib.optionalAttrs fetchKDBs import ./deps.nix {
inherit fetchurl;
mirror = "https://repo.radeon.com/rocm/miopen-kernel/rel-5.0";
};
in stdenv.mkDerivation (finalAttrs: {
pname = "miopen";
# We have to manually specify the repoVersion for now
# Find the github release or `-- MIOpen_VERSION= X.X.X` in the build log
repoVersion = "2.18.0";
rocmVersion = "5.3.3";
version = "${finalAttrs.repoVersion}-${finalAttrs.rocmVersion}";

outputs = [
"out"
] ++ lib.optionals buildDocs [
"doc"
] ++ lib.optionals buildTests [
"test"
];

src = fetchFromGitHub {
owner = "ROCmSoftwarePlatform";
repo = "MIOpen";
rev = "rocm-${finalAttrs.rocmVersion}";
hash = "sha256-5/JitdGJ0afzK4pGOOywRLsB3/Thc6/71sRkKIxf2Lg=";
};

nativeBuildInputs = [
pkg-config
cmake
rocm-cmake
hip
clang
llvm
];

buildInputs = [
rocm-runtime
rocm-device-libs
rocm-comgr
rocm-opencl-runtime
rocblas
rocmlir
clang-ocl
miopengemm
composable_kernel
half
boost
sqlite
bzip2
] ++ lib.optionals buildDocs [
latex
doxygen
sphinx
Copy link
Member

@mweinelt mweinelt Nov 27, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

use SphinxHook, build the documentation unconditionally and enable the doc and/or man outputs. They can be installed per user-choice through the documentation module.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Many of the other ROCm derivations I've made do the same, why use SphinxHook here?
The others also don't build the documentation unconditionally, so again, why here?

Copy link
Member

@mweinelt mweinelt Nov 27, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Because I first spotted it here. It's a recommendation to simplify things, not a precondition to get things merged.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not having this built on hydra is rough and makes it obvious why parts of the build were made conditional.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Okay makes sense.
It might be a little bit before I commit the new changes since I gotta rebuild everything.

Copy link
Contributor Author

@Madouura Madouura Nov 27, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There are a few other derivations that build documentation so it might be easier to merge this as-is and then I'll submit a rocm-related PR concerning sphinxHook.
The directories are all over the place for ROCm, so I'm not entirely sure one solution will work for all of them.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah right, need to fix the docs review before merge. One moment.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

After some testing, simplifying this with sphinxHook isn't a good option.
The documentation seems to depend on certain variables from cmake.
Here's what I have done: Madouura@f4ae10e

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We also lose the ability to reasonably generate the pdf.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Clarification: The documentation does build, but will silently error in the actual documents.
You'll get missing parts and messed up in-lines.

python3Packages.sphinx_rtd_theme
python3Packages.breathe
python3Packages.myst-parser
] ++ lib.optionals buildTests [
zlib
];

cmakeFlags = [
"-DMIOPEN_USE_MIOPENGEMM=ON"
# Manually define CMAKE_INSTALL_<DIR>
# See: https://github.com/NixOS/nixpkgs/pull/197838
"-DCMAKE_INSTALL_BINDIR=bin"
"-DCMAKE_INSTALL_LIBDIR=lib"
"-DCMAKE_INSTALL_INCLUDEDIR=include"
] ++ lib.optionals (!useOpenCL) [
"-DCMAKE_C_COMPILER=hipcc"
"-DCMAKE_CXX_COMPILER=hipcc"
"-DMIOPEN_BACKEND=HIP"
] ++ lib.optionals useOpenCL [
"-DCMAKE_C_COMPILER=${clang}/bin/clang"
"-DCMAKE_CXX_COMPILER=${clang}/bin/clang++"
"-DMIOPEN_BACKEND=OpenCL"
] ++ lib.optionals buildTests [
"-DBUILD_TESTS=ON"
"-DMIOPEN_TEST_ALL=ON"
"-DMIOPEN_TEST_GFX900=ON"
"-DMIOPEN_TEST_GFX906=ON"
"-DMIOPEN_TEST_GFX908=ON"
"-DMIOPEN_TEST_GFX90A=ON"
"-DMIOPEN_TEST_GFX103X=ON"
];

postPatch = ''
substituteInPlace CMakeLists.txt \
--replace "enable_testing()" "" \
--replace "MIOPEN_HIP_COMPILER MATCHES \".*clang\\\\+\\\\+$\"" "true" \
--replace "/opt/rocm/hip" "${hip}" \
--replace "/opt/rocm/llvm" "${llvm}" \
--replace "3 REQUIRED PATHS /opt/rocm)" "3 REQUIRED PATHS ${hip})" \
--replace "hip REQUIRED PATHS /opt/rocm" "hip REQUIRED PATHS ${hip}" \
--replace "rocblas REQUIRED PATHS /opt/rocm" "rocblas REQUIRED PATHS ${rocblas}" \
--replace "miopengemm PATHS /opt/rocm" "miopengemm PATHS ${miopengemm}" \
--replace "set(MIOPEN_TIDY_ERRORS ALL)" "" # Fix clang-tidy at some point
'' + lib.optionalString (!buildTests) ''
substituteInPlace CMakeLists.txt \
--replace "add_subdirectory(test)" ""
'' + lib.optionalString fetchKDBs ''
cp -a ${kdbs.gfx1030_36} src/kernels/gfx1030_36.kdb
cp -a ${kdbs.gfx900_56} src/kernels/gfx900_56.kdb
cp -a ${kdbs.gfx900_64} src/kernels/gfx900_64.kdb
cp -a ${kdbs.gfx906_60} src/kernels/gfx906_60.kdb
cp -a ${kdbs.gfx906_64} src/kernels/gfx906_64.kdb
cp -a ${kdbs.gfx90878} src/kernels/gfx90878.kdb
cp -a ${kdbs.gfx90a68} src/kernels/gfx90a68.kdb
cp -a ${kdbs.gfx90a6e} src/kernels/gfx90a6e.kdb
'';

# Unfortunately, it seems like we have to call make on these manually
postBuild = lib.optionalString buildDocs ''
export HOME=$(mktemp -d)
make doc
mweinelt marked this conversation as resolved.
Show resolved Hide resolved
'' + lib.optionalString buildTests ''
make -j$NIX_BUILD_CORES check
'';

postInstall = ''
rm $out/bin/install_precompiled_kernels.sh
'' + lib.optionalString buildTests ''
mkdir -p $test/bin
mv bin/test_* $test/bin
patchelf --set-rpath ${lib.makeLibraryPath (finalAttrs.nativeBuildInputs ++ finalAttrs.buildInputs)}:$out/lib $test/bin/*
'';

postFixup = lib.optionalString (buildDocs && !useOpenCL) ''
export docDir=$doc/share/doc/miopen-hip
'' + lib.optionalString (buildDocs && useOpenCL) ''
export docDir=$doc/share/doc/miopen-opencl
'' + lib.optionalString buildDocs ''
mkdir -p $docDir
mv ../doc/html $docDir
mv ../doc/pdf/miopen.pdf $docDir
'';
mweinelt marked this conversation as resolved.
Show resolved Hide resolved

passthru.updateScript = writeScript "update.sh" ''
#!/usr/bin/env nix-shell
#!nix-shell -i bash -p curl jq common-updater-scripts
rocmVersion="$(curl -sL "https://api.github.com/repos/ROCmSoftwarePlatform/MIOpen/releases?per_page=1" | jq '.[0].tag_name | split("-") | .[1]' --raw-output)"
update-source-version miopen "$rocmVersion" --ignore-same-hash --version-key=rocmVersion
'';

meta = with lib; {
description = "Machine intelligence library for ROCm";
homepage = "https://github.com/ROCmSoftwarePlatform/MIOpen";
license = with licenses; [ mit ];
maintainers = teams.rocm.members;
broken = finalAttrs.rocmVersion != hip.version;
# MIOpen will produce a very large output due to KDBs fetched
# Also possibly in the future because of KDB generation
hydraPlatforms = [ ];
};
})
45 changes: 45 additions & 0 deletions pkgs/development/libraries/miopen/deps.nix

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

13 changes: 13 additions & 0 deletions pkgs/top-level/all-packages.nix
Original file line number Diff line number Diff line change
Expand Up @@ -14973,6 +14973,19 @@ with pkgs;

rocthrust = callPackage ../development/libraries/rocthrust { };

miopen = callPackage ../development/libraries/miopen {
inherit (llvmPackages_rocm) clang llvm;
boost = boost.override { enableStatic = true; };
};

miopen-hip = miopen.override {
useOpenCL = false;
};

miopen-opencl = miopen.override {
useOpenCL = true;
};

rtags = callPackage ../development/tools/rtags {
inherit (darwin) apple_sdk;
};
Expand Down