Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

ssse3 _mm_abs_pi8 : Intrinsic has incorrect return type! #74

Closed
gwenn opened this issue Sep 29, 2017 · 11 comments
Closed

ssse3 _mm_abs_pi8 : Intrinsic has incorrect return type! #74

gwenn opened this issue Sep 29, 2017 · 11 comments

Comments

@gwenn
Copy link
Contributor

gwenn commented Sep 29, 2017

Try fn _mm_abs_pi8(a: i8x8) -> u8x8 patch or fn _mm_abs_pi8(a: i8x8) -> i8x8 to reproduce the error:

Intrinsic has incorrect return type!
<8 x i8> (<8 x i8>)* @llvm.x86.ssse3.pabs.b
LLVM ERROR: Broken function found, compilation aborted!
error: Could not compile `stdsimd`.
@BurntSushi
Copy link
Member

For context, LLVM seems to think this should return an MMX type, and I've had problems getting other intrinsics with similar types to work with Rust as well. I don't quite know how to debug it though.

@alexcrichton
Copy link
Member

According to the LLVM test for this there's a dedicated x86_mmx type for this which we're not currently using. I think this'll require compiler changes to use x86_mmx for sure?

@gwenn
Copy link
Contributor Author

gwenn commented Sep 30, 2017

@alexcrichton You mean rustc changes ? And then we can cast i8x8 to x86_mmx ?

@alexcrichton
Copy link
Member

I think the i8x8 type needs to be defined as x86_mmx on x86 targets similarly to how i16x8 is defined as <i16 x 8> as a type. Not sure why these are different at the LLVM layer, but something we'll have to take care of in rustc for sure.

@alexcrichton alexcrichton mentioned this issue Oct 18, 2017
Merged
@alexcrichton
Copy link
Member

I've opened up an upstream PR to add this type

nominolo added a commit to nominolo/stdsimd that referenced this issue Oct 22, 2017
@nominolo
Add single output _mm_cvt[t]ss_* variants
f4633aa 
The *_pi variants are currently blocked by
rust-lang#74
nominolo added a commit to nominolo/stdsimd that referenced this issue Oct 22, 2017
@nominolo
Add _mm_cvtsi*_ss
cd25347 
The _mm_cvtpi*_ps intrinsics are blocked by
rust-lang#74
@nominolo nominolo mentioned this issue Oct 22, 2017
Merged
alexcrichton pushed a commit that referenced this issue Oct 23, 2017
@nominolo @alexcrichton
Add some SSE _mm_cvt* instructions (#136)
2afb984 
* Add single output _mm_cvt[t]ss_* variants

The *_pi variants are currently blocked by
#74

* Add _mm_cvtsi*_ss

The _mm_cvtpi*_ps intrinsics are blocked by
#74

* Fix Linux builds

Also the si64 variants are only available on x86_64
alexcrichton added a commit to alexcrichton/rust that referenced this issue Nov 21, 2017
@alexcrichton
rustc: Add support for some more x86 SIMD ops
74dd1c2 
This commit adds compiler support for two basic operations needed for binding
SIMD on x86 platforms:

* First, a `nontemporal_store` intrinsic was added for the `_mm_stream_ps`, seen
  in rust-lang/stdarch#114. This was relatively straightforward and is
  quite similar to the volatile store intrinsic.

* Next, and much more intrusively, a new type to the backend was added. The
  `x86_mmx` type is used in LLVM for a 64-bit vector register and is used in
  various intrinsics like `_mm_abs_pi8` as seen in rust-lang/stdarch#74.
  This new type was added as a new layout option as well as having support added
  to the trans backend. The type is enabled with the `#[repr(x86_mmx)]`
  attribute which is intended to just be an implementation detail of SIMD in
  Rust.

I'm not 100% certain about how the `x86_mmx` type was added, so any extra eyes
or thoughts on that would be greatly appreciated!
bors added a commit to rust-lang/rust that referenced this issue Nov 22, 2017
@bors
Auto merge of #45367 - alexcrichton:simd-llvm-changes, r=eddyb
7eb90e4 
rustc: Add support for some more x86 SIMD ops

This commit adds compiler support for two basic operations needed for binding
SIMD on x86 platforms:

* First, a `nontemporal_store` intrinsic was added for the `_mm_stream_ps`, seen
  in rust-lang/stdarch#114. This was relatively straightforward and is
  quite similar to the volatile store intrinsic.

* Next, and much more intrusively, a new type to the backend was added. The
  `x86_mmx` type is used in LLVM for a 64-bit vector register and is used in
  various intrinsics like `_mm_abs_pi8` as seen in rust-lang/stdarch#74.
  This new type was added as a new layout option as well as having support added
  to the trans backend. The type is enabled with the `#[repr(x86_mmx)]`
  attribute which is intended to just be an implementation detail of SIMD in
  Rust.

I'm not 100% certain about how the `x86_mmx` type was added, so any extra eyes
or thoughts on that would be greatly appreciated!
alexcrichton added a commit to alexcrichton/rust that referenced this issue Nov 25, 2017
@alexcrichton
rustc: Add support for some more x86 SIMD ops
fe53a81 
This commit adds compiler support for two basic operations needed for binding
SIMD on x86 platforms:

* First, a `nontemporal_store` intrinsic was added for the `_mm_stream_ps`, seen
  in rust-lang/stdarch#114. This was relatively straightforward and is
  quite similar to the volatile store intrinsic.

* Next, and much more intrusively, a new type to the backend was added. The
  `x86_mmx` type is used in LLVM for a 64-bit vector register and is used in
  various intrinsics like `_mm_abs_pi8` as seen in rust-lang/stdarch#74.
  This new type was added as a new layout option as well as having support added
  to the trans backend. The type is enabled with the `#[repr(x86_mmx)]`
  attribute which is intended to just be an implementation detail of SIMD in
  Rust.

I'm not 100% certain about how the `x86_mmx` type was added, so any extra eyes
or thoughts on that would be greatly appreciated!
bors added a commit to rust-lang/rust that referenced this issue Nov 25, 2017
@bors
Auto merge of #45367 - alexcrichton:simd-llvm-changes, r=eddyb
128b40f 
rustc: Add support for some more x86 SIMD ops

This commit adds compiler support for two basic operations needed for binding
SIMD on x86 platforms:

* First, a `nontemporal_store` intrinsic was added for the `_mm_stream_ps`, seen
  in rust-lang/stdarch#114. This was relatively straightforward and is
  quite similar to the volatile store intrinsic.

* Next, and much more intrusively, a new type to the backend was added. The
  `x86_mmx` type is used in LLVM for a 64-bit vector register and is used in
  various intrinsics like `_mm_abs_pi8` as seen in rust-lang/stdarch#74.
  This new type was added as a new layout option as well as having support added
  to the trans backend. The type is enabled with the `#[repr(x86_mmx)]`
  attribute which is intended to just be an implementation detail of SIMD in
  Rust.

I'm not 100% certain about how the `x86_mmx` type was added, so any extra eyes
or thoughts on that would be greatly appreciated!
@alexcrichton
Copy link
Member

Ok! Took awhile but the upstream support is now landed and should be available for us to integrate with :)

@gwenn
Copy link
Contributor Author

gwenn commented Nov 26, 2017

@alexcrichton Sorry, but still the same error on my side

rustc 1.23.0-nightly (e97ba8328 2017-11-25)

@MaloJaffre
Copy link
Contributor

I think this landed in master, but not yet in nightly.

@gwenn
Copy link
Contributor Author

gwenn commented Nov 26, 2017

@MaloJaffre Thanks, I will try tomorrow.

@alexcrichton
Copy link
Member

@gwenn the types will need to look like:

#[simd]
struct i8x8(i64);

@gwenn
Copy link
Contributor Author

gwenn commented Nov 28, 2017

@alexcrichton Ok, it works.
But it seems that I am not the only one working on this: https://github.com/gnzlbg/stdsimd/tree/intr2
Maybe I should wait until the intr2 is merged ?
And @gnzlbg created/moved the x86_mmx/_m64 type in the i686 module.
Should ssse3 also be moved in i686 ?
Thanks.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
4 participants
@gwenn @alexcrichton @BurntSushi @MaloJaffre