Optimize scalar and scalar pair representations loaded from ByRef in llvm #111768
Conversation
|
Thanks for the pull request, and welcome! The Rust team is excited to review your changes, and you should hear from @TaKO8Ki (or someone else) soon. Please see the contribution instructions for more information. Namely, in order to ensure the minimum review times lag, PR authors and assigned reviewers should ensure that the review label (
|
rustbot
added
S-waiting-on-review
|
Some changes occurred in compiler/rustc_codegen_gcc cc @antoyo |
|
@bors try @rust-timer queue |
This comment has been minimized.
This comment has been minimized.
rustbot
added
the
S-waiting-on-perf
|
⌛ Trying commit 0fe02e65f275e3c8c042fd79a2a62d226328d57a with merge 5ee8ee5de1f637859b1e46db36149f8db2f6d377... |
|
☀️ Try build successful - checks-actions |
This comment has been minimized.
This comment has been minimized.
|
Finished benchmarking commit (5ee8ee5de1f637859b1e46db36149f8db2f6d377): comparison URL. Overall result: no relevant changes - 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 benchmark run did not return any relevant results for this metric. 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.
CyclesThis benchmark run did not return any relevant results for this metric. Binary sizeResultsThis 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.
Bootstrap: 643.295s -> 642.362s (-0.15%) |
rustbot
removed
the
S-waiting-on-perf
|
Some changes occurred in compiler/rustc_codegen_gcc cc @antoyo |
|
Addressed all review comments and added a codegen test (before the commit that changes things, so you can see the behaviour change to the test on the second commit) |
|
Thanks. Should we eventually have a In the mean time: |
bors
added
S-waiting-on-bors
|
⌛ Testing commit 164d041 with merge 658227491b895be301efb45e70e2844021ab7421... |
|
The job Click to see the possible cause of the failure (guessed by this bot) |
|
💔 Test failed - checks-actions |
bors
removed
the
S-waiting-on-bors
bors
added
the
S-waiting-on-review
I'm trying to remove that, and instead just rely on |
|
@bors retry windows rustdoc stack overflow |
bors
added
S-waiting-on-bors
|
☀️ Test successful - checks-actions |
|
Finished benchmarking commit (3266c36): comparison URL. Overall result: no relevant changes - no action needed@rustbot label: -perf-regression Instruction countThis benchmark run did not return any relevant results for this metric. 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.
CyclesThis benchmark run did not return any relevant results for this metric. Binary sizeResultsThis 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.
Bootstrap: 642.797s -> 643.826s (0.16%) |
Only rewrite valtree-constants to patterns and keep other constants opaque
Now that we can reliably fall back to comparing constants with `PartialEq::eq` to the match scrutinee, we can
1. eagerly try to convert constants to valtrees
2. then deeply convert the valtree to a pattern
3. if the to-valtree conversion failed, create an "opaque constant" pattern.
This PR specifically avoids any behavioral changes or major cleanups. What we can now do as follow ups is
* move the two remaining call sites to `destructure_mir_constant` off that query
* make valtree to pattern conversion infallible
* this needs to be done after careful analysis of the effects. There may be user visible changes from that.
based on rust-lang#111768
Optimize scalar and scalar pair representations loaded from ByRef in llvm in rust-lang#105653 I noticed that we were generating suboptimal LLVM IR if we had a `ConstValue::ByRef` that could be represented by a `ScalarPair`. Before rust-lang#105653 this is probably rare, but after it, every slice will go down this suboptimal code path that requires LLVM to untangle a bunch of indirections and translate static allocations that are only used once to read a scalar pair from.
in #105653 I noticed that we were generating suboptimal LLVM IR if we had a
ConstValue::ByRefthat could be represented by aScalarPair. Before #105653 this is probably rare, but after it, every slice will go down this suboptimal code path that requires LLVM to untangle a bunch of indirections and translate static allocations that are only used once to read a scalar pair from.