cmake: Add option to generate precomputed files #1791

pull hebasto wants to merge 2 commits into bitcoin-core:master from hebasto:251220-cmake-precomputed changing 8 files +68 −9
  1. hebasto commented at 8:02 PM on December 20, 2025: member

    Using the CMake-based build system enables smoother integration with parent projects by providing both CMake package configuration files and the traditional libsecp256k1.pc.

    However, packagers on some systems, such as Gentoo and Guix, insist on recreating the precomputed files, which is not currently supported by the CMake-based build system.

    This PR addresses the issue by introducing a new SECP256K1_BUILD_PRECOMPUTED option (OFF by default) that generates precomputed_ecmult.c and precomputed_ecmult_gen.c in the build tree. Those files are then compiled instead of the ones checked into the source tree.

    Here is an example:

    $ cmake -B build -G Ninja -DSECP256K1_BUILD_PRECOMPUTED=ON
    $ ninja -C build src/precomputed_ecmult.c src/precomputed_ecmult_gen.c | cat
    ninja: Entering directory `build'
    [1/6] Building C object src/CMakeFiles/secp256k1_precompute_ecmult.dir/precompute_ecmult.c.o
    [2/6] Building C object src/CMakeFiles/secp256k1_precompute_ecmult_gen.dir/precompute_ecmult_gen.c.o
    [3/6] Linking C executable secp256k1_precompute_ecmult_gen
    [4/6] Linking C executable secp256k1_precompute_ecmult
    [5/6] Generating precomputed_ecmult_gen.c
    [6/6] Generating precomputed_ecmult.c
    

    When configuring with CMake 3.31 or newer, the built-in codegen target can be used to generate those files:

    $ cmake -B build -G Ninja -DSECP256K1_BUILD_PRECOMPUTED=ON
    $ cmake --build build -t codegen | cat
    [1/6] Building C object src/CMakeFiles/secp256k1_precompute_ecmult.dir/precompute_ecmult.c.o
    [2/6] Building C object src/CMakeFiles/secp256k1_precompute_ecmult_gen.dir/precompute_ecmult_gen.c.o
    [3/6] Linking C executable secp256k1_precompute_ecmult_gen
    [4/6] Linking C executable secp256k1_precompute_ecmult
    [5/6] Generating precomputed_ecmult_gen.c
    [6/6] Generating precomputed_ecmult.c
    
  2. hebasto force-pushed on Dec 20, 2025
  3. hebasto force-pushed on Dec 20, 2025
  4. kevkevinpal commented at 12:15 AM on December 24, 2025: contributor

    are we able to do this via ./configure?

    or is this meant to be only done for CMake?

  5. hebasto commented at 1:43 AM on December 24, 2025: member

    are we able to do this via ./configure?

    Yes. Run make clean-precomp && make precomp.

  6. real-or-random added the label feature on Jan 6, 2026
  7. real-or-random added the label build on Jan 6, 2026
  8. in src/CMakeLists.txt:70 in 5373c6aa6f
      69 | +      ${CMAKE_CURRENT_BINARY_DIR}/precomputed_ecmult_gen.c
      70 | +    COMMAND secp256k1_precompute_ecmult
      71 | +    COMMAND secp256k1_precompute_ecmult_gen
      72 | +    DEPENDS secp256k1_precompute_ecmult secp256k1_precompute_ecmult_gen
      73 | +    WORKING_DIRECTORY ${PROJECT_BINARY_DIR}
      74 | +  )
    


    real-or-random commented at 8:33 AM on January 6, 2026:

    Shouldn't these conceptually be two separate commands?


    hebasto commented at 6:13 PM on January 6, 2026:

    Thanks! Reworked.

  9. in src/CMakeLists.txt:55 in 5373c6aa6f
      54 | +  add_executable(secp256k1_precompute_ecmult EXCLUDE_FROM_ALL precompute_ecmult.c)
      55 | +  target_compile_definitions(secp256k1_precompute_ecmult PRIVATE VERIFY)
      56 | +  if(CMAKE_C_COMPILER_ID STREQUAL "MSVC")
      57 | +    # Disable warning C4334 "'operator': result of 32-bit shift implicitly converted to 64 bits (was 64-bit shift intended?)".
      58 | +    target_compile_options(secp256k1_precompute_ecmult PRIVATE /wd4334)
      59 | +  endif()
    


    real-or-random commented at 8:35 AM on January 6, 2026:

    In which line does this warning appear? Could we fix it in the code instead?


    hebasto commented at 6:13 PM on January 6, 2026:

    In which line does this warning appear?

    https://github.com/bitcoin-core/secp256k1/blob/2d9137ce9d7e576bfada2859292ecaa0887a6b8c/src/ecmult.h#L41

    Could we fix it in the code instead?

    Certainly. Fixed in 19529e70680a5edaa0210b74420693d8e689c7ac.


    real-or-random commented at 10:44 AM on January 7, 2026:

    I think conceptually the result of ECMULT_TABLE_SIZE should be a size_t because it is an array size. What do you think of this refactor instead? https://github.com/bitcoin-core/secp256k1/pull/1794


    hebasto commented at 12:53 PM on January 7, 2026:

    Rebased on the top of the first commit from #1794.

  10. real-or-random commented at 8:35 AM on January 6, 2026: contributor

    Concept ACK

  11. hebasto force-pushed on Jan 6, 2026
  12. hebasto commented at 6:14 PM on January 6, 2026: member

    @real-or-random

    Thank you for the review! Your feedback has been addressed.

  13. hebasto force-pushed on Jan 7, 2026
  14. real-or-random referenced this in commit 14e56970cb on Jan 27, 2026
  15. hebasto force-pushed on Jan 27, 2026
  16. hebasto commented at 11:26 AM on January 27, 2026: member

    Rebased on the top of the first commit from #1794.

    Rebased on top of the merged bitcoin-core/secp256k1#1794.

  17. theStack commented at 2:02 PM on June 5, 2026: contributor

    Concept ACK

    Tested quickly that

    $ cmake -B build -DSECP256K1_ECMULT_WINDOW_SIZE=16 -DSECP256K1_BUILD_PRECOMPUTED=ON && cmake --build build
    

    succeeds on the PR branch as expected, while on master

    $ cmake -B build -DSECP256K1_ECMULT_WINDOW_SIZE=16 && cmake --build build
    

    fails (error: #error configuration mismatch, invalid ECMULT_WINDOW_SIZE).

    The generated files precomputed_ecmult{,gen}.c files are created within the build directory. Given the out-of-tree build nature of CMake, I guess this is intended and there are no plans to support directly modifying the source tree? (Which is no big deal, developers can simply do a $ cp ./build/src/precomputed_ecmult*.c ./src/ after manually or just re-run the precompute binaries).

  18. hebasto commented at 2:11 PM on June 5, 2026: member

    Given the out-of-tree build nature of CMake, I guess this is intended and there are no plans to support directly modifying the source tree?

    That's correct. We have to assume the source tree might be strictly read-only.

  19. theStack approved
  20. theStack commented at 6:54 PM on June 8, 2026: contributor

    Light ACK 8b58c55e3d4b5e18a6bb626c5b55548b832965eb

    I'm not too familiar with the ins and outs of CMake, but the changes look good and I tested the new build setting both with higher ecmult window sizes (as shown above) and with different ecmult_gen CONFIGS table values (taken from https://github.com/theStack/secp256k1/commit/3421153be6ae8d8d88406d518c9e66ef4d49fb0a), and everything seems to work as expected.

    // EDIT: I guess this is worth a release note

  21. real-or-random added the label needs-changelog on Jun 9, 2026
  22. in CMakeLists.txt:144 in 8b58c55e3d
     140 | @@ -141,6 +141,8 @@ option(SECP256K1_BUILD_EXHAUSTIVE_TESTS "Build exhaustive tests." ON)
     141 |  option(SECP256K1_BUILD_CTIME_TESTS "Build constant-time tests." ${SECP256K1_VALGRIND})
     142 |  option(SECP256K1_BUILD_EXAMPLES "Build examples." OFF)
     143 |  
     144 | +option(SECP256K1_BUILD_PRECOMPUTED "Recreate the precomputed files." OFF)
    


    real-or-random commented at 9:18 AM on June 9, 2026:

    nit: Do you think it will add clarity to expand this to "Recreate the precomputed files in the build tree"?

    It's kind of implied that a CMake build creates stuff only in the build tree. But @theStack felt the need to point this out, so this may be surprising to users who don't really think about carefully (which includes almost everyone when it comes to build systems, I guess).


    hebasto commented at 11:04 AM on June 9, 2026:

    Thanks! Amended.

  23. hebasto force-pushed on Jun 9, 2026
  24. hebasto commented at 11:04 AM on June 9, 2026: member

    The most recent feedback has been addressed.

  25. theStack approved
  26. theStack commented at 2:28 PM on June 9, 2026: contributor

    re-ACK 8460c5902cf83d3b83e22021c7d73d6991347464

    Verified that the only change since my last review was the more detailed build option description (+"in the build tree"), which I agree makes sense.

  27. real-or-random commented at 3:17 PM on June 9, 2026: contributor

    fails (error: #error configuration mismatch, invalid ECMULT_WINDOW_SIZE).

    The truth is that it also outputs Try deleting precomputed_ecmult.c before the build. And this is not true for the approach in this PR. With Automake, we construct real dependencies, i.e., the main artifact depends on the pregenerated files. Here, this is just a build option.

    I think either approach is fine. I guess even mixing them is fine. Maybe we should just change the error message for now? "If this is an autotools build, try deleting precomputed_ecmult.c before the build. If this is a CMake build, pass -DSECP256K1_BUILD_PRECOMPUTED=ON to cmake."


    Further thoughts:

    I'm wondering if I like the "build option" approach as in this PR more. It's easier for those users who need it, and the code is simpler. And even for contributors working on changes to the generation binaries, they can just set -DSECP256K1_BUILD_PRECOMPUTED=ON in their cache.

    If others agree, this raises the question of whether we want to get rid of that stuff in autotools. It never worked 100%. And it seems that all users who want this feature use CMake. (@kevkevinpal Why did you ask about autotools? Do you need this feature in autotools?)

  28. in CMakePresets.json:11 in 8460c5902c
       6 | @@ -7,7 +7,8 @@
       7 |        "cacheVariables": {
       8 |          "SECP256K1_EXPERIMENTAL": "ON",
       9 |          "SECP256K1_ENABLE_MODULE_RECOVERY": "ON",
      10 | -        "SECP256K1_BUILD_EXAMPLES": "ON"
      11 | +        "SECP256K1_BUILD_EXAMPLES": "ON",
      12 | +        "SECP256K1_BUILD_PRECOMPUTED": "ON"
    


    real-or-random commented at 3:19 PM on June 9, 2026:

    I'm not entirely sure if we want this in dev mode. In autotools, we don't have it in dev mode.

    It's something a dev needs when working on the source for the precomputation binaries. But on the other hand, this is rare and this thing increases compile times. Maybe leave it off for now?


    hebasto commented at 4:02 PM on June 9, 2026:

    Thanks! Amended.


    hebasto commented at 4:16 PM on June 9, 2026:

    Maybe leave it off for now?

    That raises the question: which CI job should use -DSECP256K1_BUILD_PRECOMPUTED=ON?


    real-or-random commented at 11:02 AM on June 10, 2026:

    My first thought is "Why not all?"

  29. real-or-random commented at 3:20 PM on June 9, 2026: contributor

    Consider rebasing this. The branch is way behind (for our standards ;)).

  30. real-or-random requested review from Copilot on Jun 9, 2026
  31. ?
    copilot_work_started real-or-random
  32. in cmake/Precompute.cmake:12 in 8460c5902c outdated
       7 | +  add_custom_command(
       8 | +    OUTPUT ${PROJECT_BINARY_DIR}/src/precomputed_${base_name}.c
       9 | +    COMMAND secp256k1_precompute_${base_name}
      10 | +    DEPENDS secp256k1_precompute_${base_name}
      11 | +    WORKING_DIRECTORY ${PROJECT_BINARY_DIR}
      12 | +  )
    


    Copilot commented at 3:25 PM on June 9, 2026:

    add_custom_command() invokes secp256k1_precompute_${base_name} without a path. In typical Make/Ninja builds, the build directory is not on PATH (and . is usually not searched), so this custom command can fail at build time. Use $<TARGET_FILE:...> so CMake resolves the correct executable path (and also handles multi-config generators).


    hebasto commented at 4:05 PM on June 9, 2026:

    secp256k1_precompute_${base_name} is an executable target name.

    From CMake docs:

    If COMMAND specifies an executable target name (created by the add_executable() command), it will automatically be replaced by the location of the executable created at build time...

  33. in src/CMakeLists.txt:52 in 8460c5902c outdated
      51 | -)
      52 | +add_library(secp256k1_precomputed OBJECT EXCLUDE_FROM_ALL)
      53 | +if(SECP256K1_BUILD_PRECOMPUTED)
      54 | +  include(Precompute)
      55 | +  precompute(ecmult)
      56 | +  precompute(ecmult_gen)
    


    Copilot commented at 3:25 PM on June 9, 2026:

    When SECP256K1_BUILD_PRECOMPUTED is enabled, the build needs to execute freshly built precompute tools. During cross-compilation this typically cannot work (target executables can’t be run on the build host), so the failure will be confusing at build time. Consider explicitly erroring out with a clear message when CMAKE_CROSSCOMPILING is true.


    hebasto commented at 4:09 PM on June 9, 2026:

    Yes. The current CMake implementation doesn't support cross-compiling. And neither does Autotools.

    FWIW, I have a branch that introduces this support, but I'm not sure whether the extra complexity is justified.


    real-or-random commented at 10:39 AM on June 10, 2026:

    FWIW, I have a branch that introduces this support, but I'm not sure whether the extra complexity is justified.

    Ah please don't. :D Years ago, when we always ran the precomputation during the build (no precomputed files in repo), we had support for this in Autotools. But handling the different compilers (one for host and one for target) was a nightmare for users who wanted to cross-compile and for us to maintain. It's one of the reasons why added the precomputed files to the repo.

    I think if we really want to support this during cross-compilation, we'd rather rewrite the precomputation helpers in python or sage.


    hebasto commented at 10:44 AM on June 10, 2026:

    Ah please don't. :D Years ago, when we always ran the precomputation during the build (no precomputed files in repo), we had support for this in Autotools.

    TIL. Not arguing, but CMake has built-in ways to achieve this.


    real-or-random commented at 11:00 AM on June 10, 2026:

    CMake has built-in ways to achieve this.

    autoconf has this here: https://www.gnu.org/software/autoconf-archive/ax_prog_cc_for_build.html (not exactly built-in but that's the standard way on autoconf)

    I totally believe that the CMake way of doing this is much cleaner, but I think part of the problem is on a conceptual level. Now you get effectively two build configs with all their complexity. You have separate CFLAGS, CPPFLAGS, LDFLAGS, which raises all kinds of questions, e.g., which warning options do you want to set on the compiler for the build machine? Maybe the compiler is entirely different, e.g., you have a new clang for the build machine but an old gcc for the target, etc... And some of this complexity is necessarily pushed down to the user.

    Just assuming the existence of Python or similar is probably simpler. Back then we had already considered using Python instead, but approach to add the precomputed files to the repo was just less work, so we went with this instead.


    real-or-random commented at 11:03 AM on June 10, 2026:

    But do you think some error message like suggested by the LLM makes sense for now?


    hebasto commented at 1:03 PM on June 11, 2026:

    But do you think some error message like suggested by the LLM makes sense for now?

    Done.

  34. Copilot commented at 3:25 PM on June 9, 2026: none

    Pull request overview

    This PR adds a new CMake build option, SECP256K1_BUILD_PRECOMPUTED, to optionally regenerate the precomputed_ecmult*.c sources in the build tree for packagers that require recreating these generated files.

    Changes:

    • Introduces SECP256K1_BUILD_PRECOMPUTED (default OFF) and wires it into the src/ CMake logic to switch between in-tree vs build-tree precomputed sources.
    • Adds a new cmake/Precompute.cmake helper that builds/runs precompute_* generators to produce src/precomputed_*.c in the build directory.
    • Enables the option in the dev-mode CMake preset and in the Windows CMake CI job to exercise the path.

    Reviewed changes

    Copilot reviewed 5 out of 5 changed files in this pull request and generated 2 comments.

    <details> <summary>Show a summary per file</summary>

    File Description
    src/CMakeLists.txt Switches secp256k1_precomputed object sources between shipped and regenerated precomputed .c files based on the new option.
    CMakePresets.json Enables precompute regeneration in the dev-mode preset.
    CMakeLists.txt Adds the new SECP256K1_BUILD_PRECOMPUTED option definition.
    cmake/Precompute.cmake Adds a precompute() helper to build generator executables and register custom commands for generated .c outputs.
    .github/workflows/ci.yml Turns on SECP256K1_BUILD_PRECOMPUTED for Windows CMake CI configuration.

    </details>


    💡 <a href="/bitcoin-core/secp256k1/new/master?filename=.github/instructions/*.instructions.md" class="Link--inTextBlock" target="_blank" rel="noopener noreferrer">Add Copilot custom instructions</a> for smarter, more guided reviews. <a href="https://docs.github.com/en/copilot/customizing-copilot/adding-repository-custom-instructions-for-github-copilot" class="Link--inTextBlock" target="_blank" rel="noopener noreferrer">Learn how to get started</a>.

  35. hebasto force-pushed on Jun 9, 2026
  36. hebasto commented at 4:00 PM on June 9, 2026: member

    Consider rebasing this. The branch is way behind (for our standards ;)).

    Rebased. The recent feedback has been addressed.

    Maybe we should just change the error message for now? "If this is an autotools build, try deleting precomputed_ecmult.c before the build. If this is a CMake build, pass -DSECP256K1_BUILD_PRECOMPUTED=ON to cmake."

    Done.

  37. kevkevinpal commented at 4:34 PM on June 9, 2026: contributor

    If others agree, this raises the question of whether we want to get rid of that stuff in autotools. It never worked 100%. And it seems that all users who want this feature use CMake. (@kevkevinpal Why did you ask about autotools? Do you need this feature in autotools?)

    Trying to remember why but I think was just trying to understand why it wasn't available. If anything I agree with you and also prefer CMake.

  38. real-or-random commented at 11:01 AM on June 10, 2026: contributor

    Maybe we should just change the error message for now? "If this is an autotools build, try deleting precomputed_ecmult.c before the build. If this is a CMake build, pass -DSECP256K1_BUILD_PRECOMPUTED=ON to cmake."

    Done.

    We have the same message in precomputed_ecmult_gen.c. Sorry, I assumed that's implied. I should have mentioned this explicitly.

  39. real-or-random commented at 11:07 AM on June 10, 2026: contributor

    For autotools builds on CI, we have git diff that checks the files in the repo are not outdated. We could do the same on CI for CMake, i.e., copy the files back to the source tree and run git diff --exit-code.

    https://github.com/bitcoin-core/secp256k1/blob/0f4a7e6bf9d971addb2b851df7cd2777fc62b212/ci/ci.sh#L138-L147

  40. hebasto commented at 11:10 AM on June 10, 2026: member

    Maybe we should just change the error message for now? "If this is an autotools build, try deleting precomputed_ecmult.c before the build. If this is a CMake build, pass -DSECP256K1_BUILD_PRECOMPUTED=ON to cmake."

    Done.

    We have the same message in precomputed_ecmult_gen.c. Sorry, I assumed that's implied. I should have mentioned this explicitly.

    Of course, I've regenerated precomputed_ecmult_gen.c in the source tree :)

  41. real-or-random commented at 11:30 AM on June 10, 2026: contributor

    Maybe we should just change the error message for now? "If this is an autotools build, try deleting precomputed_ecmult.c before the build. If this is a CMake build, pass -DSECP256K1_BUILD_PRECOMPUTED=ON to cmake."

    Done.

    We have the same message in precomputed_ecmult_gen.c. Sorry, I assumed that's implied. I should have mentioned this explicitly.

    Of course, I've regenerated precomputed_ecmult_gen.c in the source tree :)

    Oops, we're talking past each other. Sorry, I meant to write precompute_ecmult_gen.c (without the d).

    So you've changed precompute_ecmult.c and precomputed_ecmult.c but not precompute_ecmult_gen.c and precomputed_ecmult_gen.c.

  42. hebasto force-pushed on Jun 11, 2026
  43. hebasto renamed this:
    cmake: Add option to recreate precomputed files
    cmake: Add option to generate precomputed files
    on Jun 11, 2026
  44. hebasto commented at 1:07 PM on June 11, 2026: member

    Reworked per recent feedback. Added CODEGEN support for modern CMake.

    The PR description has been updated.

    So you've changed precompute_ecmult.c and precomputed_ecmult.c but not precompute_ecmult_gen.c and precomputed_ecmult_gen.c.

    My bad. Fixed.

    For autotools builds on CI, we have git diff that checks the files in the repo are not outdated. We could do the same on CI for CMake, i.e., copy the files back to the source tree and run git diff --exit-code.

    Done.

  45. in CMakePresets.json:11 in 37152afa0e
       6 | @@ -7,7 +7,8 @@
       7 |        "cacheVariables": {
       8 |          "SECP256K1_EXPERIMENTAL": "ON",
       9 |          "SECP256K1_ENABLE_MODULE_RECOVERY": "ON",
      10 | -        "SECP256K1_BUILD_EXAMPLES": "ON"
      11 | +        "SECP256K1_BUILD_EXAMPLES": "ON",
      12 | +        "SECP256K1_BUILD_PRECOMPUTED": "OFF"
    


    real-or-random commented at 1:35 PM on June 11, 2026:

    nit: Why not omit this line entirely?


    hebasto commented at 1:56 PM on June 11, 2026:

    Thanks! Dropped.

  46. in .github/workflows/ci.yml:544 in 37152afa0e outdated
     537 | @@ -538,6 +538,10 @@ jobs:
     538 |      name: ${{ matrix.configuration.job_name }}
     539 |      # See: https://github.com/actions/runner-images#available-images.
     540 |      runs-on: windows-2022
     541 | +    defaults:
     542 | +      run:
     543 | +        # Enforce fail-fast behavior for PowerShell.
     544 | +        shell: pwsh -Command "$PSNativeCommandUseErrorActionPreference = $true; $ErrorActionPreference = 'Stop'; & '{0}'"
    


    real-or-random commented at 1:38 PM on June 11, 2026:

    Shouldn't this be implied according to the docs?


    hebasto commented at 1:55 PM on June 11, 2026:

    Shouldn't this be implied according to the docs?

    The docs state: "Fail-fast behavior when possible." Effectively, it works for cmdlets only. Non-zero exit codes from native commands are ignored unless $PSNativeCommandUseErrorActionPreference is also set.


    real-or-random commented at 1:58 PM on June 11, 2026:

    Sigh, why can't we just have nice things...

  47. cmake: Add option to generate precomputed files
    Add a new `SECP256K1_BUILD_PRECOMPUTED` option (`OFF` by default) that
    generates `precomputed_ecmult.c` and `precomputed_ecmult_gen.c` in the
    build tree. Those files are then compiled instead of the ones checked
    into the source tree.
    
    When configuring with CMake 3.31 or newer, the built-in `codegen` target
    can be used to generate those files.
    d36580cdc3
  48. ci: Add `-DSECP256K1_BUILD_PRECOMPUTED=ON` to CMake jobs 9c08b450d7
  49. hebasto force-pushed on Jun 11, 2026

github-metadata-mirror

This is a metadata mirror of the GitHub repository bitcoin-core/secp256k1. This site is not affiliated with GitHub. Content is generated from a GitHub metadata backup.
generated: 2026-06-12 08:15 UTC

This site is hosted by @0xB10C
More mirrored repositories can be found on mirror.b10c.me