Skip to content

arm64: fix clang ICE on Windows for thunderx2t kernels #5618

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
martin-frbg merged 1 commit into from
Jan 20, 2026
+11 −1
Merged

arm64: fix clang ICE on Windows for thunderx2t kernels #5618

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 3 additions & 0 deletions kernel/arm64/dznrm2_thunderx2t99_fast.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -155,7 +155,10 @@ static double nrm2_compute(BLASLONG n, FLOAT *x, BLASLONG inc_x)
" cmp "J", xzr \n"
" beq .Lnrm2_kernel_F1 \n"

/* https://github.com/llvm/llvm-project/issues/149547 */
#if !(defined(__clang__) && defined(OS_WINDOWS))
" .align 5 \n"
#endif
".Lnrm2_kernel_F: \n"
" "KERNEL_F" \n"
" subs "J", "J", #1 \n"
Expand Down
3 changes: 3 additions & 0 deletions kernel/arm64/scnrm2_thunderx2t99.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -238,7 +238,10 @@ static double nrm2_compute(BLASLONG n, FLOAT *x, BLASLONG inc_x)
" cmp "J", xzr \n"
" beq 5f //nrm2_kernel_S_BEGIN \n"

/* https://github.com/llvm/llvm-project/issues/149547 */
#if !(defined(__clang__) && defined(OS_WINDOWS))
" .align 5 \n"
#endif
"2: //nrm2_kernel_F: \n"
" "KERNEL_F" \n"
" subs "J", "J", #1 \n"
Expand Down
3 changes: 2 additions & 1 deletion kernel/arm64/zdot_thunderx2t99.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -243,7 +243,8 @@ static void zdot_compute(BLASLONG n, FLOAT *x, BLASLONG inc_x, FLOAT *y, BLASLON
" asr "J", "N", #"N_DIV_SHIFT" \n"
" cmp "J", xzr \n"
" beq 3f //dot_kernel_F1 \n"
#ifndef _MSC_VER
/* https://github.com/llvm/llvm-project/issues/149547 */
#if !(defined(__clang__) && defined(OS_WINDOWS))
Copy link
Collaborator

@martin-frbg martin-frbg Jan 19, 2026

Choose a reason for hiding this comment

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

I believe the _MSC_VER to be equivalent here (it will never match plain MSVC as that can't handle the inline assembly anyway, but it matches any LLVM derivative that uses the clang-cl shim) ?

Copy link
Contributor Author

@vtjnash vtjnash Jan 19, 2026 •
edited
Loading

Choose a reason for hiding this comment

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

It might be similar? Hard to check exactly if all libc specify that clang+_WIN32 is exactly the same as MSC when this needs to do is to specify clang+_WIN32 and exclude MSVC. It is just very confusing to intentionally use a macro to work around a bug in one specific compiler by specifying that this should be disabled for a completely different compiler

Copy link
Collaborator

@martin-frbg martin-frbg Jan 20, 2026

Choose a reason for hiding this comment

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

Oh well, I'm not opposed to switching to your version if it feels less confusing. My idea was that it would catch any LLVM-derived compiler that goes through the MSVC backend as well, not immediately sure if those would also define __clang__

Copy link
Contributor Author

@vtjnash vtjnash Jan 20, 2026

Choose a reason for hiding this comment

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

My thought is that eventually it gets fixed in llvm, and then these checks can be changed to an exact __clang_major__ check

There is clang/c2, which is clang with msvc backend (https://devblogs.microsoft.com/cppblog/clang-c2-we-need-your-advice/), but that wouldn't have this bug. I don't think there is an implementation for using msvc frontend with llvm backend

" .align 5 \n"
#endif
"2: //dot_kernel_F: \n"
Expand Down
3 changes: 3 additions & 0 deletions kernel/arm64/zsum_thunderx2t99.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -136,7 +136,10 @@ static FLOAT zasum_compute(BLASLONG n, FLOAT *x, BLASLONG inc_x)
" cmp "J", xzr \n"
" beq 3f //asum_kernel_F1 \n"

/* https://github.com/llvm/llvm-project/issues/149547 */
#if !(defined(__clang__) && defined(OS_WINDOWS))
".align 5 \n"
#endif
"2: //asum_kernel_F16: \n"
" "KERNEL_F16" \n"
" subs "J", "J", #1 \n"
Expand Down
Loading