Skip to content

Donot assert SSE/AVX resolvers at windows arm64 #7704

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

Closed
wants to merge 1 commit into from

Conversation

dixyes
Copy link
Contributor

@dixyes dixyes commented Dec 1, 2021

This is a series of work on building php for windows arm64 (cross compile only yet)

zend_portability.h have asserted windows machines have SSE/AVX resolvers, this patch make exception for arm64

(or maybe we can enable it only for x86/x86_64 that it still work when Microsoft decides to support other architectures?)

@devnexen
Copy link
Member

devnexen commented Jul 12, 2022

@cmb69 so what should we do about this serie in your opinion ? so the real concern would be this particular pr, am I correct ? So would it makes sense to merge the two others but then disabling intrinsics optimisation code path for arm64 only for the aforementioned pr (for now) or do we wait until we can actually test it ?

@cmb69
Copy link
Member

cmb69 commented Jul 14, 2022

@devnexen, my concern is not about any of these PRs in particular, but rather that (to my knowledge) none of the core developers would be able to do any testing of Windows on ARM64. That means that it would be hard or even impossible to fix possible bugs on that platform.

That said, I don't think we can afford to ignore Windows on ARM64 altogether. So it's probably best to merge these PRs (I'll review soonish), and to clearly mark support for this platform as highly experimental.

@devnexen
Copy link
Member

That said, I don't think we can afford to ignore Windows on ARM64 altogether.

Yes.

So it's probably best to merge these PRs (I'll review soonish), and to clearly mark support for this platform as highly experimental.

Cool, thank you :-)

Copy link
Member

@cmb69 cmb69 left a comment

Choose a reason for hiding this comment

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

Thank you for the PR! I think we should go with this for now; we may later revise to cater to changes in the toolchain.

@cmb69 cmb69 closed this in 180557d Jul 19, 2022
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.

3 participants