Repository navigation
Arm NEON bf16 intrinsics - #2256
Jamesbarford wants to merge 5 commits into
Conversation
|
r? @folkertdev rustbot has assigned @folkertdev. Use Why was this reviewer chosen?The reviewer was selected based on:
|
| if self.kind() == TypeKind::BFloat && self.simd_len.is_none() && self.vec_len.is_none() { | ||
| return "f16b".to_string(); | ||
| } |
There was a problem hiding this comment.
Why does this need special-casing? A comment would help.
There was a problem hiding this comment.
Added, it's so we now create NanEqF16b
| } | ||
|
|
||
| fn comparison_function(&self) -> String { | ||
| if self.kind() == TypeKind::BFloat { |
There was a problem hiding this comment.
similarly a comment on why this type is special would help.
There was a problem hiding this comment.
Removed and renamed NanEqBF16 to NanEqF16b which makes the code simpler, removing this case.
| if arg.ty.kind() == TypeKind::BFloat { | ||
| let ty = arg.ty.rust_type(); | ||
| return format!( | ||
| "let {name}: {ty} = std::ptr::read_unaligned(\ | ||
| {vals_name}.as_ptr().add((i+{idx}) % {PASSES}) as *const {ty});", | ||
| vals_name = test_values_array_name(&arg.ty), | ||
| ); | ||
| } |
There was a problem hiding this comment.
why is this special-cased? could use a comment.
There was a problem hiding this comment.
We don't need this. I was experimenting and forgot to remove it.
| CcArgStyle::Clang if !a32 && !big_endian => vec
1a54bf3 to
f3ac668
Compare
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
4542524 to
892a26d
Compare
|
This PR was rebased onto a different main commit. Here's a range-diff highlighting what actually changed. Rebasing is a normal part of keeping PRs up to date, so no action is needed—this note is just to help reviewers. |
| "vbfdot_f32", | ||
| "vbfdot_lane_f32", | ||
| "vbfdot_laneq_f32", | ||
| "vbfdotq_f32", | ||
| "vbfdotq_lane_f32", | ||
| "vbfdotq_laneq_f32", | ||
| "vbfmlalbq_f32", | ||
| "vbfmlalbq_lane_f32", | ||
| "vbfmlalbq_laneq_f32", |
There was a problem hiding this comment.
so why are all of these skipped?
There was a problem hiding this comment.
and consequently, how were the intrinsics added by this PR tested?
There was a problem hiding this comment.
so why are all of these skipped?
We use the intrinsic-test tool, from the code comment;
// FIXME(arm-maintainers): With the advent of the
intrinsic-testtool, new tests of this kind
// are no longer being added and just adding to this list indefinitely isn't the best solution for
// dealing with that.
So I've added to the list 🫠, obviously not ideal.
and consequently, how were the intrinsics added by this PR tested?
For runtime tests I've tested with the intrinsic test tool, running;
./ci/intrinsic-test-docker.sh aarch64-unknown-linux-gnu clangThen for instruction assertions and ensuring the signatures are correct;
./ci/run-docker.sh aarch64-unknown-linux-gnuSo they get tested in the CI too, if you grep the raw ci logs for vbfdot_f32 you will see it is there.
However The lut intrinsics are skipped with no tests. The CI uses Ubuntu 25:10 of which qemu-user is version 10.1.0 which does not have FEAT_LUT. FEAT_LUT is made available in QEMU version 11.1.0
Locally I am using 11.1.2 and thus can execute lut at one point in this PR I did add lut as I forgot why we were skipping them.
Adds support for bfloat intrinsics for NEON