Skip to content

Exclude alignment for MaskBuffer for i686-win7-windows-msvc #87

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.

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
merged 1 commit into from
May 11, 2025

Conversation

drewkett
Copy link
Contributor

@drewkett drewkett commented Mar 25, 2025

Windows 7 does not respect 16 byte alignment for thread locals on i686
builds. Rust 1.79 changed i686 windows builds to use native thread local
support. As a result, using matrixmultiply on i686 win7 builds leads
to a UB check panic nounwind when run on Windows 7. See
rust-lang/rust#138903 for more info.

This change adds i686-win7-windows-msvc as an excluded target for the
alignment attribute on MaskBuffer.

Not sure how you feel about adding this exclusion in matrixmultiply as its
very much for a tier 3 target that you can't test in CI that its useful. But
figured I'd raise it, as it would affect users using that target on any recent
toolchain version. And fwiw, i did test that this does in fact have the desire
for compilations targeting i686-win7

Windows 7 does not respect 16 byte alignment for thread locals on i686
builds. Rust 1.79 changed i686 windows builds to use native thread local
support. As a result, using `matrixmultiply` on i686 win7 builds leads
to a UB check panic nounwind when run on Windows 7. See
<rust-lang/rust#138903> for more info.

This change adds `i686-win7-windows-msvc` as an excluded target for the
alignment attribute on `MaskBuffer`.
@bluss
Copy link
Owner

bluss commented May 11, 2025

Thanks, at this point we should have more or less abandoned any hope for that custom alignment. I will include your change.

@bluss bluss merged commit 301ebc5 into bluss:master May 11, 2025
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.

2 participants