Adding __clzsi2

[Lokathor]

Hey all, these days LLVM does attempt to use __clzsi2 some of the time (despite what the README says), typically only with debug builds but sometimes in release mode.

So, here's an implementation of it. I attempted to make it correct for any pointer size all the way down to 16 (which is the minimum pointer size rust could ever support, AIUI). It's placed in the arm file because I need it for ARM and there didn't seem to be a clearer place to put it, but it can be moved.

@alexcrichton said that it can be hooked into the test suites and such, but I don't know how to do that, so this is just the start.

[alexcrichton]

Thanks!

This may be best suited in the various src/int/* modules perhaps? There you'll also find an intrinsics! macro which should help with all the attributes here

The tests are all generated in this file and the generation here can probably just look at the surrounding functions and match them.

[Lokathor]

Also, there are some build fails for GNU on windows, and they don't seem to be related to any file I changed. Is that normal?

[alexcrichton]

Ah yeah that's a bug on our CI, you can safely ignore them (sorry for the noise!)

[Lokathor]

@alexcrichton Alright, maybe I'm not understanding how the test suite works :(

What I really want to express is very simple:

#[test]
fn __clzsi2_test() {
  let mut i: usize = core::usize::MAX;
  while i > 0 {
    assert_eq!(__clzsi2(i) as u32, i.leading_zeros());
    i >>= 1;
  }
  // check 0 also
  i = 0;
  assert_eq!(__clzsi2(i) as u32, i.leading_zeros());
  // double check for bit patterns that aren't solid 1s
  i = 1;
  for _ in 0 .. 63 {
      assert_eq!(__clzsi2(i) as u32, i.leading_zeros());
      i <<= 2;
      i += 1;
  }
}

[alexcrichton]

@Lokathor I think that the errors on CI look legitimate, like it's a bug in the function or a bug in the test? You should be able to inline that test directly into the tests folder I think as well

[Lokathor]

Yeah the test is very bugged because it's not setup for usize and so I tried to badly fake it with u64. I was not aware that just putting a normal test into the tests folder was an option (in terms of allowed style).

I will do that and update later tonight.

[Lokathor]

@alexcrichton okay much better

[alexcrichton]

Ok great, thanks! Could this use the intrinsics! macro to define the intrinsic as well?

[Lokathor]

I can make that update.

Should any of the attributes be used? I've had success with the plain form, so my first guess is that we don't need any special attribute magic for this one.

[alexcrichton]

I'd probably stick to the plain form for now, the attributes can be added as necessary for various platforms. Most of them are just inherited for other weird intrinsics.

[Lokathor]

@alexcrichton So when I tried to wrap the function in the intrinsics! macro just now it started causing a lot of errors about "duplicate lang items" for the i128 and u128 operations. I do not know why, and I do not know how to solve such a thing, so unless the use of the macro is strictly required for the PR to be accepted then perhaps we better leave it off for now.

[alexcrichton]

The macro is ideally required yeah because it correctly handles conventions across platforms and mangling vs not, can you share the errors you were getting?

[Lokathor]

Seems like the 128-bit errors that my RLS was reporting are just the same false errors that appveyor gets? Travis seems fine with it all.

I'm good with things like this if you are.

[alexcrichton]

👍

[Lokathor]

Will this change go out with the next nightly? Or is there some other pace it uses?

[mati865]

It will be available once new version is released and Rust's Cargo.lock is updated.

[Lokathor]

So it might be quite a while then?

[mati865]

It's not up to me but it for sure won't be in next nightly.

[alexcrichton]

All that needs to happen is a publish and then an update to rust-lang/rust, @Lokathor if you want to push a version bump I can merge and publish and then you can update the lock file in rust-lang/rust

[Lokathor]

Uh, sure, I can do that first part really fast.

[Lokathor]

#271