build: Fix regression in “ARMv8 CRC32 intrinsics” test #28919

1. 
   hebasto commented at 1:49 pm on November 20, 2023: member

   In the master branch, the aarch64 binaries lack support for CRC32 intrinsics.

   The vmull_p64 is a part of the Crypto extensions from the ACLE. They are optional extensions, so they get enabled with a +crypto for architecture flags.

   The regression was introduced in #26183 (v25.0).

   The ./configure script log excerpts:

   • the master branch @ d752349029ec7a76f1fd440db2ec2e458d0f3c99:

   0checking whether C++ compiler accepts -march=armv8-a+crc... yes
   1checking whether C++ compiler accepts -march=armv8-a+crypto... yes
   2checking for ARMv8 CRC32 intrinsics... no
   3checking for ARMv8 SHA-NI intrinsics... yes
   

   • this PR:

   0checking whether C++ compiler accepts -march=armv8-a+crc+crypto... yes
   1checking whether C++ compiler accepts -march=armv8-a+crypto... yes
   2checking for ARMv8 CRC32 intrinsics... yes
   3checking for ARMv8 SHA-NI intrinsics... yes
   

   Guix build:

   0x86_64
   12afd81f540c6d3b36ff305e88bafe935e4272cd3efef3130aa69d49a0522541b  guix-build-228d6a2969e4/output/aarch64-linux-gnu/SHA256SUMS.part
   26c704d6d30d495adb3fb86befdb500eb389a02c1167163f14ab5c3c3e630e6b3  guix-build-228d6a2969e4/output/aarch64-linux-gnu/bitcoin-228d6a2969e4-aarch64-linux-gnu-debug.tar.gz
   3e4419963c9c0d99adc4e38538900b648f2c14f793b60c8ee2e6f5acc9d3fadd3  guix-build-228d6a2969e4/output/aarch64-linux-gnu/bitcoin-228d6a2969e4-aarch64-linux-gnu.tar.gz
   47d11052b6bd28cdf26d5f2a4987f02d32c93a061907bcd048fb6d161a0466ca9  guix-build-228d6a2969e4/output/dist-archive/bitcoin-228d6a2969e4.tar.gz
   

2. build: Fix regression in "ARMv8 CRC32 intrinsics" test

   The `vmull_p64` is a part of the Crypto extensions from the ACLE. They
   are optional extensions, so they get enabled with a `+crypto` for
   architecture flags.

   228d6a2969
3. hebasto added the label Build system on Nov 20, 2023
4. 
   DrahtBot commented at 1:49 pm on November 20, 2023: contributor

   The following sections might be updated with supplementary metadata relevant to reviewers and maintainers.

   Code Coverage

   For detailed information about the code coverage, see the test coverage report.

   Reviews

   See the guideline for information on the review process.

   Type	Reviewers
   ACK	TheCharlatan

   If your review is incorrectly listed, please react with 👎 to this comment and the bot will ignore it on the next update.

5. 
   TheCharlatan commented at 11:31 am on November 21, 2023: contributor

   ACK 228d6a2969e4fcee573c9df7aad31550eab9c8d4

   I checkd that the bianry now contains both crc and crypto intrinsic opcodes.

   Guix build (x86_64 and aarch64):

   02afd81f540c6d3b36ff305e88bafe935e4272cd3efef3130aa69d49a0522541b  guix-build-228d6a2969e4/output/aarch64-linux-gnu/SHA256SUMS.part
   16c704d6d30d495adb3fb86befdb500eb389a02c1167163f14ab5c3c3e630e6b3  guix-build-228d6a2969e4/output/aarch64-linux-gnu/bitcoin-228d6a2969e4-aarch64-linux-gnu-debug.tar.gz
   2e4419963c9c0d99adc4e38538900b648f2c14f793b60c8ee2e6f5acc9d3fadd3  guix-build-228d6a2969e4/output/aarch64-linux-gnu/bitcoin-228d6a2969e4-aarch64-linux-gnu.tar.gz
   

6. TheCharlatan approved
7. 
   fanquake commented at 5:08 pm on November 22, 2023: member

   The vmull_p64 is a part of the Crypto extensions from the ACLE. They are optional extensions, so they get enabled with a +crypto for architecture flags.

   I guess this isn’t true for all aarch64, as the checks currently work fine, for example, on Apple arm64 (M1):

   0checking for AVX2 intrinsics... no
   1checking for x86 SHA-NI intrinsics... no
   2checking whether C++ compiler accepts -march=armv8-a+crc... yes
   3checking whether C++ compiler accepts -march=armv8-a+crypto... yes
   4checking for ARMv8 CRC32 intrinsics... yes
   5checking for ARMv8 SHA-NI intrinsics... yes
   

8. 
   hebasto commented at 5:11 pm on November 22, 2023: member

   The vmull_p64 is a part of the Crypto extensions from the ACLE. They are optional extensions, so they get enabled with a +crypto for architecture flags.

   I guess this isn’t true for all aarch64, as the checks currently work fine, for example, on Apple arm64 (M1)

   Right. Mind suggesting a more precise wording?

9. 
   fanquake commented at 5:16 pm on November 22, 2023: member

   I guess this isn’t true for all aarch64, as the checks currently work fine, for example, on Apple arm64 (M1):

   Actually, I think this is just an Apple special case, and because Apple Clang just enables +crypto and +crc as part of the default compile flags.

10. fanquake merged this on Nov 22, 2023
11. fanquake closed this on Nov 22, 2023

    ---

12. 
    hebasto commented at 5:22 pm on November 22, 2023: member

    Is it worth backporting to 26.x?
13. fanquake referenced this in commit 6045f38dc8 on Nov 22, 2023
14. 
    fanquake commented at 5:22 pm on November 22, 2023: member

    Backported in #28872.
15. fanquake referenced this in commit 180565266a on Nov 22, 2023
16. hebasto deleted the branch on Nov 22, 2023
17. fanquake referenced this in commit e4fef4ae65 on Nov 22, 2023
18. hebasto referenced this in commit 02392e7062 on Nov 27, 2023
19. hebasto referenced this in commit 399642ae2b on Dec 4, 2023
20. fanquake referenced this in commit b34f54db08 on Dec 11, 2023
21. bitcoin locked this on Dec 3, 2024

Contributors
hebasto DrahtBot TheCharlatan fanquake

Labels
Build system