Don't use Immediate::offset to transmute pointers to integers#131068
Don't use Immediate::offset to transmute pointers to integers#131068bors merged 2 commits intorust-lang:masterfrom
Conversation
|
r? @chenyukang rustbot has assigned @chenyukang. Use |
|
Some changes occurred to the CTFE / Miri interpreter cc @rust-lang/miri |
This comment has been minimized.
This comment has been minimized.
|
☔ The latest upstream changes (presumably #131006) made this pull request unmergeable. Please resolve the merge conflicts. |
fafb277 to
a8f9a32
Compare
This comment has been minimized.
This comment has been minimized.
|
Some changes occurred to MIR optimizations cc @rust-lang/wg-mir-opt |
|
This is unblocked now, I added a second commit that fixes the ICE. Cc @cjgillot |
35fa9ca to
d329d94
Compare
This comment has been minimized.
This comment has been minimized.
d329d94 to
7d63efd
Compare
|
Some changes occurred in coverage tests. cc @Zalathar |
|
This changes something in the coverage output. I don't know what that output means or why it changes now, but if it relied on bogus optimizations then it's probably better to have it fixed now... |
|
Looks like an artifact of some MIR node no longer being optimized away. As you say, if a bogus optimization is no longer happening, then that's probably for the best. |
|
r? compiler |
| fn offset_(&self, offset: Size, layout: TyAndLayout<'tcx>, cx: &impl HasDataLayout) -> Self { | ||
| debug_assert!(layout.is_sized(), "unsized immediates are not a thing"); | ||
| // Verify that the input matches its type. | ||
| if cfg!(debug_assertions) { |
There was a problem hiding this comment.
Is it worth adding a debug_assert_matches_abi method?
There was a problem hiding this comment.
IMO no, I'd rather not duplicate this logic.
There was a problem hiding this comment.
Wouldn't it look like this, with no logic duplication?
pub fn debug_assert_matches_abi(self, abi: Abi, cx: &impl HasDataLayout) {
if cfg!(debug_assertions) {
self.assert_matches_abi(abi, cx);
}
}There was a problem hiding this comment.
Ah, that's what you mean.
Didn't seem worth it to me. 🤷
|
@bors r+ |
|
☀️ Test successful - checks-actions |
|
Finished benchmarking commit (a964a92): comparison URL. Overall result: ❌ regressions - no action needed@rustbot label: -perf-regression 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)Results (primary 2.1%)This 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.
CyclesResults (secondary -0.5%)This 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: 774.632s -> 773.837s (-0.10%) |
This applies the relatively new
assert_matches_abicheck in theoffsetoperation on immediates, which makes sure that if offsets are used to alter the layout (which is possible because the field layout is arbitrarily picked by the caller), this is not done in a way that breaks the invariant of theImmediatetype.This leads to ICEs in a GVN mir-opt test, so the second commit fixes GVN.
Fixes #131064.