[Date Prev][Date Next] [Thread Prev][Thread Next] [Date Index] [Thread Index]

Re: ROCm 4.5.2 Compiler support code object manager - Debian package wonder



Hi Maxime,

I am not an LLVM/Comgr developer, my involvement is purely to resolve packaging and license related concerns, so feel free to reach out if you have issues along those lines.

With that said, I did reach out to the developers to see if your patch is ok. For future reference, please either reach out to the developers via creating a github issue:
https://github.com/RadeonOpenCompute/ROCm-CompilerSupport/issues
Or the distribution list for smaller/quick questions like this:
rocm.compiler.support@amd.com

If you're using LLVM from sid/bookworm, then this should be fine. The comgr component from 4.5.* is targeting llvm 13 with the next bump (5.0.0) including an update to llvm 14, so I would recommend sticking to 4.5.* until LLVM in sid is updated to LLVM 14 (maybe this summer/fall?).

I'm working on adding comgr packages to Fedora in my spare time, so feel free to reach out to me if you have issues with the Debian packaging effort. I don't currently have a comgr package that compiles as-is, so I see why you built using amd-stg-open. I personally hope to use 4.5.2 and cherry-pick from amd-stg-open if possible.

For some context, ROCm's compiler team rebases llvm/comgr/device-libs amd-stg-open branch almost daily against LLVM's trunk (main branch). This code is then branched and has up to a 3 month delay any to ROCm release branch. E.g. 4.5.0 was tagged in October, so it likely comgr was developed against a snapshot of llvm trunk from July, about a month before it was branched into the LLVM 13 release branch. You can see what LLVM version was used by checking the ROCm llvm-project github tree here:
https://github.com/RadeonOpenCompute/llvm-project/tree/rocm-4.5.0
The cmake file has the version embedded in it for reference:
https://github.com/RadeonOpenCompute/llvm-project/blob/rocm-4.5.0/llvm/CMakeLists.txt

I'm not familiar with the nitty gritties of LLVM, but if I understand correctly, the developers said that the function in question has unused arguments in LLVM 13. The change to the arguments was introduced in LLVM trunk mid last year, but implementation was ultimately deferred to LLVM 14. Upstream reverted the function in trunk prior to branching LLVM 13, but unfortunately, ROCm rebased its LLVM branch prior to this revert.

Anyway, please go ahead with your patch if you plan to use comgr's rocm-4.5.2 tag, I realise you likely don't want to build another copy of llvm just for comgr :)

Fortunately, ROCm is trying to move towards targeting the latest LLVM release branch, instead of LLVM trunk, to avoid these sorts of issues in the future, but unfortunately, there's no timeline on this effort.

Thanks

On Sun, Jan 23, 2022, at 9:28 PM, Maxime Chambonnet wrote:
 Hello Jeremy,

I am busy with the Debian ROCm team [1] packaging ROCm.
I have a wonder as I saw you active in comgr development:
We are trying to package ROCm 4.5.2 with the official LLVM 13
toolchain instead of AMD LLVM forks [2].

Compiling comgr with upstream llvm-13 runs into issues, I patched them
from the amd-stg-open branch, current commit HEAD~1 [SHA1|3].
I am especially worried about the 0002-MCStreamer-InitSections patch.
Is such a patch acceptable [4]? Will it break subtly something
important? Does it do anything except allowing me to compile comgr?

So far as you can see from [2], the whole stack has been passing most
tests and benchmarks (but not all) with this patch and llvm-13.

Best regards, Maxime

[1] https://salsa.debian.org/rocm-team
[2] https://lists.debian.org/debian-ai/2022/01/msg00000.html
[3] 675501d47b582807183c4bc28d848e76118d3aa9
[4] 
https://salsa.debian.org/rocm-team/rocm-compilersupport/-/blob/master/debian/patches/0002-MCStreamer-InitSections.patch

Attachments:


Reply to: