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

Fix cmake detection of risc-v (32 and 64 bit) #942

Merged
merged 1 commit into from
May 6, 2021

Conversation

Civil
Copy link
Contributor

@Civil Civil commented May 2, 2021

Add a case to detect risc-v architectures.

Fixes #941

cmake/detect-arch.cmake Outdated Show resolved Hide resolved
Copy link
Member

@nmoinvaz nmoinvaz left a comment

Choose a reason for hiding this comment

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

LGTM

@codecov
Copy link

codecov bot commented May 2, 2021

Codecov Report

Merging #942 (ad38bc7) into develop (fc7719d) will decrease coverage by 0.50%.
The diff coverage is n/a.

Impacted file tree graph

@@             Coverage Diff             @@
##           develop     #942      +/-   ##
===========================================
- Coverage    77.42%   76.92%   -0.51%     
===========================================
  Files           74       74              
  Lines         8307     8306       -1     
  Branches      1374     1374              
===========================================
- Hits          6432     6389      -43     
- Misses        1346     1384      +38     
- Partials       529      533       +4     
Flag Coverage Δ
macos_clang 68.55% <ø> (ø)
macos_gcc 67.45% <ø> (ø)
ubuntu_clang 73.61% <ø> (+0.19%) ⬆️
ubuntu_clang_debug 73.65% <ø> (-0.07%) ⬇️
ubuntu_clang_inflate_allow_invalid_dist 73.34% <ø> (+0.19%) ⬆️
ubuntu_clang_inflate_strict 73.60% <ø> (+0.19%) ⬆️
ubuntu_clang_mmap 73.31% <ø> (+0.19%) ⬆️
ubuntu_clang_msan 73.61% <ø> (+0.19%) ⬆️
ubuntu_gcc 73.00% <ø> (+0.03%) ⬆️
ubuntu_gcc_aarch64 73.82% <ø> (-0.44%) ⬇️
ubuntu_gcc_aarch64_compat_no_opt 73.32% <ø> (-0.46%) ⬇️
ubuntu_gcc_aarch64_no_acle 73.64% <ø> (-0.44%) ⬇️
ubuntu_gcc_aarch64_no_neon 73.38% <ø> (-0.45%) ⬇️
ubuntu_gcc_armhf 71.76% <ø> (-0.44%) ⬇️
ubuntu_gcc_armhf_compat_no_opt 73.31% <ø> (-0.49%) ⬇️
ubuntu_gcc_armhf_no_acle 72.50% <ø> (-0.45%) ⬇️
ubuntu_gcc_armhf_no_neon 74.49% <ø> (-0.45%) ⬇️
ubuntu_gcc_armsf 71.74% <ø> (-0.44%) ⬇️
ubuntu_gcc_armsf_compat_no_opt 73.31% <ø> (-0.49%) ⬇️
ubuntu_gcc_compat_no_opt 74.42% <ø> (-0.46%) ⬇️
ubuntu_gcc_mingw_i686 0.00% <ø> (ø)
ubuntu_gcc_mingw_x86_64 0.00% <ø> (ø)
ubuntu_gcc_no_avx2 74.52% <ø> (-0.42%) ⬇️
ubuntu_gcc_no_pclmulqdq 71.46% <ø> (+0.05%) ⬆️
ubuntu_gcc_no_sse2 72.54% <ø> (-1.37%) ⬇️
ubuntu_gcc_no_sse4 71.42% <ø> (+0.05%) ⬆️
ubuntu_gcc_o3 73.79% <ø> (+0.94%) ⬆️
ubuntu_gcc_osb 75.98% <ø> (+0.15%) ⬆️
ubuntu_gcc_ppc 74.80% <ø> (-0.46%) ⬇️
ubuntu_gcc_ppc64 75.56% <ø> (-0.46%) ⬇️
ubuntu_gcc_ppc64le 74.32% <ø> (-0.46%) ⬇️
ubuntu_gcc_s390x 73.23% <ø> (-0.42%) ⬇️
ubuntu_gcc_sparc64 76.03% <ø> (-0.46%) ⬇️
win64_gcc 70.39% <ø> (ø)
win64_gcc_compat_no_opt 73.88% <ø> (ø)

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files Coverage Δ
match_tpl.h 71.42% <0.00%> (-28.58%) ⬇️
zutil_p.h 66.66% <0.00%> (-19.05%) ⬇️
inftrees.c 81.29% <0.00%> (-16.55%) ⬇️
chunkset_tpl.h 99.02% <0.00%> (+3.88%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update fc7719d...ad38bc7. Read the comment docs.

@Dead2
Copy link
Member

Dead2 commented May 2, 2021

Detection of the architecture as 32- or 64-bit is still useful, so I really didn't want you to remove that (from the .c file). Only basearch should be the common for the two.
Would you please revert the change to the .c file (and modify the if in .cmake)? 👍

You can have a look at how the x86 detection is done, with differing arches but also having a common basearch. Both are used in CMakeLists.txt for different things.

@Civil
Copy link
Contributor Author

Civil commented May 2, 2021

@Dead2 done.

I've also modified it a bit, as after a second thought, detecting riscv64 by pointer size seems to be wrong, and by https://github.com/riscv/riscv-toolchain-conventions correct way should be to check for __riscv_xlen

@Dead2
Copy link
Member

Dead2 commented May 3, 2021

@Civil Are you sure you pushed the update? It looks unchanged to me. But not entirely sure whether github is messing with me or not.

@Civil
Copy link
Contributor Author

Civil commented May 3, 2021

@Dead2 indeed, sorry about that.

@nmoinvaz nmoinvaz added Architecture Architecture specific Build Env labels May 3, 2021
@Civil Civil requested a review from mtl1979 May 4, 2021 21:58
Add a case to detect risc-v architectures.

Fixes zlib-ng#941
@Dead2 Dead2 merged commit 81f1c8a into zlib-ng:develop May 6, 2021
Dead2 added a commit that referenced this pull request May 8, 2021
- Include porting guide in release packages #917
- Documentation improvements #913 #949
- Added Windows ARM binaries in release packages #916
- Fix crash on ARMv7 #927
- Fix building on FreeBSD #921
- Fix building with musl on aarch64 #936
- Fix ARM float-abi detection #918
- Fix cmake detection of risc-v architectures #942
- Minor buildsystem fixes #922 #924 #933 #938 #950
- Improve zlib-compat build #915 #944
- CI/Test improvements #926 #929 #925 #937 #939 #940
Dead2 added a commit that referenced this pull request May 8, 2021
- Include porting guide in release packages #917
- Documentation improvements #913 #949
- Added Windows ARM binaries in release packages #916
- Fix crash on ARMv7 #927
- Fix building on FreeBSD #921
- Fix building with musl on aarch64 #936
- Fix ARM float-abi detection #918
- Fix cmake detection of risc-v architectures #942
- Minor buildsystem fixes #922 #924 #933 #938 #950
- Improve zlib-compat build #915 #944
- CI/Test improvements #926 #929 #927 #937 #939 #940
@Dead2 Dead2 mentioned this pull request May 8, 2021
Dead2 added a commit that referenced this pull request May 8, 2021
- Include porting guide in release packages #917
- Documentation improvements #913 #949
- Added Windows ARM binaries in release packages #916
- Fix crash on ARMv7 #927
- Fix building on FreeBSD #921
- Fix building with musl on aarch64 #936
- Fix ARM float-abi detection #918
- Fix cmake detection of risc-v architectures #942
- Minor buildsystem fixes #922 #924 #933 #938 #950
- Improve zlib-compat build #915 #944
- CI/Test improvements #926 #929 #927 #937 #939 #940
Dead2 added a commit that referenced this pull request May 9, 2021
- Include porting guide in release packages #917
- Documentation improvements #913 #949
- Added Windows ARM binaries in release packages #916
- Fix crash on ARMv7 #927
- Fix building on FreeBSD #921
- Fix building with musl on aarch64 #936 #952
- Fix ARM float-abi detection #918
- Fix cmake detection of risc-v architectures #942
- Minor buildsystem fixes #922 #924 #933 #938 #950
- Improve zlib-compat build #915 #944
- CI/Test improvements #926 #929 #927 #937 #939 #940
Dead2 added a commit that referenced this pull request May 13, 2021
- Include porting guide in release packages #917
- Documentation improvements #913 #949
- Added Windows ARM binaries in release packages #916
- Fix crash on ARMv7 #927
- Fix building on FreeBSD #921
- Fix building with musl on aarch64 #936 #952
- Fix ARM float-abi detection #918
- Fix cmake detection of risc-v architectures #942
- Minor buildsystem fixes #922 #924 #933 #938 #950
- Improve zlib-compat build #915 #944
- CI/Test improvements #926 #929 #927 #937 #939 #940
gsjaardema pushed a commit to gsjaardema/zlib-ng that referenced this pull request May 13, 2021
- Include porting guide in release packages zlib-ng#917
- Documentation improvements zlib-ng#913 zlib-ng#949
- Added Windows ARM binaries in release packages zlib-ng#916
- Fix crash on ARMv7 zlib-ng#927
- Fix building on FreeBSD zlib-ng#921
- Fix building with musl on aarch64 zlib-ng#936 zlib-ng#952
- Fix ARM float-abi detection zlib-ng#918
- Fix cmake detection of risc-v architectures zlib-ng#942
- Minor buildsystem fixes zlib-ng#922 zlib-ng#924 zlib-ng#933 zlib-ng#938 zlib-ng#950
- Improve zlib-compat build zlib-ng#915 zlib-ng#944
- CI/Test improvements zlib-ng#926 zlib-ng#929 zlib-ng#927 zlib-ng#937 zlib-ng#939 zlib-ng#940
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Architecture Architecture specific Build Env
Projects
None yet
Development

Successfully merging this pull request may close these issues.

riscv64 support in cmake is broken
4 participants