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

Complete the remaining neon instructions #1230

Merged
merged 3 commits into from
Oct 21, 2021
Merged

Conversation

SparrowLii
Copy link
Member

@SparrowLii SparrowLii commented Oct 13, 2021

This PR completes the remaining neon instructions, except for the following instructions:

  1. The following instructions are only available in aarch64 now, because the corresponding target_feature cannot be found in the available features of arm:
    vcadd_rotvcmlavdot

  2. The feature i8mm is not valid:
    vmmlavusmmla: https://rust.godbolt.org/z/8GbKW5ef4

  3. LLVM ERROR(Can be reproduced in godbolt):
    vsm4e: https://rust.godbolt.org/z/xhT1xvGTP

  4. LLVM ERROR(Normal in gotbolt, but LLVM ERROR: Cannot select: intrinsic raises at runtime)
    vsudotvusdot: https://rust.godbolt.org/z/aMnEvab3n
    vqshlu: https://rust.godbolt.org/z/hvGhrhdMT

  5. Not implmented in LLVM and cannot be implemented manually:
    vmull_p64(for arm)、vsm3vrax1q_u64vxarq_u64vrnd32vrnd64vsha512

Update #148

@rust-highfive
Copy link

r? @Amanieu

(rust-highfive has picked a reviewer for you, use r? to override)

@SparrowLii SparrowLii force-pushed the vcmla branch 2 times, most recently from 4a75f6b to 818e547 Compare October 13, 2021 08:03
@SparrowLii
Copy link
Member Author

SparrowLii commented Oct 13, 2021

There seems to be some problems with the CI of aarch64-apple-darwin. Could get some help?

@hkratz
Copy link
Contributor

hkratz commented Oct 13, 2021

There seems to be some problems with the CI of aarch64-apple-darwin. Could get some help?

#1231.

@Amanieu
Copy link
Member

Amanieu commented Oct 14, 2021

#1231 is now merged, can you rebase?

@Amanieu
Copy link
Member

Amanieu commented Oct 14, 2021

Does this mean that we now have 100% coverage of the neon intrinsics? Is there anything still missing?

@pthariensflame
Copy link
Contributor

Does this mean that we now have 100% coverage of the neon intrinsics? Is there anything still missing?

f16 and bf16 stuff are missing, I assume waiting on Rust core support for it. Not sure what else might be.

@SparrowLii
Copy link
Member Author

SparrowLii commented Oct 15, 2021

Does this mean that we now have 100% coverage of the neon intrinsics? Is there anything still missing?

Except for the following instructions that I cannot complete the implement by myself:

  1. The following instructions are only available in aarch64 now, because the corresponding target_feature cannot be found in the available features of arm:
    vcadd_rotvcmlavdot

  2. The feature i8mm is not valid:
    vmmlavusmmla: https://rust.godbolt.org/z/8GbKW5ef4

  3. LLVM ERROR(Can be reproduced in godbolt):
    vsm4e: https://rust.godbolt.org/z/xhT1xvGTP

  4. LLVM ERROR(Normal in gotbolt, but LLVM ERROR: Cannot select: intrinsic raises at runtime)
    vsudotvusdot: https://rust.godbolt.org/z/aMnEvab3n
    vqshlu: https://rust.godbolt.org/z/hvGhrhdMT

  5. Not implmented in LLVM and cannot be implemented manually:
    vmull_p64(for arm)、vsm3vrax1q_u64vxarq_u64vrnd32vrnd64vsha512

I hope to get some help, or decide whether to not implment them at current.

@SparrowLii SparrowLii reopened this Oct 15, 2021
@SparrowLii SparrowLii force-pushed the vcmla branch 3 times, most recently from 7083b3e to 2873d80 Compare October 15, 2021 03:02
@SparrowLii
Copy link
Member Author

SparrowLii commented Oct 18, 2021

@Amanieu vcadd_rot, vcmla, vdot need to use fcma, dorprod target features on aarch64. Do you know what corresponding target features should be used on ARM?

@Amanieu
Copy link
Member

Amanieu commented Oct 19, 2021

LGTM! Could you update the intrinsic tests (which compare with the C version) to enable all the newly added intrinsics. You just need to edit the first column of crates/intrinsic-test/neon-intrinsics.csv.

@SparrowLii
Copy link
Member Author

SparrowLii commented Oct 20, 2021

It seems that the intrinsics test tool needs some updates to support multi-vector (such as float32x2x2_t).
@JamieCunliffe Could you help update it? I will try to see if I can fix it, too.

@JamieCunliffe
Copy link
Contributor

I have actually made that change for the types, and a few other changes to the tool so that it works with the intrinsic's that have constraints on them. I plan to open a pull request by the end of the week for that.

I ran this pull request the other day with those changes and there wasn't any differences.

@Amanieu To do the constraints I have had to change the data source, so if you want to merge this when I open the pull request for my changes I can add these to that list.

@Amanieu
Copy link
Member

Amanieu commented Oct 20, 2021

OK, let's revert the intrinsic checker changes in this PR for now so we can merge this.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants