WIP: bootstrap: remove use-lld #116182
WIP: bootstrap: remove use-lld #116182
Conversation
Removes the `use-lld` flag, which doesn't seem very useful.
|
(rustbot has picked a reviewer for you, use r? to override) |
rustbot
added
A-testsuite
|
@bors try @rust-timer queue |
This comment has been minimized.
This comment has been minimized.
rustbot
added
the
S-waiting-on-perf
…try> WIP: bootstrap: remove use-lld Removes the `use-lld` flag, which doesn't seem very useful - it's only used on one place in our CI explicitly, and setting of `lld` is far from being as simple as a boolean option. I think that we should just use lld based on the target specs or other inferred LLD usage. Opening a PR to see what CI has to say about it.
|
☀️ Try build successful - checks-actions |
This comment has been minimized.
This comment has been minimized.
|
Finished benchmarking commit (6d68f23): comparison URL. Overall result: ❌ regressions - no action neededBenchmarking this pull request likely means that it is perf-sensitive, so we're automatically marking it as not fit for rolling up. While you can manually mark this PR as fit for rollup, we strongly recommend not doing so since this PR may lead to changes in compiler perf. @bors rollup=never Instruction countThis is a highly reliable metric that was used to determine the overall result at the top of this comment.
Max RSS (memory usage)ResultsThis is a less reliable metric that may be of interest but was not used to determine the overall result at the top of this comment.
CyclesResultsThis is a less reliable metric that may be of interest but was not used to determine the overall result at the top of this comment.
Binary sizeThis benchmark run did not return any relevant results for this metric. Bootstrap: 633.132s -> 634.219s (0.17%) |
rustbot
removed
the
S-waiting-on-perf
Why? |
Ok! I'd like to know how it's being used. On which targets do you use In terms of our CI, we only use it for x64 Linux, and that target should hopefully switch soon to using LLD by default, so then the only usage of the flag would be removed. Regarding the flag itself, I think that it's a bit messy at the moment. At the same time, it enables the usage of LLD linker flags, and also tells bootstrap to use To clarify, I want to keep some way of telling bootstrap to use I think that one way could be to introduce something like this in bootstrap: enum LldConfig {
Disabled, // don't use lld
External(PathBuf), // use externally provided lld
LocallyBuilt, // use lld built from src/llvm-project
Snapshot // use lld from the snapshot compiler
}or, alternatively, just set |
|
I suppose it's about Windows targets. |
|
For msvc, lld should be a drop in replacement but there have been issues in practice. It's not something we test so arguably it's not technically supported. I believe Chromium use it tho and do report issues. |
On windows-gnu LLD may not work good enough to cover all corner cases, but it certainly works good enough to build and test rustc.
I also have such a hope, but I wouldn't put my money on it. |
I'm not against refactoring the options, but I would like to keep the existing functionality. |
I think that If you use the global |
|
For me it doesn't matter match whether snapshot lld or global lld is used, snapshot lld would even be more convenient - no need to install anything globally. |
Yeah, the reason why it doesn't work is #102101 and specifically this line. #116197 should fix that. |
No, it was (long?) before that. |
Aha, I see. I guess that something like this might also be problematic. |
|
Okay, it didn't take much time, here's the first failure. Log |
|
Ok, so it can't find |
You can look how |
petrochenkov
added
S-waiting-on-author
|
Thanks, I'll take a look at |
Removes the
use-lldflag, which doesn't seem very useful - it's only used on one place in our CI explicitly, and setting oflldis far from being as simple as a boolean option. I think that we should just use lld based on the target specs or other inferred LLD usage. Opening a PR to see what CI has to say about it.