3

Stabilize armv8 neon instruction set on aarch64 by SparrowLii · Pull Request #12...

 2 years ago
source link: https://github.com/rust-lang/stdarch/pull/1266
Go to the source link to view the article. You can view the picture content, updated content and better typesetting reading experience. If the link is broken, please click the button below to view the snapshot at that time.
neoserver,ios ssh client

@@ -13,6 +13,7 @@ use stdarch_test::assert_instr;

#[inline]

#[target_feature(enable = "crc")]

#[cfg_attr(test, assert_instr(crc32x))]

#[stable(feature = "neon_intrinsics", since = "1.59.0")]

pub unsafe fn __crc32d(crc: u32, data: u64) -> u32 {

The FCP only covers the NEON (SIMD) intrinsics, not crc intrinsics.

Copy link

Contributor

Author

@SparrowLii SparrowLii on Dec 11, 2021

OK. Unmarked it.

@@ -11,20 +11,23 @@ use stdarch_test::assert_instr;

/// Reverse the order of the bytes.

#[inline]

#[cfg_attr(test, assert_instr(rev))]

#[stable(feature = "neon_intrinsics", since = "1.59.0")]

pub unsafe fn _rev_u64(x: u64) -> u64 {

Same here, there intrinsics are not covered by the FCP.

Copy link

Contributor

Author

@SparrowLii SparrowLii on Dec 11, 2021

OK. Unmarked it.

@@ -71,7 +71,7 @@ pub const _TMFAILURE_TRIVIAL: u64 = 1 << 24;

/// [ARM TME Intrinsics](https://developer.arm.com/docs/101028/0010/transactional-memory-extension-tme-intrinsics).

#[inline]

#[target_feature(enable = "tme")]

#[cfg_attr(test, assert_instr(tstart))]

#[cfg_attr(test, assert_instr(nop))]

Why were these changed to nop? This is incorrect.

Copy link

Contributor

Author

@SparrowLii SparrowLii on Dec 11, 2021

The assert_instr of these instructions failed on my local aarch64. But since these instructions are not included in this stabilization, I think we can keep them unchanged.

It might have failed because the binutils version on your system is too old and objdump is not able to disassemble the instruction. We use a recent version of ubuntu in the docker image used in CI for this reason.

SparrowLii reacted with eyes emoji

@@ -7044,7 +7044,7 @@ a = 1.1, 1.9, -1.7, -2.3

validate 1.0, 2.0, -2.0, -2.0

target = frintts

aarch64 = frint32x

aarch64 = nop

Why is this changed to nop? This is incorrect.

Copy link

Contributor

Author

@SparrowLii SparrowLii on Dec 11, 2021

Same as above. Changes have been cancelled.

@@ -4811,7 +6687,7 @@ pub unsafe fn vpadalq_u32(a: uint64x2_t, b: uint32x4_t) -> uint64x2_t {

#[target_feature(enable = "neon")]

#[cfg_attr(target_arch = "arm", target_feature(enable = "v8"))]

#[cfg_attr(all(test, target_arch = "arm"), assert_instr(nop))]

#[cfg_attr(all(test, target_arch = "aarch64"), assert_instr(smmla))]

#[cfg_attr(all(test, target_arch = "aarch64"), assert_instr(nop))]

Why is this changed to nop? This is incorrect.

Also why is this function not marked as stable?

Copy link

Contributor

Author

@SparrowLii SparrowLii on Dec 11, 2021

edited

Same as above. And vmmla instructions require v8.6a architecture, so they are not included in this stabilization.

FPArmV8 => "fp-armv8,v8",

ArmV7 => "neon",

Vfp4 => "neon",

FPArmV8 => "neon",

Why did you change this? The previous target features were correct.

Copy link

Contributor

Author

@SparrowLii SparrowLii on Dec 11, 2021

There are two functions used to generate instructions: gen_arm and gen_aarch64. The part I changed is in gen_aarch64. The values of these targets do not affect the target_feature on aarch64, so unified to neon I think it is more appropriate

That's fine if it's only on aarch64.


About Joyk


Aggregate valuable and interesting links.
Joyk means Joy of geeK