lamikr / rocm_sdk_builder

Other
113 stars 8 forks source link

Arch dependencies #27

Closed daniandtheweb closed 4 weeks ago

daniandtheweb commented 1 month ago

Initial Arch dependencies implementation.

daniandtheweb commented 1 month ago

For now I've added all the equivalent packages I've been able to find.

I marked this pr as draft only for now as I want to test the build and I also still need to add the docs folders for Arch. If anyone else is using Arch some feedback would be great.

daniandtheweb commented 1 month ago

@lamikr Is there any other file am I supposed to add Arch to?

daniandtheweb commented 1 month ago

I'm not sure but the patch 0001-add-mageia-9-support-to-install.sh.patch maybe should also be extended for Arch as it's not explicitely supported. I'll try building without it for now.

aidanharris commented 1 month ago

This should probably be doing pacman -S --needed. There's no point installing packages again that are already installed.

daniandtheweb commented 1 month ago

Thanks, I forgot about it, I'll add it later.

Apparently some packages require dpkg and rpm (roct-thunk-interface), however it's quite risky to have those as they may break the system if used in the wrong way. If those dependencies can't be avoided by changing the build scripts it may be better to install and uninstall them after they're no longer required, what do you think about it?

daniandtheweb commented 1 month ago

I'm stuck at bulding hipblaslt, any help with this? msgpack-c and msgpack-cxx are installed so I'm not sure what else could I need.

-- Adding /home/daniandtheweb/WorkSpace/rocm_sdk_builder/builddir/025_02_hipBLASLt/virtualenv to CMAKE_PREFIX_PATH
CMake Error at /home/daniandtheweb/WorkSpace/rocm_sdk_builder/builddir/025_02_hipBLASLt/virtualenv/lib/python3.9/site-packages/Tensile/Source/lib/CMakeLists.txt:105 (find_package):
  By not providing "Findmsgpack.cmake" in CMAKE_MODULE_PATH this project has
  asked CMake to find a package configuration file provided by "msgpack", but
  CMake did not find one.

  Could not find a package configuration file provided by "msgpack" with any
  of the following names:

    msgpackConfig.cmake
    msgpack-config.cmake

  Add the installation prefix of "msgpack" to CMAKE_PREFIX_PATH or set
  "msgpack_DIR" to a directory containing one of the above files.  If
  "msgpack" provides a separate development package or SDK, be sure it has
  been installed.

-- Configuring incomplete, errors occurred!
configure failed: hipBLASLt

EDIT: I've temporarily solved this by removing the "REQUIRED" from the problematic line of the CMakeList and I'm checking if it builds correctly. It may be better to create a patch in order for the CMake command to find the package.

daniandtheweb commented 1 month ago

By changing line 105 of rocm_sdk_builder/src_projects/hipBLASLt/tensilelite/Tensile/Source/lib/CMakeLists.txt to find_package(msgpack REQUIRED NAMES msgpack msgpack-cxx msgpack-c) CMake is able to find the dependency and the build can continue. Same fix has to be applied to AMDMIGraphX at src/CMakeLists.txt.

lamikr commented 1 month ago

Thanks, those fixes may be needed also on latest ubuntu. Have not had time to test yet as working for Fedora 40 last fixes.

daniandtheweb commented 1 month ago

I've never created git patches so once I upload them tell me what you think about them. For now I'm building magma with no issues. I hope pytorch doesn't have the same issues as Fedora once I reach it.

lamikr commented 1 month ago

Yes, I just tried to search upstream pytorch for same problem but could not find anything yet. May need to open there an issue to check if somebody has glue for it. For the first pytorch problem I am almost ready to put patch out. Just doing test builds on Ubuntu 23.10 to verify it does not cause any problem with older gcc's.

daniandtheweb commented 1 month ago

Pytorch build still fails after today's new patches, however this may be the same issue found in Fedora so I'll just prepare the required patches for this pr and mark it as ready.

lamikr commented 1 month ago

Hi, I tried to merge these via github interface but it says that "Draft pull requests cannot be merged." Could you perhaps also merge these changes to one commit?

lamikr commented 1 month ago

I did not merge yet the latest pyrorch change that fixed the build problem for me on Fedora but I put to issue 12 comments how to change that file manually.

Before the build starts from clean the file is actually aten/src/ATen/native/cuda/IndexKernel.cu,build process will then make hip version of that file. ./aten/src/ATen/native/hip/IndexKernel.hip

As you have already started the build, you need to change the hip version and then just run the ./babs.sh -b again.

It's little tricky to see the exact error message as there are so many threads each building their own file and once error happens, other threads will still continue writing messages to console

daniandtheweb commented 1 month ago

I'm still unsure what to do with the rpm and dpkg dependencies. They can break Arch if used wrongly but they're required to build roct thunk. I actually wouldn't want unexperienced users to break their system by mistake by making the deps installer install such dangerous packages. Maybe this can be handled by the binfo file or something by installing those two dependencies and uninstalling them once roct-thunk has been installed?

daniandtheweb commented 1 month ago

@lamikr Would you mind creating the patches yourself? I'm able to create them but I'm probably messing something up because I can't apply them.

The required patches are: at line 105 of hipBLASLt/tensilelite/Tensile/Source/lib/CMakeLists.txt change the line for:find_package(msgpack REQUIRED NAMES msgpack msgpack-cxx msgpack-c) The next lines probably need some adjustements too as they keep referring to msgpackc and msgpackc-cxx instead of msgpack-c and msgpack-cxx, but it still builds without any further change.

A similar patch is needed for AMDMIGraphX at src/CMakeLists.txt: at line 298 there starts to be references to msgpackc-cxx, those references need to be adapted to work with msgpack-cxx.

Other than that this PR is ready to be merged and with those two patches plus your changes to pytorch everything should build fine on Arch. I hope this helps to make this work on latest Ubuntu too.

jeroen-mostert commented 4 weeks ago

FWIW I can confirm that with these dependencies (and the necessary fix for msgpack) the build also works for Manjaro (unstable, at least). Which is not surprising as it's practically the same thing as Arch, but technically means you can add another distro to the supported list, thus furthering the goal of world domination (evil laughter optional).

Note that for the build to succeed I needed the patches in the wip/rocm_sdk_builder_611_bg12_pytorch_clamp branch, so those issues are definitely related to newer versions of packages overall and not exclusive to Fedora.

daniandtheweb commented 4 weeks ago

Manjaro unstable (and stable too) is almost like Arch when it comes to packages so I'm not sure if it makes sense to add it, however I'm glad this works for someone else besides me.

jeroen-mostert commented 4 weeks ago

Well, my point was just that adding |manjaro to the checks in combination with the Arch changes here is enough to get things working on Manjaro, it's essentially a freebie. Of course it's easy enough for Manjaro users to do this themselves as well.

daniandtheweb commented 4 weeks ago

Ohh, you're right, sorry. I forgot about that.

I'll add it right now.

daniandtheweb commented 4 weeks ago

I think like this it should work, let me know if it's okay.

jeroen-mostert commented 4 weeks ago

Yep, that's what I had in mind. I no longer have a fresh base to test with but the script runs and pacman dutifully warns that all packages are already installed.

lamikr commented 4 weeks ago

Nice to have new distros available! ... and also the packaging problem solved that I did not notice... These patches looks good to me and does not even have any merge conflicts, so I will mege all of them now.

I have newer used Arch linux or Manjaro, but I could do that at least on virtual machine during the following days. Can advice which install media/iso/version I should download?

daniandtheweb commented 4 weeks ago

If you don't want to configure a lot of stuff just go with Manjaro, it's basically a preconfigured Arch with the package updates slightly delayed for stability reasons. If you want to stay with the original Arch repos you can also try EndeavourOS, it's pure Arch but preconfigured.

daniandtheweb commented 4 weeks ago

@lamikr Would you mind creating the patches yourself? I'm able to create them but I'm probably messing something up because I can't apply them.

The required patches are: at line 105 of hipBLASLt/tensilelite/Tensile/Source/lib/CMakeLists.txt change the line for:find_package(msgpack REQUIRED NAMES msgpack msgpack-cxx msgpack-c) The next lines probably need some adjustements too as they keep referring to msgpackc and msgpackc-cxx instead of msgpack-c and msgpack-cxx, but it still builds without any further change.

A similar patch is needed for AMDMIGraphX at src/CMakeLists.txt: at line 298 there starts to be references to msgpackc-cxx, those references need to be adapted to work with msgpack-cxx.

Other than that this PR is ready to be merged and with those two patches plus your changes to pytorch everything should build fine on Arch. I hope this helps to make this work on latest Ubuntu too.

The patch I mentioned first for hipBLASlt is still required.

lamikr commented 1 week ago

@daniandtheweb I created the https://github.com/lamikr/rocm_sdk_builder/pull/76 and did the patch 0007- in a way that you are the author. Is that ok?

AMDMIGraphX seems to has already some fixes to msgpack search as ubuntu has seemed to have problems also. Does the current version work with Manjaro and Arch linux or are more changes needed to msgpack search?

daniandtheweb commented 1 week ago

Thanks, that was the only missing patch in order to build for Arch. I didn't know that I could set the github email as a patch email, actually that was the main reason I wasn't creating the patch myself.