Skip to content
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

build: fix osx build #1136

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

anonkey
Copy link
Contributor

@anonkey anonkey commented Sep 13, 2024

Hi,
i hade some trouble to build on mac os aarch64, needed some small fixes.
Moreover --no-default-feature wasn't working for full workspace build i replaced it for cfg() directive in cargo.toml to rely on sse4.2 feature presence

@CLAassistant
Copy link

CLAassistant commented Sep 13, 2024

CLA assistant check
All committers have signed the CLA.

@Wonshtrum
Copy link
Member

Hi! Sorry for the late answer.
Thanks for your interest in Sozu! The PR seems great, we never took the time to integrate SIMD into the build properly.
But I'm confused, your configuration seems backward:

[target.'cfg(not(target_feature = "sse4.2"))'.dependencies]
kawa = { version = "^0.6.6" } # kawa default is to use SSE

[target.'cfg(target_feature = "sse4.2")'.dependencies]
kawa = { version = "^0.6.6", default-features = false } # removing default, kawa will not use SSE

Did this configuration really work on your mac os aarch64?

@anonkey
Copy link
Contributor Author

anonkey commented Dec 11, 2024

Yes it works, why shouldn't it?
If SSE4.2 available it use kawa with default features else disable default features to avoid kawa use sse4.2 optimizations

@Wonshtrum
Copy link
Member

I agree, but it seems your configuration says the opposite. The first option (with default) is used if sse4.2 is not available (cfg(not(target_feature = "sse4.2"))), and the second (default-features=false) is used if sse4.2 is available (cfg(target_feature = "sse4.2")).
I did some tests and found that sse4.2 is never set even if the CPU supports it, unless you build with the rust flag -target-cpu=native. I must say I'm split, I firmly believe resource-intensive programs like Sozu should be built for a specific architecture to take full advantage of specific hardware features. But I also don't like making it hard to compile Sozu on a different machine than the one it will run on.

@anonkey anonkey force-pushed the fix-osx-workspace-build branch 4 times, most recently from 83483a4 to 8ffe7dc Compare December 21, 2024 15:21
@anonkey
Copy link
Contributor Author

anonkey commented Dec 21, 2024

Ok i understand why my test was false positive so.
So if avoiding the -target-cpu=native seem better and since the problem is import of std::arch::x86_64, is checking of the x86_64 arch ok for you ?
I checked on my arm mac and on my x86 server by inserting faulty code under the std::arch::x86_64 import it's working

@anonkey anonkey force-pushed the fix-osx-workspace-build branch 2 times, most recently from c79f4be to 5f6a590 Compare December 30, 2024 18:29
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.

3 participants