Skip to content

Conversation

@alexcrichton
Copy link
Member

There may be a more efficient implementation of core::util::swap_ptr. The issue mentioned using move_val_init, but I couldn't figure out what that did, so I just used copy_memory a few times instead.

I'm not exactly the best at reading LLVM generated by rust, but this does appear to be optimized away just as expected (when possible).

@thestinger
Copy link
Contributor

The copy_memory function will be faster than move_val_init, which zeroes where it moves from. However, copy_memory uses the memmove intrinsic and the swap function itself (not the *mut version) could take advantage of the guarantee that &mut pointers are non-aliasing and use memcpy instead. It doesn't seem to be exposed anymore though...

@graydon
Copy link
Contributor

graydon commented May 7, 2013

needs a rebase

@alexcrichton
Copy link
Member Author

Should be good to go again

@pcwalton
Copy link
Contributor

Unfortunately this is bitrotted again :(

@alexcrichton
Copy link
Member Author

Rebased for now at least, these keep creeping in...

@alexcrichton
Copy link
Member Author

Well that's unfortunate, right after I pushed I wanted to make sure that the failing test from last time passed, and then I just forgot to check the output an hour later. This time the test actually passes! Perhaps the 5th time's the charm?

bors added a commit that referenced this pull request May 11, 2013
There may be a more efficient implementation of `core::util::swap_ptr`. The issue mentioned using `move_val_init`, but I couldn't figure out what that did, so I just used `copy_memory` a few times instead.

I'm not exactly the best at reading LLVM generated by rust, but this does appear to be optimized away just as expected (when possible).
@bors bors closed this May 11, 2013
flip1995 pushed a commit to flip1995/rust that referenced this pull request Nov 3, 2020
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

Successfully merging this pull request may close these issues.

5 participants