-
Notifications
You must be signed in to change notification settings - Fork 13k
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
Add nowrap_{add|sub|mul|neg} intrinsics #52205
Conversation
LLVM cannot deduce nsw/nuw for things like Range::next today, so add these so that my next PR can use them to improve what Range tells LLVM.
r? @varkor (rust_highfive has picked a reviewer for you, use r? to override) |
#[no_mangle] | ||
pub unsafe fn nowrap_neg_unsigned(a: u32) -> u32 { | ||
// CHECK: ret i32 0 | ||
nowrap_neg(a) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, this one is useless, since a non-overflowing negation of an unsigned type is always zero, but it was easier to support than to not support.
@bors r+ I see no reason not to do this, and if it ends up not helping we can always remove them |
📌 Commit 9135ec5 has been approved by |
@bors r- There’s no necessity to land intrinsics before a PR with an actual use case can be made (such as e.g. Please implement actual changes against |
Also,
is a very slippery slope, simply because it is way easier to forget and not remove something that ended up not being used (or not useful for some immediate use case). It is way easier to land everything together or not land stuff at all :) |
Closing while I do that. (Aside: cool to see that bors knows that an r- means the author probably needs to do something) |
Here is an use case for these intrinsics: rust-lang/rfcs#2508 Basically, there is no easy way to generate LLVM-IR with So I'd argue that these intrinsics are at least useful for hacking on rustc/LLVM, debugging performance issues, etc. Also, I'd like to be able to play with intrinsics that also add both, the
|
LLVM cannot deduce nsw/nuw for things like
Range::next
today, so add these so that my next PR can use them to improve whatRange
tells LLVM.(This adds the ones that
rustc_codegen_llvm::builder::Builder
andrustc_llvm::ffi
currently support, which is why it hasnowrap_neg
but notnowrap_shl
.)tag guess: T-compiler