doc: Recommend clang-cl when building on Windows

hebasto commented at 8:12 pm on June 4, 2025: member

There are several reasons to prefer clang-cl over MSVC, such as improved security and performance.

Below are the benchmark results for the master branch @ 201b2b8f06eb2daa5342c1fe5a14cb9934773cc3:

using MSVC:

 0Benchmark                     ,    Min(us)    ,    Avg(us)    ,    Max(us)    
 1
 2ecdsa_verify                  ,    66.0       ,    71.0       ,   113.0    
 3ecdsa_sign                    ,    37.0       ,    37.1       ,    37.5    
 4ec_keygen                     ,    28.5       ,    28.9       ,    29.0    
 5ecdh                          ,    66.0       ,    66.2       ,    67.0    
 6ecdsa_recover                 ,    67.0       ,    74.9       ,   123.0    
 7schnorrsig_sign               ,    30.0       ,    30.3       ,    30.5    
 8schnorrsig_verify             ,    66.5       ,    70.6       ,   104.0    
 9ellswift_encode               ,    17.5       ,    17.9       ,    18.0    
10ellswift_decode               ,    14.5       ,    15.3       ,    19.0    
11ellswift_keygen               ,    55.0       ,    56.4       ,    63.5    
12ellswift_ecdh                 ,    72.5       ,    73.5       ,    79.5

using clang-cl:

 0Benchmark                     ,    Min(us)    ,    Avg(us)    ,    Max(us)    
 1
 2ecdsa_verify                  ,    41.0       ,    47.5       ,   100.0    
 3ecdsa_sign                    ,    27.0       ,    27.2       ,    27.5    
 4ec_keygen                     ,    19.0       ,    19.3       ,    19.5    
 5ecdh                          ,    42.0       ,    42.4       ,    43.0    
 6ecdsa_recover                 ,    41.5       ,    45.7       ,    80.0    
 7schnorrsig_sign               ,    20.0       ,    20.5       ,    20.5    
 8schnorrsig_verify             ,    41.5       ,    45.5       ,    77.5    
 9ellswift_encode               ,    13.0       ,    13.0       ,    13.0    
10ellswift_decode               ,    10.0       ,    10.4       ,    10.5    
11ellswift_keygen               ,    38.5       ,    39.1       ,    41.5    
12ellswift_ecdh                 ,    47.0       ,    48.5       ,    59.0

On my local machine, the “Release” build configuration:

using MSVC:

 0> .\build-msvc\bin\Release\bench.exe
 1Benchmark                     ,    Min(us)    ,    Avg(us)    ,    Max(us)
 2
 3ecdsa_verify                  ,    81.2       ,    90.6       ,   102.0
 4ecdsa_sign                    ,    46.5       ,    48.6       ,    52.9
 5ec_keygen                     ,    31.6       ,    34.8       ,    36.2
 6ecdh                          ,    73.0       ,    76.4       ,    79.5
 7schnorrsig_sign               ,    32.1       ,    34.4       ,    35.8
 8schnorrsig_verify             ,    74.6       ,    76.2       ,    79.8
 9ellswift_encode               ,    33.4       ,    34.0       ,    34.8
10ellswift_decode               ,    14.9       ,    15.5       ,    17.1
11ellswift_keygen               ,    64.5       ,    65.6       ,    67.1
12ellswift_ecdh                 ,    78.3       ,    80.7       ,    90.1

using clang-cl:

 0> .\build-clangcl\bin\Release\bench.exe
 1Benchmark                     ,    Min(us)    ,    Avg(us)    ,    Max(us)
 2
 3ecdsa_verify                  ,    40.3       ,    40.6       ,    40.9
 4ecdsa_sign                    ,    30.6       ,    30.9       ,    31.3
 5ec_keygen                     ,    21.2       ,    21.3       ,    21.5
 6ecdh                          ,    41.5       ,    42.4       ,    44.8
 7schnorrsig_sign               ,    22.5       ,    22.7       ,    22.8
 8schnorrsig_verify             ,    41.2       ,    41.4       ,    41.7
 9ellswift_encode               ,    20.3       ,    20.6       ,    20.8
10ellswift_decode               ,     8.50      ,     8.64      ,     8.76
11ellswift_keygen               ,    41.7       ,    42.0       ,    42.4
12ellswift_ecdh                 ,    45.1       ,    45.5       ,    46.3

real-or-random added the label user-documentation on Jun 5, 2025

real-or-random added the label build on Jun 5, 2025

fanquake commented at 8:42 am on June 5, 2025: member

Are benchmarks all that should be considered? From what I can see there’s there’s only a single clang-cl CI job in this repo; if we’re going to change the recommended way to build the library, then we should also replace the majority of the MSVC jobs, with clang-cl, to reflect that preference?

It would also be interesting to elaborate on why the performance of MSVC is so much worse than Clang.

hebasto commented at 10:33 am on June 5, 2025: member

It would also be interesting to elaborate on why the performance of MSVC is so much worse than Clang.

Several factors may contribute to the performance gap:

Different optimization strategies.
Differences in inlining heuristics.
A number of other compiler‑level details.

However, I haven’t compared the generated assembly code.

hebasto commented at 11:31 am on June 5, 2025: member

It would also be interesting to elaborate on why the performance of MSVC is so much worse than Clang.

Another factor is that clang-cl uses native 128-bit integer types, whereas MSVC relies on int128_struct.

hebasto commented at 12:19 pm on June 5, 2025: member

Although clang-cl supports inline assembly, as indicated by the configure summary:

0secp256k1 configure summary
1===========================
2<snip>
3Optional features:
4  assembly ............................ x86_64

I observed no benchmark difference between -DSECP256K1_ASM=x86_64 and -DSECP256K1_ASM=OFF.

hebasto commented at 1:09 pm on June 5, 2025: member

From what I can see there’s there’s only a single clang-cl CI job in this repo; if we’re going to change the recommended way to build the library, then we should also replace the majority of the MSVC jobs, with clang-cl, to reflect that preference?

Fair enough. More clang-cl CI tasks have been added.

in .github/workflows/ci.yml:608 in 7428961441 outdated

603@@ -604,8 +604,16 @@ jobs:
604             cpp_flags: '/DSECP256K1_MSVC_MULH_TEST_OVERRIDE'
605           - job_name: 'x86 (MSVC): Windows (VS 2022)'
606             cmake_options: '-A Win32'
607-          - job_name: 'x64 (MSVC): Windows (clang-cl)'
608-            cmake_options: '-T ClangCL'
609+          - job_name: 'x64 (MSVC): Windows (clang-cl, shared)'
610+            cmake_options: '-T ClangCL -DBUILD_SHARED_LIBS=ON'

real-or-random commented at 2:41 pm on June 5, 2025:

Naming: Shouldn’t this be: x64 (clang-cl): Windows (VS 2022, shared) then? It’s not a MSVC build.

(We could rework the naming of the jobs in general, it’s not very consistent, and it’s also not very helpful because the long names are truncated too early in the sidebar on https://github.com/bitcoin-core/secp256k1/pull/1681/checks, but that’s a separate issue.)

hebasto commented at 3:01 pm on June 5, 2025:

Thanks! Renamed.

real-or-random commented at 2:44 pm on June 5, 2025: contributor

if we’re going to change the recommended way to build the library,

I doubt that MSVC was ever supposed to be a recommendation. (I wouldn’t recommend MSVC to anyone…) It’s just an example, and it’s the default compiler in VS as far as I understand.

then we should also replace the majority of the MSVC jobs, with clang-cl, to reflect that preference?

But sure, I can’t hurt to test more in clang-cl. Though we have a good clang coverage already, just not on Windows. But since we hardly use the C stdlib or syscalls, that’s still a good coverage.

hebasto force-pushed on Jun 5, 2025

in README.md:141 in 9243e21997 outdated

136@@ -137,11 +137,13 @@ To cross compile for Android with [NDK](https://developer.android.com/ndk/guides
137 
138 To build on Windows with Visual Studio, a proper [generator](https://cmake.org/cmake/help/latest/manual/cmake-generators.7.html#visual-studio-generators) must be specified for a new build tree.
139 
140+Using clang-cl is recommended, as it tends to produce better-performing binaries compared to MSVC.
141+

real-or-random commented at 7:45 pm on June 5, 2025:

0Using clang-cl is recommended.

There are more reasons to prefer clang, e.g., the (forgotten) #1164 or just the fact its output more testing (though mostly on Linux). But it’s difficult to explain in one or two sentences, and I think it’s ok not to explain it here.

edit: I also suggest dropping the empty line after this sentence, but somehow the GitHub suggestion feature doesn’t understand this.

hebasto commented at 8:08 pm on June 5, 2025:

Thanks! Updated, including dropping the empty line.

real-or-random commented at 7:47 pm on June 5, 2025: contributor

Concept ACK

hebasto force-pushed on Jun 5, 2025

doc: Recommend clang-cl when building on Windows 1f92d3b384

ci: Add more tests for clang-cl db0c5f9439

hebasto force-pushed on Jun 5, 2025

real-or-random commented at 8:27 pm on June 5, 2025: contributor

@sipa What do you think?

doc: Recommend clang-cl when building on Windows #1681