-
Notifications
You must be signed in to change notification settings - Fork 12.7k
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
Update the minimum external LLVM to 7 #66973
Conversation
LLVM 7 is over a year old, which should be plenty for compatibility. The last LLVM 6 holdout was llvm-emscripten, which went away in rust-lang#65501. I've also included a fix for LLVM 8 lacking `MemorySanitizerOptions`, which was broken by rust-lang#66522.
r? @kennytm (rust_highfive has picked a reviewer for you, use r? to override) |
const bool CompileKernel = false; | ||
|
||
return wrap(createMemorySanitizerLegacyPassPass( | ||
MemorySanitizerOptions{TrackOrigins, Recover, CompileKernel})); | ||
#elif LLVM_VERSION_GE(8, 0) | ||
return wrap(createMemorySanitizerLegacyPassPass(TrackOrigins, Recover)); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is the LLVM 8 fix.
cc @rust-lang/infra -- do we have any budget for more CI builders? Currently we only test the minimum external LLVM and rust's own src/llvm-project fork, but there are more possible LLVM versions between...
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I suspect no, or if we did, this would not be a good use IMO.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Even just ./x.py check
on those middle versions would be an improvement.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Currently x.py check doesn't do anything at all LLVM-wise -- we don't attempt to compile it, intentionally, so that even people with smaller machines can at least sort of contribute to the compiler.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ah, darn. We could let that build script work in full when using external LLVM, which shouldn't be expensive, but I won't press further on this point. Thanks anyway.
@bors: r+ Thanks for this @cuviper! I agree with @Mark-Simulacrum that we unfortunately probably don't have budget right now to test the build against multiple LLVM versions. |
📌 Commit 2304c25 has been approved by |
Update the minimum external LLVM to 7 LLVM 7 is over a year old, which should be plenty for compatibility. The last LLVM 6 holdout was llvm-emscripten, which went away in rust-lang#65501. I've also included a fix for LLVM 8 lacking `MemorySanitizerOptions`, which was broken by rust-lang#66522.
Update the minimum external LLVM to 7 LLVM 7 is over a year old, which should be plenty for compatibility. The last LLVM 6 holdout was llvm-emscripten, which went away in rust-lang#65501. I've also included a fix for LLVM 8 lacking `MemorySanitizerOptions`, which was broken by rust-lang#66522.
Rollup of 7 pull requests Successful merges: - #66750 (Update the `wasi` crate for `wasm32-wasi`) - #66878 (Move Sessions into (new) librustc_session) - #66903 (parse_enum_item -> parse_enum_variant) - #66951 (miri: add throw_machine_stop macro) - #66957 (Change Linker for x86_64-fortanix-unknown-sgx target to rust-lld) - #66960 ([const-prop] Fix ICE calculating enum discriminant) - #66973 (Update the minimum external LLVM to 7) Failed merges: r? @ghost
LLVM 7 is over a year old, which should be plenty for compatibility. The
last LLVM 6 holdout was llvm-emscripten, which went away in #65501.
I've also included a fix for LLVM 8 lacking
MemorySanitizerOptions
,which was broken by #66522.