Stop pinning clang in Windows ucrt #55
Conversation
stanhu
force-pushed
the
sh-remove-clang-pin
branch
from
352f1b9 to
09cb870
Compare
| extra_includes=$msys_root/opt/rb-sys/include | ||
| mkdir -p $msys_root/opt/rb-sys/include | ||
| cp 'C:\Program Files\LLVM\lib\clang\18\include\stdckdint.h' $extra_includes | ||
| bindgen_extra_clang_args="--target=${{ steps.derive-toolchain.outputs.toolchain }} --sysroot=$msys_root -D__AVX512VLFP16INTRIN_H -D__AVX512FP16INTRIN_H" |
There was a problem hiding this comment.
@jbourassa @ianks I might be too optimistic, but did this just work?
- This build: https://github.com/stanhu/actions/actions/runs/12956772391
- Previously failing build: https://github.com/jbourassa/oxidize-rb-actions/actions/runs/12583936454/job/35072741265
- Reference:
_Float16 _Complexleads to panic (such as when includingimmintrin.hon clang 16) rust-lang/rust-bindgen#2500 (comment)
There was a problem hiding this comment.
Looks like it did! Not sure what the exact implications of defining those headers are, but it sounds reasonable to me.
I'm willing to merge this, but I'll wait a bit to see if Ian has something to add.
There was a problem hiding this comment.
oxidize-rb/oxi-test#13 fixes the ruby-head errors.
There was a problem hiding this comment.
I think defining those preprocessor headers effectively disables the ability to use of AVX-512 FP16 intrinsics.
I see that these intrinsics were added in clang 14 (llvm/llvm-project@6f7f5b5), but in clang 16 these intrinsics may have been included by default if SSE2 is available: llvm/llvm-project@08388ad.
Try the workaround mentioned in rust-lang/rust-bindgen#2500 (comment).
stanhu
force-pushed
the
sh-remove-clang-pin
branch
from
09cb870 to
af3df1a
Compare
| extra_includes=$msys_root/opt/rb-sys/include | ||
| mkdir -p $msys_root/opt/rb-sys/include | ||
| cp 'C:\Program Files\LLVM\lib\clang\18\include\stdckdint.h' $extra_includes | ||
| bindgen_extra_clang_args="--target=${{ steps.derive-toolchain.outputs.toolchain }} --sysroot=$msys_root -D__AVX512VLFP16INTRIN_H -D__AVX512FP16INTRIN_H" |
There was a problem hiding this comment.
Looks like it did! Not sure what the exact implications of defining those headers are, but it sounds reasonable to me.
I'm willing to merge this, but I'll wait a bit to see if Ian has something to add.
|
Great find, this was the source of so many headaches |
|
Awesome, thanks! Finally see a full green build in https://github.com/oxidize-rb/rb-sys/actions/runs/12976870600. |
Try the workaround mentioned in rust-lang/rust-bindgen#2500 (comment).
Previous attempt: #47
Closes #54