Skip to content

add vldx neon instructions#1200

Merged
Amanieu merged 7 commits into
rust-lang:masterfrom
SparrowLii:vldx
Aug 24, 2021
Merged

add vldx neon instructions#1200
Amanieu merged 7 commits into
rust-lang:masterfrom
SparrowLii:vldx

Conversation

@SparrowLii

Copy link
Copy Markdown
Member

This PR adds vld1 neon intruscions which read multiple vectors at once.
Fixes #1143

@rust-highfive

Copy link
Copy Markdown

r? @Amanieu

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

Comment thread crates/stdarch-gen/src/main.rs Outdated
"unadjusted"
} else {
"C"
};

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I think we would want to always use "unadjusted" when linking with LLVM intrinsics, unless I'm missing something?

@SparrowLii SparrowLii Aug 23, 2021

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

I'm not sure if these links depend on "C" Abi, especially other architectures, like x86. In view of the fact that most of the structs are marked by #[repr(simd)], the difference in Abi should only affect the structs containing multiple vectors IMO. Let's try it.

@SparrowLii SparrowLii Aug 23, 2021

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Using "unadjusted" looks fine, at least for arm/aarch64. It also slightly simplifies the compilation process, so I think it makes sense.
A few assert_instrs of some instructions using simd_extract need to be correct, I think this may be due to some updates to the nightly compiler

#[rustc_legacy_const_generics(1)]
#[cfg_attr(all(test, target_arch = "arm"), assert_instr("vmov.32", IMM5 = 1))]
#[cfg_attr(all(test, target_arch = "arm"), assert_instr("vmov", IMM5 = 1))]
#[cfg_attr(all(test, target_arch = "aarch64"), assert_instr(mov, IMM5 = 1))]

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I think for these we should just use assert_instr(nop, ...). nop is handled specially and accepts any instruction.

@SparrowLii SparrowLii Aug 24, 2021

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

It makes sense. We should not care about the specific implementation of simd_extract here. So I replaced with "nop" for vget instructions.

Comment on lines +3120 to +3121
#[cfg_attr(all(test, target_arch = "arm"), assert_instr("nop", IMM5 = 1))]
#[cfg_attr(all(test, target_arch = "aarch64"), assert_instr(nop, IMM5 = 1))]

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Suggested change
#[cfg_attr(all(test, target_arch = "arm"), assert_instr("nop", IMM5 = 1))]
#[cfg_attr(all(test, target_arch = "aarch64"), assert_instr(nop, IMM5 = 1))]
#[cfg_attr(test, assert_instr(nop, IMM5 = 1))]

@SparrowLii SparrowLii Aug 24, 2021

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Thanks, I have made the corresponding changes.
I think I can submit a new PR later to correct all assert_instr in arm/aarch64 mod.

@Amanieu Amanieu merged commit 6ddda5c into rust-lang:master Aug 24, 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.

Can't get the correct return type of llvm link

3 participants