Add a layout_of_val intrinsic for when you want both size_of_val+align_of_val#152867
Add a layout_of_val intrinsic for when you want both size_of_val+align_of_val#152867scottmcm wants to merge 3 commits intorust-lang:mainfrom
layout_of_val intrinsic for when you want both size_of_val+align_of_val#152867Conversation
|
Some changes occurred to the intrinsics. Make sure the CTFE / Miri interpreter cc @rust-lang/miri, @RalfJung, @oli-obk, @lcnr
cc @rust-lang/miri |
|
This has another library change that the previous perf run didn't, so checking things are still good |
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
Add a `layout_of_val` intrinsic for when you want both `size_of_val`+`align_of_val`
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
|
Finished benchmarking commit (bb0e06e): comparison URL. Overall result: ❌✅ regressions and improvements - please read the text belowBenchmarking this pull request means it may be perf-sensitive – we'll automatically label it not fit for rolling up. You can override this, but we strongly advise not to, due to possible changes in compiler perf. Next Steps: If you can justify the regressions found in this try perf run, please do so in sufficient writing along with @bors rollup=never Instruction countOur most reliable metric. Used to determine the overall result above. However, even this metric can be noisy.
Max RSS (memory usage)Results (primary -0.3%, secondary -3.8%)A less reliable metric. May be of interest, but not used to determine the overall result above.
CyclesResults (secondary 1.7%)A less reliable metric. May be of interest, but not used to determine the overall result above.
Binary sizeResults (primary -0.3%, secondary -1.3%)A less reliable metric. May be of interest, but not used to determine the overall result above.
Bootstrap: 481.703s -> 481.307s (-0.08%) |
fdaf983 to
583c5ff
Compare
|
Some changes occurred to MIR optimizations cc @rust-lang/wg-mir-opt |
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
583c5ff to
d56e1b5
Compare
This comment has been minimized.
This comment has been minimized.
d56e1b5 to
fb5b025
Compare
|
@bors try @rust-timer queue |
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
Add a `layout_of_val` intrinsic for when you want both `size_of_val`+`align_of_val`
This comment has been minimized.
This comment has been minimized.
|
Finished benchmarking commit (42ba6e2): comparison URL. Overall result: ❌✅ regressions and improvements - please read the text belowBenchmarking this pull request means it may be perf-sensitive – we'll automatically label it not fit for rolling up. You can override this, but we strongly advise not to, due to possible changes in compiler perf. Next Steps: If you can justify the regressions found in this try perf run, please do so in sufficient writing along with @bors rollup=never Instruction countOur most reliable metric. Used to determine the overall result above. However, even this metric can be noisy.
Max RSS (memory usage)Results (primary 0.6%, secondary -2.8%)A less reliable metric. May be of interest, but not used to determine the overall result above.
CyclesResults (primary 3.2%, secondary -3.8%)A less reliable metric. May be of interest, but not used to determine the overall result above.
Binary sizeResults (primary -0.3%, secondary -1.4%)A less reliable metric. May be of interest, but not used to determine the overall result above.
Bootstrap: 496.246s -> 480.362s (-3.20%) |
|
I think the perf is still not as nicely green as #152786 (comment) |
fb5b025 to
09634e6
Compare
This comment has been minimized.
This comment has been minimized.
…r=mati865 Tighten the `!range` bounds on alignments in vtables Right now we're only telling LLVM that they're non-zero, but alignments must be powers of two so can't be more than `isize::MAX+1`. And we actually never emit anything beyond LLVM's limit of 2²⁹, so outside of 16-bit targets the limit is that. (Pulled out from rust-lang#152867 which is starting to have too much in it.)
…r=mati865 Tighten the `!range` bounds on alignments in vtables Right now we're only telling LLVM that they're non-zero, but alignments must be powers of two so can't be more than `isize::MAX+1`. And we actually never emit anything beyond LLVM's limit of 2²⁹, so outside of 16-bit targets the limit is that. (Pulled out from rust-lang#152867 which is starting to have too much in it.)
Rollup merge of #152929 - scottmcm:better-alignment-ranges, r=mati865 Tighten the `!range` bounds on alignments in vtables Right now we're only telling LLVM that they're non-zero, but alignments must be powers of two so can't be more than `isize::MAX+1`. And we actually never emit anything beyond LLVM's limit of 2²⁹, so outside of 16-bit targets the limit is that. (Pulled out from #152867 which is starting to have too much in it.)
It was already computing both anyway with `size_of_val::size_and_align_of_dst`, so we just need to return them both instead of ignoring one.
Same as rust-lang/rust 152689 did for align/size.
09634e6 to
68910ca
Compare
|
This PR was rebased onto a different main commit. Here's a range-diff highlighting what actually changed. Rebasing is a normal part of keeping PRs up to date, so no action is needed—this note is just to help reviewers. |
|
@bors try @rust-timer queue |
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
Add a `layout_of_val` intrinsic for when you want both `size_of_val`+`align_of_val`
This comment has been minimized.
This comment has been minimized.
|
Finished benchmarking commit (6db21b4): comparison URL. Overall result: ❌✅ regressions and improvements - please read the text belowBenchmarking this pull request means it may be perf-sensitive – we'll automatically label it not fit for rolling up. You can override this, but we strongly advise not to, due to possible changes in compiler perf. Next Steps: If you can justify the regressions found in this try perf run, please do so in sufficient writing along with @bors rollup=never Instruction countOur most reliable metric. Used to determine the overall result above. However, even this metric can be noisy.
Max RSS (memory usage)Results (primary -0.6%, secondary -0.3%)A less reliable metric. May be of interest, but not used to determine the overall result above.
CyclesResults (primary 2.3%, secondary -0.9%)A less reliable metric. May be of interest, but not used to determine the overall result above.
Binary sizeResults (primary -0.4%, secondary -1.6%)A less reliable metric. May be of interest, but not used to determine the overall result above.
Bootstrap: 480.585s -> 482.646s (0.43%) |
View all comments
(This is a replacement for #152786 where github broke)
Today we're emitting a bunch more MIR and LLVM-IR than we need for stuff like this.
Fixes #152773
Closes #152641 because this solves the motivating example from that PR of avoiding the alignment transmutes when getting a layout from a DST, just in a better way.
While I was in the area I also put a more specific alignment
!rangemetadata on loading alignment from a vtable and some extra no-wrap flags when calculating the size of an ADT with an unsized tail.Same reviewer as was rolled before,
r? @JonathanBrouwer