Implemente overflowing_sh* with new unchecked_sh* intrinsics #40521
Conversation
|
r? @aturon (rust_highfive has picked a reviewer for you, use r? to override) |
src/librustc_trans/intrinsic.rs
Outdated
| @@ -311,6 +311,8 @@ pub fn trans_intrinsic_call<'a, 'tcx>(bcx: &Builder<'a, 'tcx>, | |||
| } else { | |||
| bcx.urem(llargs[0], llargs[1]) | |||
| }, | |||
| "unchecked_shl" => bcx.shl(llargs[0], llargs[1]), | |||
| "unchecked_shr" => bcx.lshr(llargs[0], llargs[1]), | |||
There was a problem hiding this comment.
This should be either ashr when types are signed or lshr when types are unsigned.
See for example the rustc_trans::common::build_unchecked_rshift (you can’t reuse it sadly, because it tries to ensure the validity of RHS).
src/libcore/num/mod.rs
Outdated
| #[cfg(not(stage0))] | ||
| pub fn overflowing_shl(self, rhs: u32) -> (Self, bool) { | ||
| (unsafe { | ||
| intrinsics::unchecked_shl(self, (rhs & ($BITS - 1)) as $SelfT) |
There was a problem hiding this comment.
Since you’re already working around this code, it would be nice if you used this to implement wrapping_shl and used wrapping_shl to implement overflowing_shl instead of the inverse that we have today.
src/libcompiler_builtins/lib.rs
Outdated
| @@ -83,8 +83,8 @@ pub mod reimpls { | |||
| macro_rules! lshr { | |||
| ($a: expr, $b: expr, $ty:ty) => {{ | |||
| let (a, b) = ($a, $b); | |||
| let bits = (::core::mem::size_of::<$ty>() * 8) as $ty; | |||
| let half_bits = bits >> 1; | |||
| let bits = ::core::mem::size_of::<$ty>().overflowing_mul(8) as $ty; | |||
|
r=me |
|
@bors r=nagisa delegate=nagisa |
|
✌️ @nagisa can now approve this pull request |
|
📌 Commit ceccedd has been approved by |
|
I missed one of the Maybe leave this PR until tomorrow, I'll have a full unoptimized ARM build done by then to verify that actually all occurrences of |
|
Good news! (The travis failure is about the mac bots being... broken.) Do you want me to squash the commits? |
|
Sure. |
Also update some 128 bit builtins to be panic-free without relying on the const evaluator.
I assume this was regarding the squash? In that case: Done. |
|
@bors r+ |
|
📌 Commit cc23d17 has been approved by |
Implemente overflowing_sh* with new unchecked_sh* intrinsics Also update some 128 bit builtins to not rely on the constant evaluator to avoid checked operations. Fixes rust-lang#40508. cc @nagisa, @alexcrichton Note: I still have a build running to see if the 128 bit changes worked (unoptimized builds take *forever* to compile), however at least the overflowing builtins no longer reference `core::panicking::panic`.
Implemente overflowing_sh* with new unchecked_sh* intrinsics Also update some 128 bit builtins to not rely on the constant evaluator to avoid checked operations. Fixes rust-lang#40508. cc @nagisa, @alexcrichton Note: I still have a build running to see if the 128 bit changes worked (unoptimized builds take *forever* to compile), however at least the overflowing builtins no longer reference `core::panicking::panic`.
Implemente overflowing_sh* with new unchecked_sh* intrinsics Also update some 128 bit builtins to not rely on the constant evaluator to avoid checked operations. Fixes rust-lang#40508. cc @nagisa, @alexcrichton Note: I still have a build running to see if the 128 bit changes worked (unoptimized builds take *forever* to compile), however at least the overflowing builtins no longer reference `core::panicking::panic`.
|
@bors: r- This is likely the cause of this failure, I think maybe the methods just need to be |
|
I have added the additional inline attributes and verified that this fixes the issue for |
|
@bors: r+ |
|
📌 Commit e16d286 has been approved by |
Implemente overflowing_sh* with new unchecked_sh* intrinsics Also update some 128 bit builtins to not rely on the constant evaluator to avoid checked operations. Fixes rust-lang#40508. cc @nagisa, @alexcrichton Note: I still have a build running to see if the 128 bit changes worked (unoptimized builds take *forever* to compile), however at least the overflowing builtins no longer reference `core::panicking::panic`.
Implemente overflowing_sh* with new unchecked_sh* intrinsics Also update some 128 bit builtins to not rely on the constant evaluator to avoid checked operations. Fixes rust-lang#40508. cc @nagisa, @alexcrichton Note: I still have a build running to see if the 128 bit changes worked (unoptimized builds take *forever* to compile), however at least the overflowing builtins no longer reference `core::panicking::panic`.
Implemente overflowing_sh* with new unchecked_sh* intrinsics Also update some 128 bit builtins to not rely on the constant evaluator to avoid checked operations. Fixes rust-lang#40508. cc @nagisa, @alexcrichton Note: I still have a build running to see if the 128 bit changes worked (unoptimized builds take *forever* to compile), however at least the overflowing builtins no longer reference `core::panicking::panic`.
Also update some 128 bit builtins to not rely on the constant evaluator to avoid checked operations.
Fixes #40508.
cc @nagisa, @alexcrichton
Note: I still have a build running to see if the 128 bit changes worked (unoptimized builds take forever to compile), however at least the overflowing builtins no longer reference
core::panicking::panic.