Implementation for Aarch64 TME intrinsics #855
Conversation
|
Thanks for the pull request, and welcome! The Rust team is excited to review your changes, and you should hear from @gnzlbg (or someone else) soon. If any changes to this PR are deemed necessary, please add them as extra commits. This ensures that the reviewer can see what has changed since they last reviewed the code. Due to the way GitHub handles out-of-date commits, this should also make it reasonably obvious what issues have or haven't been addressed. Large or tricky changes may require several passes of review and changes. Please see the contribution instructions for more information. |
…es, r=Amanieu Enable ARM TME (Transactional Memory Extensions) Enables ARM TME coming up with LLVM 10. Related ARM TME intrinsics are included by the merge of rust-lang#67900. Enables: rust-lang/stdarch#855
vertexclique
force-pushed
the
vcq/aarch64-tme-extensions
branch
6 times, most recently
from
6be4f82 to
d79e0a7
Compare
|
@Amanieu There is a problem with current tests for neon intrinsics in the test suite. Any way to get over it? Marking the PR as ready for review. |
vertexclique
force-pushed
the
vcq/aarch64-tme-extensions
branch
from
d79e0a7 to
996914e
Compare
|
I'll have a look at the tests, it doesn't seem to be due to your code. |
|
I fixed the CI failures in #857, please rebase. |
vertexclique
force-pushed
the
vcq/aarch64-tme-extensions
branch
from
996914e to
9fb45e7
Compare
|
Rebased, it seems QEMU doesn't come with this feature extension. |
| @@ -45,6 +45,7 @@ fn aarch64_linux() { | |||
| println!("rdm: {}", is_aarch64_feature_detected!("rdm")); | |||
| println!("rcpc: {}", is_aarch64_feature_detected!("rcpc")); | |||
| println!("dotprod: {}", is_aarch64_feature_detected!("dotprod")); | |||
| println!("tme: {}", is_aarch64_feature_detected!("tme")); | |||
There was a problem hiding this comment.
You need to implement detection for this feature. Do a search for dotprod to see how it is done.
There was a problem hiding this comment.
Updated with feature detection, didn't find the feature tbh. Even instructions and signatures are the same. llvm/llvm-project@a36d314#diff-3674dac5c4241890431fece8594c3143R25-R34
What am I doing wrong? (I still suspect from qemu, checked source code, it doesn't exist there yet. Might be wrong.)
There was a problem hiding this comment.
The current feature detection is fine. Linux doesn't have the TME flag in /proc/cpuinfo or AT_HWCAPS yet, so there's nothing we can do about that for now.
There was a problem hiding this comment.
Yes, when it enters to kernel I will add here.
There was a problem hiding this comment.
Addressed your comments. Thanks for the help! @Amanieu
vertexclique
force-pushed
the
vcq/aarch64-tme-extensions
branch
6 times, most recently
from
59f8fbb to
f91a223
Compare
|
It seems that the tests are failing because the Try updating the ubuntu version to 20.04 in |
vertexclique
force-pushed
the
vcq/aarch64-tme-extensions
branch
from
f91a223 to
99e6f75
Compare
|
Thanks! You'll need to send a PR to rust-lang/rust to update the stdarch submodule if you want to have this in the next nightly. |
|
I am not sure if we can have intrinsics like these in Rust. After all, the compiler may reorder unrelated memory accesses into the transaction block, which would then be un-done when the transaction is aborted, which can cause arbitrary program misbehavior. Or am I misunderstanding how these instructions work? See #1521 for discussion. |
This enables Aarch64 TME intrinsics and documents them. We need to enable
tmefeature atrust-lang/rustbefore merging this.