make memrchr use align_offset #52744
Conversation
|
r? @Kimundi (rust_highfive has picked a reviewer for you, use r? to override) |
rust-highfive
added
the
S-waiting-on-review
src/libcore/slice/memchr.rs
Outdated
| // a version of align_offset that says how much must be *subtracted* | ||
| // from a pointer to be aligned. | ||
| #[inline(always)] | ||
| fn align_offset_down(ptr: *const u8, align: usize) -> usize { |
There was a problem hiding this comment.
Consider slice::align_to::<usize>()? I believe that does exactly what a large part of this code does.
There was a problem hiding this comment.
Hm, plausible. Should I then also do that for memchr?
There was a problem hiding this comment.
Actually it's not so easy, we want to do the main loop on a u8 slice, not usize (or actually pairs of usize, which is the steps that the loop is taking).
src/libcore/slice/memchr.rs
Outdated
| 0 | ||
| } else { | ||
| // E.g. if align=8 and we have to add 1, then we can also subtract 7. | ||
| align - align_offset |
There was a problem hiding this comment.
You probably want (align - align_offset) & (align - 1) (or % align if you're sure it will optimize) instead of the last if/else. Not sure about the first one though, seems kinda fine as-is.
|
Now we just have to hope that |
|
@bors: r+ |
|
📌 Commit 3bc59b5 has been approved by |
bors
added
S-waiting-on-bors
|
closes #50567 |
make memrchr use align_offset I hope I did not screw that up... Cc @oli-obk who authored the original rust-lang#44537
|
☀️ Test successful - status-appveyor, status-travis |
fix memrchr in miri The previous PR rust-lang#52744 was not enough because it assumed that the split between the `mid` and `end` parts returned by `align_to` was aligned. But really the only guarantee we have is that the `mid` part is aligned, so make use of that.
fix memrchr in miri The previous PR rust-lang#52744 was not enough because it assumed that the split between the `mid` and `end` parts returned by `align_to` was aligned. But really the only guarantee we have is that the `mid` part is aligned, so make use of that.
I hope I did not screw that up...
Cc @oli-obk who authored the original #44537
Fixes #50567 (thanks @bjorn3)