Skip to content

Fix compile error for i686-pc-windows-msvc target#123

Merged
Lokathor merged 4 commits into
Lokathor:mainfrom
KonaeAkira:pr/fix-i686-compile-error
Nov 16, 2025
Merged

Fix compile error for i686-pc-windows-msvc target#123
Lokathor merged 4 commits into
Lokathor:mainfrom
KonaeAkira:pr/fix-i686-compile-error

Conversation

@KonaeAkira

Copy link
Copy Markdown
Contributor

This should (hopefully) fix the sporadic compile failure on CI.

+a lot of trailing whitespace removal done by my editor.

@Lokathor

Lokathor commented Nov 8, 2025

Copy link
Copy Markdown
Owner

This is extremely confusing because the avx512f cfg on the module overall should be preventing this error to begin with. Unless there's actual i686 targets with avx-512? In which case they'd need the imports you adjusted, just from ::core::arch::x86::* instead.

@Lokathor

Lokathor commented Nov 8, 2025

Copy link
Copy Markdown
Owner

Look at the CI... I guess there are i686 targets with the avx-512 flags such that we need to fix up all imports in the avx512 module.

@KonaeAkira

Copy link
Copy Markdown
Contributor Author

Looks like avx512f is a supported target feature in i686:

rustc --target i686-pc-windows-msvc --print target-features | grep avx512f

@KonaeAkira KonaeAkira marked this pull request as draft November 8, 2025 20:54
@KonaeAkira

Copy link
Copy Markdown
Contributor Author

Yeah... fixing all the doctests is gonna take a while lol

@KonaeAkira KonaeAkira marked this pull request as ready for review November 8, 2025 21:22
@KonaeAkira

Copy link
Copy Markdown
Contributor Author

Turns out doctests have access to the outer scope? So there's no need to explicitly specify core::arch::x86_64 or core::arch::x86 in the doctests. Learned another thing today :)

@Lokathor

Lokathor commented Nov 8, 2025

Copy link
Copy Markdown
Owner

Huh maybe that's new because I remember my doc tests having all sorts of imports when I used to write them a lot.

@KonaeAkira

Copy link
Copy Markdown
Contributor Author

Looks like that isn't the case after all. The build_test workflow ran successfully so I thought that was the case. Sorry :/

@KonaeAkira KonaeAkira force-pushed the pr/fix-i686-compile-error branch from 4d31875 to 4af05bb Compare November 8, 2025 21:51
@KonaeAkira

Copy link
Copy Markdown
Contributor Author

I just realized that requiring the user to specify OP from either core::arch::x86 or core::arch::x86_64 isn't really ideal. Isn't avoiding having to manually specify core::arch::* the whole point of the crate?

@Lokathor

Copy link
Copy Markdown
Owner

It lets you write, for example, "SSE code" that's portable across both x86 and x86_64 SSE. It's handy enough, even though it's not perfect.

@Lokathor Lokathor merged commit 794be26 into Lokathor:main Nov 16, 2025
12 checks passed
@Lokathor

Copy link
Copy Markdown
Owner

Released in 0.9.3

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

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants