Skip to content

x86_64 reference implementation of f16 fma uses wrong instruction #44

Open

Description

@programmerjake

while reading through the new sqrt code I noticed we use the f32 fma instruction where we should be using the f16 fma:

if is_x86_feature_detected!("avx512fp16") {
unsafe {
let status = with_fp_env!(
rm,
"vfmadd213ss {x}, {y}, {z}",
x = inout(xmm_reg) self,
y = in(xmm_reg) mul,
z = in(xmm_reg) add,
);
Some(status.and(self))
}
} else {
no_fp_env(rm, || self.mul_add(mul, add))
}

that should be vfmadd213sh

Activity

  1. tgross35 commented on Oct 11, 2026

    @tgross35
    Member

    Oh, darn, I don't have an avx512 machine to test this on.

    Do you happen to know when .arch_extension is needed? I thought it technically was here but the assembler seems to accept without it.

  2. programmerjake commented on Oct 11, 2026

    @programmerjake
    MemberAuthor
    1. Do you happen to know when .arch_extension is needed?

    seems to only be on aarch64/arm, from searching for arch_extension in gas's manual: https://sourceware.org/binutils/docs/as/AS-Index.html

  3. programmerjake commented on Oct 11, 2026

    @programmerjake
    MemberAuthor
    1. Oh, darn, I don't have an avx512 machine to test this on.

    idk how much it helps, but Compiler Explorer seems to use an avx512 runner about half the time, you can click the refresh button until you get it

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions