Skip to content

Conversation

hebasto
Copy link
Member

@hebasto hebasto commented May 18, 2025

The missed flags were noticed when building with clang-cl on Windows.

@DrahtBot
Copy link
Contributor

DrahtBot commented May 18, 2025

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

Code Coverage & Benchmarks

For details see: https://corecheck.dev/bitcoin/bitcoin/pulls/32550.

Reviews

See the guideline for information on the review process.
A summary of reviews will appear here.

@hebasto hebasto marked this pull request as draft May 18, 2025 14:52
@fanquake
Copy link
Member

Just roll this into #32551?

@hebasto
Copy link
Member Author

hebasto commented May 19, 2025

Just roll this into #32551?

Done.

@hebasto hebasto closed this May 19, 2025
fanquake added a commit that referenced this pull request May 22, 2025
… from `bitcoin-build-config.h`

800b7cc cmake: Add missed `SSE41_CXXFLAGS` (Hennadii Stepanov)
028476e cmake: Remove `ENABLE_ARM_SHANI` from `bitcoin-build-config.h` (Hennadii Stepanov)
1e90052 cmake: Remove `ENABLE_X86_SHANI` from `bitcoin-build-config.h` (Hennadii Stepanov)
8689628 cmake: Remove `ENABLE_AVX2` from `bitcoin-build-config.h` (Hennadii Stepanov)
a8e2342 cmake: Remove `ENABLE_SSE41` from `bitcoin-build-config.h` (Hennadii Stepanov)

Pull request description:

  `ENABLE_{SSE41,AVX2,X86_SHANI,ARM_SHANI}`  are already conditionally defined for the [`bitcoin_crypto`](https://github.com/bitcoin/bitcoin/blob/master/src/crypto/CMakeLists.txt) target, and they are not used by any other targets. Defining them globally in `bitcoin-build-config.h` is therefore redundant.

  Additionally, the previously missing `SSE41_CXXFLAGS` variable has been [added](#32550 (comment)).

ACKs for top commit:
  fanquake:
    ACK 800b7cc

Tree-SHA512: da792a0b780c67b432b09c9288ca98d62545315c721fed13510d1c11f8bb0cddd9a4ed7a009b4d052471dda19d0641bbc1eae4805fc306d23bf9b4ef510089c8
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants