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

Division by nonzero #54868

Closed
leonardo-m opened this issue Oct 6, 2018 · 2 comments
Closed

Division by nonzero #54868

leonardo-m opened this issue Oct 6, 2018 · 2 comments
Labels
I-slow Issue: Problems and improvements with respect to performance of generated code. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.

Comments

@leonardo-m
Copy link

Rustc seems unable to use the fact that NonZeroU64 isn't zero:

use std::num::NonZeroU64;

#[inline(never)]
fn foo(x: u64, y: NonZeroU64) -> u64 {
    x / y.get()
}

Gives the asm (with -O):

_ZN4test3foo17h70af812d10d8d714E:
    subq    $40, %rsp
    testq   %rdx, %rdx
    je  .LBB9_2
    movq    %rdx, %r8
    xorl    %edx, %edx
    movq    %rcx, %rax
    divq    %r8
    addq    $40, %rsp
    retq
.LBB9_2:
    leaq    .L__unnamed_6(%rip), %rcx
    callq   _ZN4core9panicking5panic17h6a1e45881f6169d7E
    ud2

Adding an assume() solves the problem:

#![feature(core_intrinsics)]
use std::num::NonZeroU64;
use std::intrinsics::assume;

#[inline(never)]
fn foo2(x: u64, y: NonZeroU64) -> u64 {
    let y2 = y.get();
    unsafe { assume(y2 != 0); }
    x / y2
}


_ZN5test24foo217hf17fb494647eabb3E:
    movq    %rdx, %r8
    xorl    %edx, %edx
    movq    %rcx, %rax
    divq    %r8
    retq

Perhaps such assume() should be added at the end of NonZeroU64::get()?

@Centril Centril added I-slow Issue: Problems and improvements with respect to performance of generated code. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. labels Oct 6, 2018
tromey added a commit to tromey/rust that referenced this issue Oct 11, 2018
Change the `get` method on NonZero integer types to insert an
`assume`, so that LLVM can optimize better.

Closes rust-lang#54868
@scottmcm
Copy link
Member

Dup of #49572?

@leonardo-m
Copy link
Author

Yet, it seems a dupe

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
I-slow Issue: Problems and improvements with respect to performance of generated code. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.
Projects
None yet
Development

No branches or pull requests

4 participants