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

Updated c-blosc to commit 9dc93b1 and zstd to v1.5.2 #200

Merged
merged 8 commits into from
Nov 8, 2022

Conversation

t20100
Copy link
Member

@t20100 t20100 commented Nov 8, 2022

This PR updates c-blosc to commit 9dc93b1 initially to fix macos universal2 build.
It also updates zstd to v1.5.2 which includes a piece of x86_64 assembly code (that looks to work by passing it as extra_objects to the Extensions).
This still needs testing on architecture different from x86_64.

closes #186

@t20100 t20100 added this to the 4.0.0 milestone Nov 8, 2022
@t20100 t20100 marked this pull request as ready for review November 8, 2022 15:22
@t20100
Copy link
Member Author

t20100 commented Nov 8, 2022

The assembly code is only enabled under linux and macos and explicitly disabled otherwise.
There is further checks with C macros to check if it is actually there.

Tested also on Power9 and macos ARM.

One issue may be for generic builds (manylinux, conda): This assembly code uses BMI2 instructions which are apparently available starting from Haswell Intel processors and Excavator AMD processors, so available since about 10 years.

We can:

  • Disable it all the time
  • Add (yet another) env. var. to configure the build (e.g., HDF5PLUGIN_BMI2).

@t20100
Copy link
Member Author

t20100 commented Nov 8, 2022

I added an env. var. HDF5PLUGIN_BMI2=True|False (default True) to allow disabling this.

Ready for review.

@t20100 t20100 requested a review from vasole November 8, 2022 16:09
Copy link
Member

@vasole vasole left a comment

Choose a reason for hiding this comment

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

LGTM. I like the conservative approach concerning assembler code and windows...

@vasole vasole merged commit d97db94 into silx-kit:main Nov 8, 2022
@t20100 t20100 deleted the update-blosc branch November 8, 2022 17:47
@t20100
Copy link
Member Author

t20100 commented Nov 8, 2022

I like the conservative approach concerning assembler code and windows

If you know how to handle assembly code in build through setuptools on Windows, I'd take it.

@t20100
Copy link
Member Author

t20100 commented Nov 9, 2022

From a quick look (I haven't tested anything), Visual Studio does not seems to support assembly code by default.
From what I've seen, one need to install an extra tool like nasm to compile the assembly code first.
With gcc and clang, one can pass the .S file directly at link time: thus the use of extra_objects in this PR and that I disabled it elsewhere including Windows.
We can consider enabling it for Windows when it is not using Visual Studio, but it is really a corner case and not worth the effort to me.

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.

Build issue on macos12.6
2 participants