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

Drop unsafe code from next_u64 #961

Closed
wants to merge 1 commit into from
Closed

Conversation

Shnatsel
Copy link
Contributor

@Shnatsel Shnatsel commented Apr 4, 2020

The optimizer is smart enough to optimize the naive version into a single load. This produces identical assembly on x86_64 as the unsafe approach. Tested with the following code:

use rand;
use rand::Rng;
use rand_hc::Hc128Rng;
use rand::SeedableRng;

fn main() {
    let mut rng: Hc128Rng = Hc128Rng::from_entropy();
    let value: u64 = rng.gen();
    println!("{}", value);
}

The generated assembly for <rand_core::os::OsRng as rand_core::RngCore>::next_u64 is:

<rand_core::os::OsRng as rand_core::RngCore>::next_u64:
 push    rbx
 sub     rsp, 96
 mov     qword, ptr, [rsp, +, 8], 0
 lea     rdi, [rsp, +, 8]
 mov     esi, 8
 call    qword, ptr, [rip, +, _ZN9getrandom9getrandom17hf138ee78ea21d54aE@GOTPCREL]
 test    eax, eax
 jne     .LBB22_1
 mov     rax, qword, ptr, [rsp, +, 8]
 add     rsp, 96
 pop     rbx
 ret
.LBB22_1:
 mov     ebx, eax
 mov     edi, 4
 mov     esi, 4
 call    qword, ptr, [rip, +, __rust_alloc@GOTPCREL]
 test    rax, rax
 jne     .LBB22_4
 mov     edi, 4
 mov     esi, 4
 call    qword, ptr, [rip, +, _ZN5alloc5alloc18handle_alloc_error17h64858b6e648c0ffcE@GOTPCREL]
 ud2
.LBB22_4:
 mov     dword, ptr, [rax], ebx
 mov     qword, ptr, [rsp, +, 16], rax
 lea     rax, [rip, +, .Lanon.d19b81ac8d75321f86db47584407e1c3.7]
 mov     qword, ptr, [rsp, +, 24], rax
 lea     rax, [rsp, +, 16]
 mov     qword, ptr, [rsp, +, 32], rax
 mov     rax, qword, ptr, [rip, +, _ZN62_$LT$rand_core..error..Error$u20$as$u20$core..fmt..Display$GT$3fmt17h5b71b604d6d2a4b6E@GOTPCREL]
 mov     qword, ptr, [rsp, +, 40], rax
 lea     rax, [rip, +, .Lanon.d19b81ac8d75321f86db47584407e1c3.11]
 mov     qword, ptr, [rsp, +, 48], rax
 mov     qword, ptr, [rsp, +, 56], 1
 mov     qword, ptr, [rsp, +, 64], 0
 lea     rax, [rsp, +, 32]
 mov     qword, ptr, [rsp, +, 80], rax
 mov     qword, ptr, [rsp, +, 88], 1
 lea     rsi, [rip, +, .Lanon.d19b81ac8d75321f86db47584407e1c3.13]
 lea     rdi, [rsp, +, 48]
 call    qword, ptr, [rip, +, _ZN3std9panicking15begin_panic_fmt17h3f391831d1286decE@GOTPCREL]
 ud2
.LBB22_3:
 mov     rbx, rax
 lea     rdi, [rsp, +, 16]
 call    core::ptr::drop_in_place
 mov     rdi, rbx
 call    _Unwind_Resume
 ud2

@Shnatsel
Copy link
Contributor Author

Shnatsel commented Apr 4, 2020

I have also confirmed that this particular path is exercised by my test code by inserting panic!() in the function and observing that my test code does indeed panic.

@Shnatsel
Copy link
Contributor Author

Shnatsel commented Apr 4, 2020

Hmm, behavior changed, so I must have been looking at the wrong function.

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.

1 participant