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.

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

Support repr(simd) on ADTs containing a single array field #78863

Merged
merged 10 commits into from
Nov 29, 2020
Merged

Support repr(simd) on ADTs containing a single array field #78863

merged 10 commits into from
Nov 29, 2020

Conversation

KodrAus
Copy link
Contributor

@KodrAus KodrAus commented Nov 8, 2020

This is a squash and rebase of @gnzlbg's #63531

I've never actually written code in the compiler before so just fumbled my way around until it would build 😅

I imagine there'll be some work we need to do in rustc_codegen_cranelift too for this now, but might need some input from @bjorn3 to know what that is.

cc @rust-lang/project-portable-simd


This PR allows using #[repr(simd)] on ADTs containing a single array field:

 #[repr(simd)] struct S0([f32; 4]);
 #[repr(simd)] struct S1<const N: usize>([f32; N]);
 #[repr(simd)] struct S2<T, const N: usize>([T; N]);

This should allow experimenting with portable packed SIMD abstractions on nightly that make use of const generics.

@rust-highfive
Copy link
Collaborator

r? @oli-obk

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

@rust-highfive rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Nov 8, 2020
This PR allows using `#[repr(simd)]` on ADTs containing a
single array field:

```rust
 #[repr(simd)] struct S0([f32; 4]);
 #[repr(simd)] struct S1<const N: usize>([f32; N]);
 #[repr(simd)] struct S2<T, const N: usize>([T; N]);
```

This should allow experimenting with portable packed SIMD
abstractions on nightly that make use of const generics.
@bjorn3
Copy link
Member

bjorn3 commented Nov 8, 2020

I think clif_vector_type will already work fine: https://github.com/bjorn3/rustc_codegen_cranelift/blob/11dd005a88eeb6129c8c19ab02cba608d3b7d682/src/intrinsics/mod.rs#L195

@KodrAus
Copy link
Contributor Author

KodrAus commented Nov 14, 2020

I've added a compile-fail test for a #[repr(simd)] type that depends on a generic parameter, along with a note inline pointing to here and to that test.

@oli-obk
Copy link
Contributor

oli-obk commented Nov 15, 2020

@bors r+

@bors
Copy link
Contributor

bors commented Nov 15, 2020

📌 Commit e217fc4 has been approved by oli-obk

@bors bors added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Nov 15, 2020
Dylan-DPC-zz pushed a commit to Dylan-DPC-zz/rust that referenced this pull request Nov 17, 2020
Support repr(simd) on ADTs containing a single array field

This is a squash and rebase of `@gnzlbg's` rust-lang#63531

I've never actually written code in the compiler before so just fumbled my way around until it would build 😅

I imagine there'll be some work we need to do in `rustc_codegen_cranelift` too for this now, but might need some input from `@bjorn3` to know what that is.

cc `@rust-lang/project-portable-simd`

-----

This PR allows using `#[repr(simd)]` on ADTs containing a single array field:

```rust
 #[repr(simd)] struct S0([f32; 4]);
 #[repr(simd)] struct S1<const N: usize>([f32; N]);
 #[repr(simd)] struct S2<T, const N: usize>([T; N]);
```

This should allow experimenting with portable packed SIMD abstractions on nightly that make use of const generics.
@m-ou-se
Copy link
Member

m-ou-se commented Nov 17, 2020

@bors bors added S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. and removed S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. labels Nov 17, 2020
Comment on lines 41 to 42
// CHECK: call void @llvm.memcpy.p0i8.p0i8.i64(i8* {{.*}} %{{[0-9]+}}, i8* {{.*}} %3, i64 16, i1 false)
// CHECK: call void @llvm.memcpy.p0i8.p0i8.i64(i8* {{.*}} %{{[0-9]+}}, i8* {{.*}} %6, i64 16, i1 false)
Copy link
Member

@workingjubilee workingjubilee Nov 17, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

14:  call void @llvm.memcpy.p0i8.p0i8.i32(i8* align 4 %2, i8* align 4 %3, i32 16, i1 false)
check:25'0     ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
check:25'1      ?                                         possible intended match

On the i586 platform, it uses i32 here instead, but this test passes on the 64-bit platforms. If this is a pointer size then it needs to be allowed to vary based on platform, I suppose? Or to exclude platforms where it won't work.

Copy link
Contributor Author

@KodrAus KodrAus Nov 18, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ahh, since these are regex matches we should be able to do something like:

call void @llvm.memcpy.p0i8.p0i8.{{i32|i64}}(i8* {{.*}} %{{[0-9]+}}, i8* {{.*}} %3, {{i32|i64}} 16, i1 false)

I’ll give it a try!

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'd guess you also want the i16 version for 16-bit targets such as msp430 or avr

@KodrAus
Copy link
Contributor Author

KodrAus commented Nov 23, 2020

@bors try

@bors
Copy link
Contributor

bors commented Nov 23, 2020

💔 Test failed - checks-actions

@bors bors added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. and removed S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. labels Nov 23, 2020
@KodrAus
Copy link
Contributor Author

KodrAus commented Nov 29, 2020

@bors r=oli-obk rollup=never

@bors
Copy link
Contributor

bors commented Nov 29, 2020

📌 Commit 90255c8 has been approved by oli-obk

@bors bors added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Nov 29, 2020
@bors
Copy link
Contributor

bors commented Nov 29, 2020

⌛ Testing commit 90255c8 with merge 592a105933bbca87806f71c11640a71131c7e949...

@bors
Copy link
Contributor

bors commented Nov 29, 2020

💔 Test failed - checks-actions

@bors bors added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. and removed S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. labels Nov 29, 2020
@KodrAus
Copy link
Contributor Author

KodrAus commented Nov 29, 2020

@bors r=oli-obk rollup=never

@bors
Copy link
Contributor

bors commented Nov 29, 2020

📌 Commit 354c7d0 has been approved by oli-obk

@bors bors added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Nov 29, 2020
@bors
Copy link
Contributor

bors commented Nov 29, 2020

⌛ Testing commit 354c7d0 with merge 760430e...

@bors
Copy link
Contributor

bors commented Nov 29, 2020

☀️ Test successful - checks-actions
Approved by: oli-obk
Pushing 760430e to master...

@bors bors added the merged-by-bors This PR was explicitly merged by bors. label Nov 29, 2020
@bors bors merged commit 760430e into rust-lang:master Nov 29, 2020
@rustbot rustbot added this to the 1.50.0 milestone Nov 29, 2020
@KodrAus KodrAus deleted the feat/simd-array branch November 29, 2020 23:45
bjorn3 added a commit to rust-lang/rustc_codegen_cranelift that referenced this pull request Dec 3, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
merged-by-bors This PR was explicitly merged by bors. S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

10 participants