Skip to content

Conversation

@wubowski
Copy link

@wubowski wubowski commented Oct 10, 2025

  • Rewrote bf16 to f32 conversion intrinsics in avx512bf16intrin.h & avx512vlbf16intrin.h to replace manual bit manipulations with standard conversions using __builtin_convertvector (including masked variants).

avx512bf16intrin.h functions rewritten:

  • _mm_cvtsbh_ss
  • _mm512_cvtpbh_ps
  • _mm512_maskz_cvtpbh_ps
  • _mm512_mask_cvtpbh_ps

avx512vlbf16intrin.h functions rewritten:

  • _mm_cvtpbh_ps
  • _mm_maskz_cvtpbh_ps
  • _mm_mask_cvtpbh_ps
  • _mm256_cvtpbh_ps
  • _mm256_maskz_cvtpbh_ps
  • _mm256_mask_cvtpbh_ps

TODO:

  • Conform code to LLVM coding standards
  • Create additional unit test(s)

CC: @RKSimon –feedback very appreciated!

Fixes #154911

@github-actions
Copy link

Thank you for submitting a Pull Request (PR) to the LLVM Project!

This PR will be automatically labeled and the relevant teams will be notified.

If you wish to, you can add reviewers by using the "Reviewers" section on this page.

If this is not working for you, it is probably because you do not have write permissions for the repository. In which case you can instead tag reviewers by name in a comment by using @ followed by their GitHub username.

If you have received no comments on your PR for a week, you can request a review by "ping"ing the PR by adding a comment “Ping”. The common courtesy "ping" rate is once a week. Please remember that you are asking for valuable time from other developers.

If you have further questions, they may be answered by the LLVM GitHub User Guide.

You can also ask questions in a comment on this PR, on the LLVM Discord or on the forums.

@RKSimon RKSimon requested review from RKSimon and tbaederr October 12, 2025 16:34
Copy link
Collaborator

@RKSimon RKSimon left a comment

Choose a reason for hiding this comment

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

We should be able to remove cvtsbf162ss from X86Builtins.td (and its use in X86.cpp) entirely now

_mm512_maskz_cvtpbh_ps(__mmask16 __U, __m256bh __A) {
return _mm512_castsi512_ps((__m512i)_mm512_slli_epi32(
(__m512i)_mm512_maskz_cvtepi16_epi32((__mmask16)__U, (__m256i)__A), 16));
return _mm512_maskz_mov_ps(__U, (__m512)__builtin_convertvector(__A, __v16sf));
Copy link
Collaborator

Choose a reason for hiding this comment

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

(style) - wrap _mm512_cvtpbh_ps instead of calling __builtin_convertvector directly - same for the other mask/maskz intrinsics

@RKSimon RKSimon changed the title Convert bf16 to f32 conversions to generic constexpr implementations [Headers][X86] Convert bf16 to f32 conversions to generic constexpr implementations Oct 14, 2025
@github-actions
Copy link

⚠️ C/C++ code formatter, clang-format found issues in your code. ⚠️

You can test this locally with the following command:
git-clang-format --diff origin/main HEAD --extensions h -- clang/lib/Headers/avx512bf16intrin.h clang/lib/Headers/avx512vlbf16intrin.h

⚠️
The reproduction instructions above might return results for more than one PR
in a stack if you are using a stacked PR workflow. You can limit the results by
changing origin/main to the base branch/commit you want to compare against.
⚠️

View the diff from clang-format here.
diff --git a/clang/lib/Headers/avx512bf16intrin.h b/clang/lib/Headers/avx512bf16intrin.h
index 4968136f7..5782d81ea 100644
--- a/clang/lib/Headers/avx512bf16intrin.h
+++ b/clang/lib/Headers/avx512bf16intrin.h
@@ -236,7 +236,7 @@ _mm512_maskz_dpbf16_ps(__mmask16 __U, __m512 __D, __m512bh __A, __m512bh __B) {
 ///    A 256-bit vector of [16 x bfloat].
 /// \returns A 512-bit vector of [16 x float] come from conversion of __A
 static __inline__ __m512 __DEFAULT_FN_ATTRS512 _mm512_cvtpbh_ps(__m256bh __A) {
-  return (__m512)__builtin_convertvector(__A, __v16sf);
+  return (__m512) __builtin_convertvector(__A, __v16sf);
 }
 
 /// Convert Packed BF16 Data to Packed float Data using zeroing mask.
@@ -251,7 +251,8 @@ static __inline__ __m512 __DEFAULT_FN_ATTRS512 _mm512_cvtpbh_ps(__m256bh __A) {
 /// \returns A 512-bit vector of [16 x float] come from conversion of __A
 static __inline__ __m512 __DEFAULT_FN_ATTRS512
 _mm512_maskz_cvtpbh_ps(__mmask16 __U, __m256bh __A) {
-  return _mm512_maskz_mov_ps(__U, (__m512)__builtin_convertvector(__A, __v16sf));
+  return _mm512_maskz_mov_ps(__U,
+                             (__m512) __builtin_convertvector(__A, __v16sf));
 }
 
 /// Convert Packed BF16 Data to Packed float Data using merging mask.
@@ -268,7 +269,8 @@ _mm512_maskz_cvtpbh_ps(__mmask16 __U, __m256bh __A) {
 /// \returns A 512-bit vector of [16 x float] come from conversion of __A
 static __inline__ __m512 __DEFAULT_FN_ATTRS512
 _mm512_mask_cvtpbh_ps(__m512 __S, __mmask16 __U, __m256bh __A) {
-  return _mm512_mask_mov_ps(__S, __U, (__m512)__builtin_convertvector(__A, __v16sf));
+  return _mm512_mask_mov_ps(__S, __U,
+                            (__m512) __builtin_convertvector(__A, __v16sf));
 }
 
 #undef __DEFAULT_FN_ATTRS
diff --git a/clang/lib/Headers/avx512vlbf16intrin.h b/clang/lib/Headers/avx512vlbf16intrin.h
index 2051694fc..1b5110aa3 100644
--- a/clang/lib/Headers/avx512vlbf16intrin.h
+++ b/clang/lib/Headers/avx512vlbf16intrin.h
@@ -422,7 +422,7 @@ static __inline__ __bf16 __DEFAULT_FN_ATTRS128 _mm_cvtness_sbh(float __A) {
 ///    A 128-bit vector of [4 x bfloat].
 /// \returns A 128-bit vector of [4 x float] come from conversion of __A
 static __inline__ __m128 __DEFAULT_FN_ATTRS128 _mm_cvtpbh_ps(__m128bh __A) {
-  return (__m128)__builtin_convertvector(__A, __v4sf);
+  return (__m128) __builtin_convertvector(__A, __v4sf);
 }
 
 /// Convert Packed BF16 Data to Packed float Data.
@@ -433,7 +433,7 @@ static __inline__ __m128 __DEFAULT_FN_ATTRS128 _mm_cvtpbh_ps(__m128bh __A) {
 ///    A 128-bit vector of [8 x bfloat].
 /// \returns A 256-bit vector of [8 x float] come from conversion of __A
 static __inline__ __m256 __DEFAULT_FN_ATTRS256 _mm256_cvtpbh_ps(__m128bh __A) {
-  return (__m256)__builtin_convertvector(__A, __v8sf);
+  return (__m256) __builtin_convertvector(__A, __v8sf);
 }
 
 /// Convert Packed BF16 Data to Packed float Data using zeroing mask.
@@ -448,7 +448,7 @@ static __inline__ __m256 __DEFAULT_FN_ATTRS256 _mm256_cvtpbh_ps(__m128bh __A) {
 /// \returns A 128-bit vector of [4 x float] come from conversion of __A
 static __inline__ __m128 __DEFAULT_FN_ATTRS128
 _mm_maskz_cvtpbh_ps(__mmask8 __U, __m128bh __A) {
-  return _mm_maskz_mov_ps(__U, (__m128)__builtin_convertvector(__A, __v4sf));
+  return _mm_maskz_mov_ps(__U, (__m128) __builtin_convertvector(__A, __v4sf));
 }
 
 /// Convert Packed BF16 Data to Packed float Data using zeroing mask.
@@ -463,7 +463,8 @@ _mm_maskz_cvtpbh_ps(__mmask8 __U, __m128bh __A) {
 /// \returns A 256-bit vector of [8 x float] come from conversion of __A
 static __inline__ __m256 __DEFAULT_FN_ATTRS256
 _mm256_maskz_cvtpbh_ps(__mmask8 __U, __m128bh __A) {
-  return _mm256_maskz_mov_ps(__U, (__m256)__builtin_convertvector(__A, __v8sf));
+  return _mm256_maskz_mov_ps(__U,
+                             (__m256) __builtin_convertvector(__A, __v8sf));
 }
 
 /// Convert Packed BF16 Data to Packed float Data using merging mask.
@@ -481,7 +482,8 @@ _mm256_maskz_cvtpbh_ps(__mmask8 __U, __m128bh __A) {
 /// \returns A 128-bit vector of [4 x float] come from conversion of __A
 static __inline__ __m128 __DEFAULT_FN_ATTRS128
 _mm_mask_cvtpbh_ps(__m128 __S, __mmask8 __U, __m128bh __A) {
-  return _mm_mask_mov_ps(__S, __U, (__m128)__builtin_convertvector(__A, __v4sf));
+  return _mm_mask_mov_ps(__S, __U,
+                         (__m128) __builtin_convertvector(__A, __v4sf));
 }
 
 /// Convert Packed BF16 Data to Packed float Data using merging mask.
@@ -499,7 +501,8 @@ _mm_mask_cvtpbh_ps(__m128 __S, __mmask8 __U, __m128bh __A) {
 /// \returns A 256-bit vector of [8 x float] come from conversion of __A
 static __inline__ __m256 __DEFAULT_FN_ATTRS256
 _mm256_mask_cvtpbh_ps(__m256 __S, __mmask8 __U, __m128bh __A) {
-  return _mm256_mask_mov_ps(__S, __U, (__m256)__builtin_convertvector(__A, __v8sf));
+  return _mm256_mask_mov_ps(__S, __U,
+                            (__m256) __builtin_convertvector(__A, __v8sf));
 }
 
 #undef __DEFAULT_FN_ATTRS128

static __inline__ __m128 __DEFAULT_FN_ATTRS128 _mm_cvtpbh_ps(__m128bh __A) {
return _mm_castsi128_ps(
(__m128i)_mm_slli_epi32((__m128i)_mm_cvtepi16_epi32((__m128i)__A), 16));
return (__m128)__builtin_convertvector(__A, __v4sf);
Copy link
Collaborator

Choose a reason for hiding this comment

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

You're missing a shuffle vector to only access the bottom 4 elements of __A:

Suggested change
return (__m128)__builtin_convertvector(__A, __v4sf);
return (__m128)__builtin_convertvector(__builtin_shufflevector(__A, __A, 0, 1, 2, 3), __v4sf);

(will need clang-format)

@RKSimon
Copy link
Collaborator

RKSimon commented Nov 5, 2025

@wubowski ping?

@RKSimon
Copy link
Collaborator

RKSimon commented Nov 17, 2025

@wubowski are you still looking at this please?

@RKSimon
Copy link
Collaborator

RKSimon commented Nov 24, 2025

@wubowski Are you still looking at this or should I release the issue again?

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.

[Headers][X86] Convert bf16 to f32 conversions to generic constexpr implementations

2 participants