fix: Add back support for ppc64le #35
Conversation
matthewfeickert
commented
Dec 21, 2024
•
matthewfeickert
commented
- ppc64 little-endian is a supported architecture both on conda-forge and used in HPC systems. Add back support for it by only disabling libquadmath on aarch64 builds and by checking for the ppc64 in header files.
- Amends parts of PR Add support for aarch64 #34.
- c.f. qcdloop v2.1.0 conda-forge/qcdloop-feedstock#8
* ppc64 little-endian is a supported architecture both on conda-forge and used in HPC systems. Add back support for it by only disabling libquadmath on aarch64 builds and by checking for the ppc64 in header files.
matthewfeickert
force-pushed
the
fix/patch-back-ppc64le-support
branch
from
a4c0e09 to
6e3aac0
Compare
|
@scarrazza this is ready for review. c.f. conda-forge/qcdloop-feedstock#8 |
CMakeLists.txt
Outdated
| # Only enable quadmath for non-aarch64 as aarch64 supports float128 and so | ||
| # doesn't have libquadmath | ||
| IF(NOT ${CMAKE_SYSTEM_PROCESSOR} MATCHES "aarch64") |
There was a problem hiding this comment.
@henryiii is there a more preferred way to do this? Or is just checking for aarch64 somewhere good enough?
There was a problem hiding this comment.
Hm. From conda-forge/qcdloop-feedstock#7 it seems like maybe ${CMAKE_SYSTEM_PROCESSOR} isn't what is desired when cross-compiling, as when cross-compiling for aarch64 on x86 it is attempting to link against libquadmath.
* updated v2.1.0 * MNT: Add patch to get ppc64le support working - c.f. scarrazza/qcdloop#35 - This patch can be removed in subsequent releases. --------- Co-authored-by: Matthew Feickert <matthew.feickert@cern.ch>
* Compare to a regex of aarch64 or arm.
| @@ -17,7 +17,7 @@ namespace std { | |||
| seed ^= hasher(v) + 0x9e3779b9 + (seed << 6) + (seed >> 2); | |||
| } | |||
|
|
|||
| #if defined(__x86_64__) || defined(__i386__) | |||
There was a problem hiding this comment.
I would limit it here just to
__ppc64__
and
__ppc__
for the sake of consistency, see https://sourceforge.net/p/predef/wiki/Architectures/
There was a problem hiding this comment.
@andriish Can you elaborate on what you mean by "consistency"? I'm not sure what you're trying to communicate and the link isn't informative. I'm sure to you it is clear, but I'm going to need you to write out your point.
| @@ -66,7 +66,9 @@ configure_file( | |||
| "${PROJECT_SOURCE_DIR}/src/qcdloop.pc.in" | |||
| "${PROJECT_SOURCE_DIR}/src/qcdloop.pc" | |||
| ) | |||
| IF(${CMAKE_SYSTEM_PROCESSOR} MATCHES "x86_64") | |||
There was a problem hiding this comment.
OK, Something like
IF(${CMAKE_HOST_SYSTEM_PROCESSOR} MATCHES "x86_64|i686|ppc64|ppc64le|ppc|ppcle" )
would be better.
But it is important to list supported architectures explicitly, and not "everything but...".
There was a problem hiding this comment.
@andriish Can you explain more on why? How is this different or better? Is there another architecture that supports float128 that you know of? Or do you mean that there could be other architectures not considered here that also don't support libquadmath and these would get picked up erroneously here?
