rust-num / num-traits

Numeric traits for generic mathematics in Rust
Apache License 2.0
732 stars 135 forks source link

PrimInt: add reverse_bits() method #202

Closed Xiretza closed 3 years ago

cuviper commented 3 years ago

Unfortunately, it's a breaking change to add a new trait method without a default implementation, because we haven't prevented downstream crates from implementing this trait, and they would fail without this new addition.

Your implementation also requires Rust 1.37.

Xiretza commented 3 years ago

I see, that's unforunate. Are there any plans for a new major version (or, since it's still at 0.x, any new version that includes breaking changes)?

tspiteri commented 3 years ago

It is possible to write a default implementation of reverse_bits which only depends on PrimInt methods. For example this works for me and it also gives me similar assembly on x86_64 to the new primitive reverse_bits methods (the only difference I could spot is in instruction order).

fn one_per_byte<P: PrimInt>() -> P {
    // i8, u8: return 0x01
    // i16, u16: return 0x0101 = (0x01 << 8) | 0x01
    // i32, u32: return 0x01010101 = (0x0101 << 16) | 0x0101
    // ...
    let mut ret = P::one();
    let mut shift = 8;
    let mut b = ret.count_zeros() >> 3;
    while b != 0 {
        ret = (ret << shift) | ret;
        shift <<= 1;
        b >>= 1;
    }
    ret
}
fn reverse_bits<P: PrimInt>(i: P) -> P {
    let rep_01: P = one_per_byte();
    let rep_03 = (rep_01 << 1) | rep_01;
    let rep_05 = (rep_01 << 2) | rep_01;
    let rep_0f = (rep_03 << 2) | rep_03;
    let rep_33 = (rep_03 << 4) | rep_03;
    let rep_55 = (rep_05 << 4) | rep_05;

    // code above only used to determine rep_0f, rep_33, rep_55;
    // optimizer should be able to do it in compile time
    let mut ret = i.swap_bytes();
    ret = ((ret & rep_0f) << 4) | ((ret >> 4) & rep_0f);
    ret = ((ret & rep_33) << 2) | ((ret >> 2) & rep_33);
    ret = ((ret & rep_55) << 1) | ((ret >> 1) & rep_55);
    ret
}

(For rustc versions supporting reverse_bits, a method should still be provided in primitive implementations using the standard method, to potentially avoid code duplication when dependencies use both trait method and inherent method.)

Xiretza commented 3 years ago

I added @tspiteri's fallback behind an autocfg gate, but I haven't been able to confirm whether it actually works as designed - I can't find any traces of the fallback method in the output binary (with stable rustc) even if I delete the method in the trait implementation entirely.

Xiretza commented 3 years ago

Sorry for the noise, I kept running into CI issues that I didn't have locally.

cuviper commented 3 years ago

Thanks!

bors r+

bors[bot] commented 3 years ago

Build succeeded: