Add a layout_of_val intrinsic for when you want both size_of_val+align_of_val#152786
Add a layout_of_val intrinsic for when you want both size_of_val+align_of_val#152786scottmcm wants to merge 6 commits intorust-lang:mainfrom
layout_of_val intrinsic for when you want both size_of_val+align_of_val#152786Conversation
|
@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.
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.
This comment has been minimized.
This comment has been minimized.
|
Finished benchmarking commit (f751a56): comparison URL. Overall result: ✅ improvements - no action neededBenchmarking 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. @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.2%, secondary -2.7%)A less reliable metric. May be of interest, but not used to determine the overall result above.
CyclesResults (primary -2.1%, secondary -3.5%)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.396s -> 480.025s (-0.08%) |
ddf6fe3 to
9cabb08
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.
This comment has been minimized.
This comment has been minimized.
This comment was marked as outdated.
This comment was marked as outdated.
3ecb0bc to
9cabb08
Compare
|
I'd tried something else that just didn't work, so I reverted back to what got the previous fully-green perf run (#152786 (comment)). |
|
Some changes occurred to the intrinsics. Make sure the CTFE / Miri interpreter cc @rust-lang/miri, @RalfJung, @oli-obk, @lcnr
cc @rust-lang/miri |
|
rustbot has assigned @JonathanBrouwer. Use Why was this reviewer chosen?The reviewer was selected based on:
|
| // Alignment is always nonzero. | ||
| bx.range_metadata(align, WrappingRange { start: 1, end: !0 }); | ||
| // Alignment is always a power of two, thus 1..=0x800…000. | ||
| let align_bound = size_bound + 1; |
There was a problem hiding this comment.
In fact the bound is Align::MAX, is it worth emitting that?
There was a problem hiding this comment.
Oh, really good point 👍 I was stuck still thinking of it as a load of ptr::Alignment, but it's not.
I don't know how much it'll really matter, since isize::MAX+1 is enough to prove that size+align doesn't unsigned-overflow, but might as well include it because it's easy, cheap, clearly not worse, and makes the test verification check slightly nicer.
| sym::size_of_val => llsize, | ||
| sym::align_of_val => llalign, | ||
| sym::layout_of_val => { | ||
| // Use the builder so we're insulated from the in-memory field order |
There was a problem hiding this comment.
We're still relying on the in-source field order here. That deserves a comment on both ends (here and in the library). Or can the field lookup here be done by name?
…lignment from a vtable
…udly and clearly ICE and fail library tests.
|
Something really odd is happening here where github is noticing the pushes in my fork but not in this pr. turning it off and on again... |
|
The job Click to see the possible cause of the failure (guessed by this bot) |
EDIT: github stopped picking up pushes here, so moved to #152867
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.