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

Problem with musl and fat runtime? #95

Open
Jc2k opened this issue Sep 8, 2023 · 5 comments
Open

Problem with musl and fat runtime? #95

Jc2k opened this issue Sep 8, 2023 · 5 comments
Assignees

Comments

@Jc2k
Copy link

Jc2k commented Sep 8, 2023

Sorry I haven't had time to dig into this as much as i'd like but i've been getting:

Illegal instruction (core dumped)

When preparing a hyperscan database, on alpine 3.18 / python 3.11 and with latest python-hyperscan tag.

It works fine on an iMac running Linux (Fedora). And also on a newer iMac running macOS. The box where its crashing (CI) does have an older processor and its in an Alpine container. But the processor does still meet the minimum requirements.

I've built a local wheel that uses -march=core2 and disabled FAT_RUNTIME and that works. -march=corei7 also works.

This post (from earlier this year) suggests musl doesn't support the ifunc feature required for fat runtime. Looking through build logs I see

-- Performing Test HAS_C_ATTR_IFUNC
-- Performing Test HAS_C_ATTR_IFUNC - Failed
-- Compiler does not support ifunc attribute, cannot build fat runtime

So the FAT_RUNTIME define is getting ignored in musl builds. I think this means that the wheels only support the arch that github runners happen to use, which is probably AVX2?

Any chance we can lower the target -march in the official wheels?

@darvid
Copy link
Owner

darvid commented Nov 21, 2023

i can disable fat runtime and set march to core2 for just the musl wheels, but that would mean applications running on alpine on hosts that support AVX2 would still use SSE3, right?

is this still a pain point for you? i'm trying to think of the best universal way to support this, and other instruction sets that are newer (like AVX512, which requires manual config at buildtime and isn't turned on with fat runtime) that might require changing the default build configuration.

my first thought is before changing the build configuration across the board for all musl-based wheels, we can avoid the crash on processors with the minimum required march by testing for libc/ifunc support and turning off fat runtime, and setting march. that way the source dist still builds for the minimum requirements. would that be a good compromise?

@Jc2k
Copy link
Author

Jc2k commented Nov 21, 2023

I have a workaround locally. It's not pretty, but it works.

But, I still wonder if this wheel is technically not valid - aren't cpu features part of the ABI? So if you are following the PEPs you'd have to build for the same minimum cpu that eg alpine uses?

In which case the "right" thing to do by the spec is to make the wheel work everywhere, and make the sdist for people with beefier processors.

@darvid
Copy link
Owner

darvid commented Nov 21, 2023

ok this will require a change in the underlying docker image i maintain with hyperscan and deps needed for static linking, but correct me if i'm wrong, based on your logs it looks like HS properly turned fat runtime off, so i'm guessing -march=native breaks on musl?

@darvid
Copy link
Owner

darvid commented Nov 21, 2023

either way i think you're right, i'll set march but only on musl

  • update manylinux-hyperscan build to conditionally set march=core2 on alpine
  • might be able to remove FAT_RUNTIME completely from build because it's default on linux
  • update docs for build process, add support for newer AVX512* that require manually enabling support when building HS: https://intel.github.io/hyperscan/dev-reference/getting_started.html#fat-runtime
  • update this project to use newer container and release new version (might be able to get away with keeping existing release and bumping build number for musl wheels, i'll try)

@darvid darvid self-assigned this Nov 21, 2023
@Jc2k
Copy link
Author

Jc2k commented Nov 21, 2023

Yes, afaict you are right. fat runtime is turned off so -march=native is a no go, because the GitHub runners have AVX.

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

No branches or pull requests

2 participants