arm64: fix clang ICE on Windows for thunderx2t kernels#5618
arm64: fix clang ICE on Windows for thunderx2t kernels#5618martin-frbg merged 1 commit intoOpenMathLib:developfrom
Conversation
Guard .align directive to avoid internal compiler error on AArch64 Windows with clang. See: llvm/llvm-project#149547 See: OpenMathLib#5076 Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
| " beq 3f //dot_kernel_F1 \n" | ||
| #ifndef _MSC_VER | ||
| /* https://github.com/llvm/llvm-project/issues/149547 */ | ||
| #if !(defined(__clang__) && defined(OS_WINDOWS)) |
There was a problem hiding this comment.
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) ?
There was a problem hiding this comment.
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
There was a problem hiding this comment.
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__
There was a problem hiding this comment.
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
|
Thanks - did you happen to check if these changes are sufficient for using these files with WoA ? I think it is likely that they might need changes to the storing of the complex results (similar to the earlier zdot changes) but I won't get around to checking this myself today |
I'm not precisely sure what you want checked. It compiles and seems to pass tests. clang has always supported c99 just fine (unlike MSVC, which still is buggy): Lines 579 to 583 in 5ffbf38 |
|
CI failures are spurious BTW
Back in december I had to work around the OPENBLAS_COMPLEX_FLOAT macro in zdot_thunderx2t99.c not compiling (with LLVM21 on a WoA laptop I had on loan from Qualcomm), but could be the conditional in common.h is borked. |
|
It might be because I'm using mingw-w64 headers, which seem to have full c99 support, while it sounded like you might have been using MSVC headers? |
I guess that would explain it - I'm doing my best to keep environments separate so that both MSYS2 and "native" LLVM package builds will work on WoA. |
Guard .align directive to avoid internal compiler error on AArch64 Windows with clang.
See: llvm/llvm-project#149547
Update of #5566 to fix remaining issues
See: #5076