Add fix for llvm/llvm-project#61402

conda-forge / llvmdev-feedstock

A conda-smithy repository for llvmdev.

BSD 3-Clause "New" or "Revised" License

8 stars 41 forks source link

Add fix for llvm/llvm-project#61402 #209

Closed gmarkall closed 1 year ago

gmarkall commented 1 year ago

This addresses an issue that is triggered by Numba 0.57 - see https://github.com/numba/numba/issues/8738

Checklist

[X] Used a personal fork of the feedstock to propose changes
[X] Bumped the build number (if the version is unchanged)
[ ] Reset the build number to 0 (if the version changed)
[x] Re-rendered with the latest conda-smithy (Use the phrase code>@<space/conda-forge-admin, please rerender in a comment in this PR for automated rerendering)
[ ] Ensured the license file is being packaged.

conda-forge-webservices[bot] commented 1 year ago

Hi! This is the friendly automated conda-forge-linting service.

I just wanted to let you know that I linted all conda-recipes in your PR (recipe) and found it was in an excellent condition.

h-vetinari commented 1 year ago

Doesn't numba + LLVM 14 also need the SVML patches? I have an open PR for that that didn't get any comments: #192

Let's see if this compiles a bit faster after a rerender: @conda-forge-admin, please rerender

gmarkall commented 1 year ago

Can we please review / merge this? It will be breaking many AArch64 users of Numba. For example, UMAP is unusable on AArch64: https://github.com/numba/numba/issues/8994 / https://github.com/lmcinnes/umap/issues/1004#issuecomment-1559574726.

This patch has already been accepted upstream by LLVM but will not be in a release until 17.x:

Issue: https://github.com/llvm/llvm-project/issues/61402
Review: https://reviews.llvm.org/D146938

h-vetinari commented 1 year ago

Does this change make sense without SVML? I thought that was a requirement for using llvm 14 with numba, as things would break without SVML as well. #192 is waiting for action from the numba/llvmlite team.

gmarkall commented 1 year ago

This is unrelated to SVML. I don't know much about SVML but it's not required for Aarch64. So #192 should be considered completely independently of this.

jakirkham commented 1 year ago

Think Graham's point here is simply this particular change has already gone upstream (and was merged). So may be more easily integrated than the other patch (orthogonal to this one), which hasn't quite made it upstream yet.

h-vetinari commented 1 year ago

Yeah, I got that. I just wanted to confirm what's necessary for numba or not. I haven't had time to double-check with the upstream PR (can do sometime in the next ~12h), but if you want to go ahead and merge, I don't mind!

jakirkham commented 1 year ago

Happy to wait 12hrs 🙂

jakirkham commented 1 year ago

No worries. Been there done that 😅

Thanks Axel! 🙏

gmarkall commented 1 year ago

Many thanks for bumping the build number and the merge!