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

do not use __float128 on non-intel platforms #296

Closed
wants to merge 8 commits into from

Conversation

brownj85
Copy link
Collaborator

@brownj85 brownj85 commented Nov 9, 2021

Context:
Aliasing of type __float128 was resulting in build failures on aarch64. This is because the GCC
extension that provides this type is not available on aarch64.

Description of the Change:
Adds a compiler check in permanent.hpp to prevent aliasing __float128 if the platform is not i386 or AMD64. Other platforms will fall back to double precision.

Benefits:
Build will succeed on aarch64.

Possible Drawbacks:
To keep the compiler check simple, it will exclude certain exotic architectures that do support __float128, such as PowerPC.

Related GitHub Issues:
#290

@brownj85 brownj85 mentioned this pull request Nov 9, 2021
@codecov
Copy link

codecov bot commented Nov 9, 2021

Codecov Report

Merging #296 (293ef09) into master (a61c85d) will not change coverage.
The diff coverage is n/a.

❗ Current head 293ef09 differs from pull request most recent head 45fef80. Consider uploading reports for the commit 45fef80 to get more accurate results

@@            Coverage Diff            @@
##            master      #296   +/-   ##
=========================================
  Coverage   100.00%   100.00%           
=========================================
  Files           21        21           
  Lines         1451      1458    +7     
=========================================
+ Hits          1451      1458    +7     
Impacted Files Coverage Δ
thewalrus/__init__.py 100.00% <0.00%> (ø)
thewalrus/quantum/fock_tensors.py 100.00% <0.00%> (ø)
thewalrus/_hermite_multidimensional.py 100.00% <0.00%> (ø)

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 a61c85d...45fef80. Read the comment docs.

@sduquemesa sduquemesa added the C++ label Nov 9, 2021
@sduquemesa sduquemesa marked this pull request as ready for review November 10, 2021 13:59
@sduquemesa sduquemesa linked an issue Nov 10, 2021 that may be closed by this pull request
@sduquemesa
Copy link
Contributor

Hi @brownj85, thanks for your contribution! 🚀
I have created a new check so that we test if the build on aarch64 works successfully.

@nquesada
Copy link
Collaborator

@brownj85 @sduquemesa note that in the development version of the walrus we do not use C++ for permanents anymore. I think this PR can be closed and just wait til the next release.

@thisac
Copy link
Contributor

thisac commented Jan 5, 2022

Closing this since the dependency on C++ has been removed in the latest release.

@thisac thisac closed this Jan 5, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

FTBFS 0.16.2 on aarch64
4 participants