Optimise openjph build - #410
Conversation
| "-msse", | ||
| "-msse2", | ||
| "-mssse3", | ||
| "-mavx", | ||
| "-mavx2", | ||
| "-mavx512f", | ||
| "-mavx512cd", |
There was a problem hiding this comment.
It just expects the compiler to support those, which sounds fine for me as of today.
91aae07 to
d2ec22a
Compare
11c0535 to
5f80275
Compare
5f80275 to
cc7b8cd
Compare
t20100
left a comment
t20100
left a comment
There was a problem hiding this comment.
This again adds some complexity to the setup.py but for a good gain of performance
| simd_cflags = { | ||
| "sse": ["-msse"], | ||
| "sse2": ["-msse2"], | ||
| "ssse3": ["-mssse3"], | ||
| "avx": ["-mavx", "/arch:AVX"], | ||
| "avx2": ["-mavx2", "/arch:AVX2"], | ||
| "avx512": ["-mavx512f", "-mavx512cd", "/arch:AVX512"], | ||
| } |
There was a problem hiding this comment.
SIMD flags must be passed only to the relevant files
| "avx2": ["-mavx2", "/arch:AVX2"], | ||
| "avx512": ["-mavx512f", "-mavx512cd", "/arch:AVX512"], | ||
| } | ||
| simd_macros = [] |
There was a problem hiding this comment.
no SIMD related macros so all are enabled: there is a runtime probe to get the host capabilities
| def portable_sources(pattern): | ||
| macros = [("_FILE_OFFSET_BITS", 64)] + simd_macros | ||
|
|
||
| source_cflags = [ |
There was a problem hiding this comment.
Using a custom construct to allow passing specific flags to only some source files: setuptools clib support is limited: we'd better use cmake, but this is out of the scope of this PR IMO
| include_dir = pkgconfig.variables(libname).get("includedir", None) | ||
| extra_link_args = pkgconfig.libs(libname).split(" ") | ||
| for arg in extra_link_args: | ||
| for arg in extra_link_args[:]: |
There was a problem hiding this comment.
small fix: copy list before loop since it is modified in the loop
|
|
||
|
|
||
| PLUGIN_LIB_DEPENDENCIES["htj2k"] = ("openjph",) | ||
| PLUGIN_LIB_DEPENDENCIES["zfp"] = ("zfp",) |
There was a problem hiding this comment.
Re-order the code to match the filter defined in previous function
|
Ready for review |
| build_info["cflags"] = self._update_cflags( | ||
| cflags + list(config.compile_args) | ||
| ) |
There was a problem hiding this comment.
Can we have flags existing in both cflags and and config.compile_args. Which mean should we take care of this ? (using set instead of list).
There was a problem hiding this comment.
It's possible, but it shouldn't happen.
|
|
||
|
|
||
| PLUGIN_LIB_DEPENDENCIES["zfp"] = ("zfp",) | ||
| PLUGIN_LIB_DEPENDENCIES["htj2k"] = ("openjph",) |
There was a problem hiding this comment.
miss those one during the first review sorry :(
2 participants
This PR use SIMD source code to build openjh.
This expects the compilers to support SIMD up to avx512, which is available since >8 years old in gcc, clang and msvc, so fine for me.