LLVM 10 #599
Merged
LLVM 10 #599
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
LLVM 10 removes llvm::make_unique in favor of std::make_unique. However, this requires C++14 and is therefore unsuitable for LLVM 9 that forces -std=c++11. Update the code to use both conditionally. This fixes all issues with LLVM 10.
This is needed as the llvmlite package we are building needs to differentiate itself somehow from the 'other' dev builds.
|
Build-farm id: |
|
Note to self:
|
|
This works with the CUDA target, with the exception of It seems that for code like: def foo(x):
# Triggers a generation of llvm.memset
for i in range(x.size):
x[i] = 0a {
B0.endif:
%.74 = icmp sgt i64 %arg.x.2, 0
br i1 %.74, label %B26.loopexit, label %B26
B26.loopexit: ; preds = %B0.endif
%arg.x.41 = bitcast i32* %arg.x.4 to i8*
%0 = shl nuw i64 %arg.x.2, 2
call void @llvm.memset.p0i8.i64(i8* align 4 %arg.x.41, i8 0, i64 %0, i1 false)
br label %B26
B26: ; preds = %B26.loopexit, %B0.endif
store i8* null, i8** %.ret, align 8
ret i32 0
}but now it looks like: entry:
%.74.inv = icmp sgt i64 %arg.x.2, 0
br i1 %.74.inv, label %B14, label %B26
B14: ; preds = %entry, %B14
%.110.sroa.0.0114 = phi i64 [ %.110.sroa.0.0, %B14 ], [ 0, %entry ]
%.120113 = phi i1 [ %.120, %B14 ], [ %.74.inv, %entry ]
%.129108112 = phi i64 [ %.129107, %B14 ], [ %arg.x.2, %entry ]
%.133110111 = phi i64 [ %.133109, %B14 ], [ 0, %entry ]
%.129 = sext i1 %.120113 to i64
%.129107 = add nsw i64 %.129108112, %.129
%.133 = zext i1 %.120113 to i64
%.133109 = add i64 %.133110111, %.133
%.198 = icmp slt i64 %.110.sroa.0.0114, 0
%.199 = select i1 %.198, i64 %arg.x.5.0, i64 0
%.200 = add i64 %.199, %.110.sroa.0.0114
%.213 = getelementptr i32, i32* %arg.x.4, i64 %.200
store i32 0, i32* %.213, align 4
%.120 = icmp sgt i64 %.129107, 0
%.110.sroa.0.0 = select i1 %.120, i64 %.133109, i64 0
br i1 %.120, label %B14, label %B26
B26: ; preds = %B14, %entry
store i8* null, i8** %.ret, align 8
ret i32 0
}i.e. a loop is generated instead of the |
|
@gmarkall awesome, thanks for looking into this! |
|
It seems that LLVM 10 doesn't do loop idiom recognition at an opt level of 1 - If I apply to Numba: diff --git a/numba/cuda/codegen.py b/numba/cuda/codegen.py
index e201a2101..0d74b1edc 100644
--- a/numba/cuda/codegen.py
+++ b/numba/cuda/codegen.py
@@ -19,7 +19,7 @@ class CUDACodeLibrary(CodeLibrary):
# Run some lightweight optimization to simplify the module.
# This seems to workaround a libnvvm compilation bug (see #1341)
pmb = ll.PassManagerBuilder()
- pmb.opt_level = 1
+ pmb.opt_level = 3
pmb.disable_unit_at_a_time = False
pmb.disable_unroll_loops = True
pmb.loop_vectorize = Falsethen the |
|
I can't see any negative knock-on effects of using the patch above to use optimization level 3 - the Numba and cuDF test suites pass with no issues, and it will probably result in easier to read optimized IR for future debugging purposes. @esc @sklam @stuartarchibald do you have any objections / concerns about just applying the above patch, when llvmlite moves to LLVM 10? |
|
@gmarkall I have an LLVM 10 enabled version of |
esc
added a commit
to esc/numba
that referenced
this pull request
Jul 9, 2020
As suggested by @gmarkall : numba/llvmlite#599 (comment)
With LLVM 10 the following CMake line no longer appears to work. ``` llvm_map_components_to_libnames(llvm_libs all) ``` ... and this commit is a suitable workaround. Current working hypothesis is that this might have been caused by the change in: llvm/llvm-project@ab41180#diff-cebfde545aa260d20ca6b0cdc3da08daR270 However the docs at https://llvm.org/docs/CMake.html are not very clear about this. And in addition the output of `llvm-config --components` does list `all` as an option. Many thanks to @angloyna for debugging this!!
esc
commented
Jul 20, 2020
conda-recipes/llvmlite/meta.yaml
Outdated
| @@ -1,4 +1,4 @@ | |||
| {% set VERSION_SUFFIX = "" %} # debug version suffix, appended to the version | |||
| {% set VERSION_SUFFIX = "_llvm10" %} # debug version suffix, appended to the version | |||
There was a problem hiding this comment.
Note to self: this change must be reverted before merging to master.
This was referenced Jul 20, 2020
Closed
|
@sklam @stuartarchibald I have implemented the LLVM version split for |
This reverts commit 3f66129. Once numba/numba#6030 is merged, it will no longer be necessary to disable the autoupgrade of atomic intrinsics for NVPTX, because LLVM from llvmlite will not be used to optimize the IR before sending it to NVVM.
The page at: https://releases.llvm.org/10.0.1/docs/ Currently (Tue Aug 4 10:48:42 2020 GMT+2) returns a 404 - page not found and this breaks our build as Sphinx can't setup the intersphinx links.
As we are doing a split release for 0.34.0 and `aarch64` will need LLVM 9.0.* -- we allow to build with this version again.
Fix missing word and missing punctuation. Co-authored-by: stuartarchibald <stuartarchibald@users.noreply.github.com>
We only support 10.0.* and 9.0.* from now on.
esc
added
Pending BuildFarm
This was referenced Aug 5, 2020
Closed
Closed
Final builds should not have a version suffix.
|
the version revert looks good |
|
damn, you have taken too long to review my PRs that I am using llvm 12 currently. |
|
BFID: |
esc
added
BuildFarm Passed
|
🎉 |
|
💯 |
sklam
added a commit
to sklam/llvmlite
that referenced
this pull request
Aug 5, 2020
ghost
mentioned this pull request
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
As title, upgrade to LLVM 10.