Implemente overflowing_sh* with new unchecked_sh* intrinsics #40521

TimNN · 2017-03-14T17:44:03Z

Also update some 128 bit builtins to not rely on the constant evaluator to avoid checked operations.

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.

rust-highfive · 2017-03-14T17:44:06Z

r? @aturon

(rust_highfive has picked a reviewer for you, use r? to override)

nagisa · 2017-03-14T17:57:35Z

src/librustc_trans/intrinsic.rs

@@ -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]),


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).

nagisa · 2017-03-14T18:08:20Z

src/libcore/num/mod.rs

+        #[cfg(not(stage0))]
+        pub fn overflowing_shl(self, rhs: u32) -> (Self, bool) {
+            (unsafe {
+                intrinsics::unchecked_shl(self, (rhs & ($BITS - 1)) as $SelfT)


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.

nagisa · 2017-03-14T18:10:28Z

src/libcompiler_builtins/lib.rs

@@ -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;


s/overflowing/wrapping/?

nagisa · 2017-03-14T19:23:39Z

r=me

alexcrichton · 2017-03-14T19:35:11Z

@bors r=nagisa delegate=nagisa

bors · 2017-03-14T19:35:13Z

✌️ @nagisa can now approve this pull request

bors · 2017-03-14T19:35:13Z

📌 Commit ceccedd has been approved by nagisa

TimNN · 2017-03-14T20:28:52Z

I missed one of the i128 macros :(

Maybe leave this PR until tomorrow, I'll have a full unoptimized ARM build done by then to verify that actually all occurrences of panic are gone.

TimNN · 2017-03-15T05:48:43Z

Good news! readelf no longer shows any references to panic. However run-pass/issue-28950.rs hanges somewhere, but I think that is a separate issue, so this is probably good to go to bors.

(The travis failure is about the mac bots being... broken.)

Do you want me to squash the commits?

nagisa · 2017-03-15T05:54:43Z

Sure.

Also update some 128 bit builtins to be panic-free without relying on the const evaluator.

TimNN · 2017-03-15T06:01:51Z

Sure.

I assume this was regarding the squash? In that case: Done.

nagisa · 2017-03-15T06:02:57Z

@bors r+

bors · 2017-03-15T06:02:58Z

📌 Commit cc23d17 has been approved by nagisa

@nagisa

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`.

@nagisa

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`.

@nagisa

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`.

alexcrichton · 2017-03-18T03:51:54Z

@bors: r-

This is likely the cause of this failure, I think maybe the methods just need to be #[inline]?

TimNN · 2017-03-18T13:54:47Z

I have added the additional inline attributes and verified that this fixes the issue for i686-gnu.

alexcrichton · 2017-03-18T16:23:31Z

@bors: r+

bors · 2017-03-18T16:23:32Z

📌 Commit e16d286 has been approved by alexcrichton

@nagisa

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`.

Rollup of 22 pull requests - Successful merges: #40241, #40346, #40348, #40377, #40398, #40409, #40441, #40445, #40509, #40521, #40523, #40532, #40538, #40564, #40581, #40583, #40588, #40589, #40590, #40603, #40611, #40621 - Failed merges: #40501, #40541

@nagisa

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`.

@nagisa

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`.

Rollup of 9 pull requests - Successful merges: #40241, #40281, #40398, #40521, #40532, #40554, #40566, #40581, #40587 - Failed merges:

rust-highfive assigned aturon Mar 14, 2017

nagisa reviewed Mar 14, 2017

View reviewed changes

TimNN force-pushed the panic-free-shift branch from bb3202e to e791a46 Compare March 14, 2017 19:21

make shift builtins panic-free with new unchecked_sh* intrinsics

cc23d17

Also update some 128 bit builtins to be panic-free without relying on the const evaluator.

TimNN force-pushed the panic-free-shift branch from 4e75e9c to cc23d17 Compare March 15, 2017 05:59

est31 mentioned this pull request Mar 16, 2017

Unoptimised builds of compiler_builtins on ARM have references to core::panicking::panic #40508

Closed

frewsxcv mentioned this pull request Mar 17, 2017

Rollup of 15 pull requests #40615

Closed

frewsxcv mentioned this pull request Mar 17, 2017

Rollup of 16 pull requests #40616

Closed

frewsxcv mentioned this pull request Mar 17, 2017

Rollup of 12 pull requests #40618

Closed

add inline attributes to stage 0 methods

e16d286

arielb1 mentioned this pull request Mar 18, 2017

Rollup of 22 pull requests #40644

Closed

arielb1 mentioned this pull request Mar 19, 2017

Rollup of 21 pull requests #40645

Closed

frewsxcv mentioned this pull request Mar 20, 2017

Rollup of 9 pull requests #40659

Merged

bors added a commit that referenced this pull request Mar 20, 2017

Auto merge of #40659 - frewsxcv:rollup, r=frewsxcv

244f893

Rollup of 9 pull requests - Successful merges: #40241, #40281, #40398, #40521, #40532, #40554, #40566, #40581, #40587 - Failed merges:

bors merged commit e16d286 into rust-lang:master Mar 20, 2017

TimNN deleted the panic-free-shift branch April 7, 2017 20:14

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Implemente overflowing_sh* with new unchecked_sh* intrinsics #40521

Implemente overflowing_sh* with new unchecked_sh* intrinsics #40521

TimNN commented Mar 14, 2017

rust-highfive commented Mar 14, 2017

nagisa Mar 14, 2017

nagisa Mar 14, 2017

nagisa Mar 14, 2017

nagisa commented Mar 14, 2017

alexcrichton commented Mar 14, 2017

bors commented Mar 14, 2017

bors commented Mar 14, 2017

TimNN commented Mar 14, 2017

TimNN commented Mar 15, 2017

nagisa commented Mar 15, 2017

TimNN commented Mar 15, 2017 •

edited

Loading

nagisa commented Mar 15, 2017

bors commented Mar 15, 2017

alexcrichton commented Mar 18, 2017

TimNN commented Mar 18, 2017

alexcrichton commented Mar 18, 2017

bors commented Mar 18, 2017

Implemente overflowing_sh* with new unchecked_sh* intrinsics #40521

Implemente overflowing_sh* with new unchecked_sh* intrinsics #40521

Conversation

TimNN commented Mar 14, 2017

rust-highfive commented Mar 14, 2017

nagisa Mar 14, 2017

Choose a reason for hiding this comment

nagisa Mar 14, 2017

Choose a reason for hiding this comment

nagisa Mar 14, 2017

Choose a reason for hiding this comment

nagisa commented Mar 14, 2017

alexcrichton commented Mar 14, 2017

bors commented Mar 14, 2017

bors commented Mar 14, 2017

TimNN commented Mar 14, 2017

TimNN commented Mar 15, 2017

nagisa commented Mar 15, 2017

TimNN commented Mar 15, 2017 • edited Loading

nagisa commented Mar 15, 2017

bors commented Mar 15, 2017

alexcrichton commented Mar 18, 2017

TimNN commented Mar 18, 2017

alexcrichton commented Mar 18, 2017

bors commented Mar 18, 2017

TimNN commented Mar 15, 2017 •

edited

Loading