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

add vcvt, vcvta, vcvtn, vcvtm, vcvtp neon instructions #1084

Merged
merged 5 commits into from
Mar 16, 2021

Conversation

SparrowLii
Copy link
Member

@SparrowLii SparrowLii commented Mar 16, 2021

Add vcvt, vcvta, vcvtn, vcvtm, vcvtp neon instructions(Floating-point convert to fixed-point with different rounding methods).

@rust-highfive
Copy link

r? @Amanieu

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

name = vcvt
double-suffixes
fn = simd_cast
a = -1.0, 2.0, -3.0, 4.0
Copy link
Contributor

Choose a reason for hiding this comment

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

Wouldn't it make more sense to use .5 and co. to actually test the rounding (for all of these instructions)?

Copy link
Member Author

@SparrowLii SparrowLii Mar 16, 2021

Choose a reason for hiding this comment

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

You are right, but I don’t have a good way to test the results. How can we simulate rounding towards to even?

Copy link
Contributor

@CryZe CryZe Mar 16, 2021

Choose a reason for hiding this comment

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

e.g.:

a = -1.5, 2.5, -3.5, 4.5
validate -2, 2, -4, 4

I'm confused what the problem is?

Copy link
Member Author

@SparrowLii SparrowLii Mar 16, 2021

Choose a reason for hiding this comment

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

Sorry I forgot that this is rounding to an integer. I will correct accordingly. Thanks!

Comment on lines 1945 to 1949
#[inline]
#[target_feature(enable = "neon")]
#[cfg_attr(target_arch = "arm", target_feature(enable = "v7"))]
#[cfg_attr(all(test, target_arch = "arm"), assert_instr(vcvt))]
pub unsafe fn vcvt_s32_f32(a: float32x2_t) -> int32x2_t {
Copy link
Contributor

Choose a reason for hiding this comment

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

It doesn't seem like there's any assert_instr for aarch64 for these?!

Copy link
Member Author

Choose a reason for hiding this comment

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

There are different implementations for arm and aarch64. You can use https://godbolt.org/ to verify

Copy link
Contributor

@CryZe CryZe Mar 16, 2021

Choose a reason for hiding this comment

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

Well it's fcvtzs and co., so idk why those aren't being asserted for here. https://rust.godbolt.org/z/Evce1T

Copy link
Member Author

Choose a reason for hiding this comment

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

I think you are right, thanks a lot

@Amanieu Amanieu merged commit 5a2c1f8 into rust-lang:master Mar 16, 2021
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.

4 participants