Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Figure out how to use push_unchecked in more places #19

Closed
Lucretiel opened this issue Nov 26, 2019 · 2 comments
Closed

Figure out how to use push_unchecked in more places #19

Lucretiel opened this issue Nov 26, 2019 · 2 comments

Comments

@Lucretiel
Copy link
Contributor

Lucretiel commented Nov 26, 2019

There's an issue in which using push_unchecked in loops, even with a fixed number of iterations, causes inferior assembly to be generated. For example:

staticvec/src/lib.rs

Lines 116 to 133 in 4b2534f

#[inline]
pub fn filled_with<F>(mut initializer: F) -> Self
where F: FnMut() -> T {
let mut res = Self::new();
//You might think it would make more sense to use `push_unchecked` here.
//Originally, I did also! However, as of today (November 19, 2019), doing so
//both in this function and several others throughout the crate inhibits the ability
//of `rustc` to fully unroll and autovectorize various constant-bounds loops. If this changes
//in the future, feel free to open a PR switching out the manual code for `get_unchecked`, if
//you happen to notice it before I do.
for i in 0..N {
unsafe {
res.data.get_unchecked_mut(i).write(initializer());
res.length += 1;
}
}
res
}

The workaround is to manually push by writing directly to self.data and incrementing self.length. However, we'd prefer to minimize the amount of unsafe code we have to write, and we'd rather use "safer unsafes" like push_unchecked that provide a friendlier contract than self.data.get_unchecked_mut().write.

@slightlyoutofphase
Copy link
Owner

slightlyoutofphase commented Nov 26, 2019

I honestly think it has something to do with the fact that MaybeUninit::write in fact has a return value, which is a mutable reference to whatever you just wrote to it.

The docs state this is for your convenience, although IMO it seems like a somewhat strange API. So it may even be that using regular pointer writes in the troublesome spots turns out to work more efficiently (although IIRC that left me with Miri errors at some point early on, because I guess in some cases it wasn't considered the "same" as using the built-in MaybeUninit::write, although generally it should be.)

I'll experiment with some things when I have a bit more time later on to see if I can arrive at a solution that's both fast and sound.

@slightlyoutofphase
Copy link
Owner

I'd say push_unchecked is now basically being used where it makes sense to use it (except for filled_with still, but that seems to be a particularly special case.)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants