stdarch icon indicating copy to clipboard operation
stdarch copied to clipboard

Flavorful return types like "uint8x16x2_t"

Open alexcrichton opened this issue 7 years ago • 8 comments

Some intrinsics on NEON like vtrnq_u8 have a nonstandard return type like uint8x16x2_t defined as:

typedef struct uint8x16x2_t {
  uint8x16_t val[2];
} uint8x16x2_t;

How should we best represent this in Rust? A few possible options are:

  1. unsafe fn vtrnq_u8(a: uint8x16_t, b: uint8x16_t) -> uint8x16x2_t; - aka we define the struct w/ pub fields in Rust like with the libc crate
  2. unsafe fn vtrnq_u8(a: uint8x16_t, b: uint8x16_t) -> [uint8x16_t; 2]
  3. unsafe fn vtrnq_u8(a: uint8x16_t, b: uint8x16_t) -> (uint8x16_t; uint8x16_6)

I'd sort of lean towards the last one, but it also depends on how "flavorful" the return types get!

alexcrichton avatar Feb 05 '18 15:02 alexcrichton

I think the last option is the ideal one.

A quick look at clang's arm_neon.h suggests that

  1. The multiple vectors wrapped by a returned C struct are always mutually of the same type. (Always declared as a C array within the struct.)
  2. The possible array lengths are 2, 3 and 4.

hsivonen avatar Feb 05 '18 15:02 hsivonen

Ok yeah that makes sense to me! In that case I think it's ok for us to take a bit of liberty here and return tuples instead of strictly the structs specified. Although I'd also be ok with fixed size arrays as well!

alexcrichton avatar Feb 05 '18 15:02 alexcrichton

I would do what the spec does, that is, option 1.

gnzlbg avatar Feb 05 '18 18:02 gnzlbg

Unless there is a good reason not to. Is it? Like if we can make a tuple, we can make a struct right? Or does this tuple need to be magical?

gnzlbg avatar Feb 05 '18 18:02 gnzlbg

We'd sort of be defining a new type either way and adding logic to ensure that it matches with the spec no matter what we choose. In that sense I think it's an equal amount of work/verification logic for any choice here, so I figured we'd go with the most ergonomic to consume one.

alexcrichton avatar Feb 05 '18 19:02 alexcrichton

A tuple is nicer for Rust destructuring. (Also, since the ARM stuff is already strongly-typed, uint8x16_t could be u8x16 from portable SIMD.)

hsivonen avatar Feb 05 '18 19:02 hsivonen

A tuple is nicer for Rust destructuring.

With #[feature(slice_patterns)] you get:

#![feature(slice_patterns)]

fn main() {
    let a = [2, 3, 4];
    let [c, d, e]: [i32; 3] = a;
    assert_eq!(c, 2);
    assert_eq!(d, 3);
    assert_eq!(e, 4);
}

For what we know, slice_patterns might be stable when we decide to stabilize the ARM API. Right now everything is unstable anyways, so it doesn't really matter, but maybe we can choose tuple now and re-evaluate array later? In any case, writing a wrapper that does a different mapping is trivial.

C uses a struct because it cannot return arrays by value ...

gnzlbg avatar Feb 05 '18 21:02 gnzlbg

I think sticking to the vendor-defined pattern is the right way forward for this. This gives us the "this is how it's defined upstream" argument when attempting to stabilize. We've also done a similar thing with the Intel intrinsics. We can leave rustification to downstream crates.

AdamNiederer avatar Feb 06 '18 19:02 AdamNiederer