When cross-compiling libsecp256k1, if the precomputed_ecmult*.c source files need to be regenerated, then the generators need to be built for the build system, not for the host system. Autoconf supports this fairly cleanly via the AX_PROG_CC_FOR_BUILD macro (from Autoconf Archive), but Automake requires some hackery. When building the generators, we override the CC, CFLAGS, CPPFLAGS, and LDFLAGS variables to their build-system counterparts, whose names are suffixed with _FOR_BUILD and whose values are populated by the aforementioned Autoconf macro and may be overridden on the make command line. Since Automake lacks support for overriding EXEEXT on a per-program basis, we define a recipe that builds the generator binaries with names suffixed with $(EXEEXT) and then renames them suffixed with $(BUILD_EXEEXT).
Makefile: build precomputed_ecmult generators using build-system toolchain to support cross-compiling #1159
pull whitslack wants to merge 1 commits into bitcoin-core:master from whitslack:programs_for_build changing 2 files +25 −6-
whitslack commented at 8:53 AM on November 20, 2022: contributor
-
772e747bd9
Makefile: build precomp generators using build-system toolchain
When cross-compiling libsecp256k1, if the `precomputed_ecmult*.c` source files need to be regenerated, then the generators need to be built for the *build* system, not for the *host* system. Autoconf supports this fairly cleanly via the `AX_PROG_CC_FOR_BUILD` macro (from Autoconf Archive), but Automake requires some hackery. When building the generators, we override the `CC`, `CFLAGS`, `CPPFLAGS`, and `LDFLAGS` variables to their build-system counterparts, whose names are suffixed with `_FOR_BUILD` and whose values are populated by the aforementioned Autoconf macro and may be overridden on the `make` command line. Since Automake lacks support for overriding `EXEEXT` on a per-program basis, we define a recipe that builds the generator binaries with names suffixed with `$(EXEEXT)` and then renames them suffixed with `$(BUILD_EXEEXT)`.
-
whitslack commented at 8:57 AM on November 20, 2022: contributor
Note: This PR introduces a dependency on Autoconf Archive. If this is undesirable, then the
ax_prog_cc_for_build.m4file can be extracted from Autoconf Archive and added to thebuild-aux/m4subdirectory of this project.Addendum: It looks like the automatic tests are failing because the build environment lacks Autoconf Archive. I don't know enough about Cirrus to fix that. Help?
-
sipa commented at 1:52 PM on November 20, 2022: contributor
The precomputed_* files don't need to be regenerated. They're independent of the host or target build environment, so they can be created on any system, and compiled on any system.
-
whitslack commented at 4:26 PM on November 20, 2022: contributor
@sipa: The precomputed files need to be regenerated if you specify non-default values for
--with-ecmult-window=and--with-ecmult-gen-precision=options toconfigure. If you're building using a cross-compiler, then the regeneration will fail because the generators will not be executable on the system executing the build. Standard practice is to compile build-time tools using a toolchain that targets the build system. -
whitslack commented at 4:40 PM on November 20, 2022: contributor
An example illustrating the failure:
$ git checkout origin/master $ git clean -fdx $ ./autogen.sh $ ./configure --build=x86_64-pc-linux-gnu --host=armv6j-unknown-linux-gnueabihf --with-ecmult-window=2 --with-ecmult-gen-precision=2 $ make clean-precomp $ make make precompute_ecmult_gen make[1]: Entering directory '/example' CC src/precompute_ecmult_gen-precompute_ecmult_gen.o CCLD precompute_ecmult_gen make[1]: Leaving directory '/example' ./precompute_ecmult_gen ./precompute_ecmult_gen: 1: Syntax error: word unexpected (expecting ")") make: *** [Makefile:1979: src/precomputed_ecmult_gen.c] Error 2The same example after applying this PR:
$ git fetch origin pull/1159/head $ git checkout FETCH_HEAD $ git clean -fdx $ ./autogen.sh $ ./configure --build=x86_64-pc-linux-gnu --host=armv6j-unknown-linux-gnueabihf --with-ecmult-window=2 --with-ecmult-gen-precision=2 $ make clean-precomp $ make make precompute_ecmult_gen make[1]: Entering directory '/example' CC src/precompute_ecmult_gen-precompute_ecmult_gen.o CCLD precompute_ecmult_gen make[1]: Leaving directory '/example' ./precompute_ecmult_gen make precompute_ecmult make[1]: Entering directory '/example' CC src/precompute_ecmult-precompute_ecmult.o CCLD precompute_ecmult make[1]: Leaving directory '/example' ./precompute_ecmult make all-am make[1]: Entering directory '/example' CC src/bench.o CC src/libsecp256k1_la-secp256k1.lo CC src/libsecp256k1_precomputed_la-precomputed_ecmult.lo CC src/libsecp256k1_precomputed_la-precomputed_ecmult_gen.lo CCLD libsecp256k1_precomputed.la CCLD libsecp256k1.la CCLD bench CC src/bench_internal-bench_internal.o CCLD bench_internal CC src/bench_ecmult-bench_ecmult.o CCLD bench_ecmult CC src/tests-tests.o CCLD tests CC src/exhaustive_tests-tests_exhaustive.o CCLD exhaustive_tests make[1]: Leaving directory '/example' -
real-or-random commented at 11:46 AM on November 21, 2022: contributor
This has a bit of a history. We used to support
AX_PROG_CC_FOR_BUILDfor years but then removed it (https://github.com/bitcoin-core/secp256k1/pull/988/commits/d94a37a20c3b5b44f1bcf60d309ffc50727e18e4) because our experience was that it results in a lot of issues on various platforms and rare configs. The macro itself and the autoconf code we'll need is a little bit hackish, plus users often didn't understand how building is supposed to work.The current resolution (as of #988) is to have the precomputed files in the repo. There should be only two reasons to rebuild them:
- As a developer, if you changed the programs that generate them.
- As a user, if you set
--with-ecmult-window=SIZEfor SIZE > 15. (But that's slower on "normal" machines.)
See the docs:
--with-ecmult-window=SIZE|auto window size for ecmult precomputation for verification, specified as integer in range [2..24]. Larger values result in possibly better performance at the cost of an exponentially larger precomputed table. The table will store 2^(SIZE-1) * 64 bytes of data but can be larger in memory due to platform-specific padding and alignment. A window size larger than 15 will require you delete the prebuilt precomputed_ecmult.c file so that it can be rebuilt. For very large window sizes, use "make -j 1" to reduce memory use during compilation. "auto" is a reasonable setting for desktop machines (currently 15). [default=auto]I see in your example that you want a SIZE of 2. And I see that you run
make clean-precomp. Maybe this not entirely clear from the docs you're never supposed to runmake clean-precompas a user. Just checkout the repo, configure with--ecmult-window-size=2, andmake. That should work. Can you try?(And maybe try a SIZE of 4. It should be much faster than 2 but still need only a negligible amount of memory.)
(And even if you want > 15, just delete
precomputed_ecmult.cas the docs say and automake should figure out how to rebuild it -- if you're not cross-compiling.) -
whitslack commented at 12:55 AM on November 22, 2022: contributor
@real-or-random: Thank you for the history review. It's a shame that a few edge cases had to ruin it for everyone. I do have to wonder if the issues you mentioned were due to Autotools not being used correctly, but I guess that ship has sailed.
My interest comes not as a user but as a packager. Gentoo takes a purist approach to open-source software: anything than can be rebuilt from sources ought to be. Standard operating procedure is to discard pregenerated assets and rebuild them from sources wherever possible. This is what the ebuilds for libsecp256k1 do with the precomputed ecmult tables. We have had to employ a hack to support cross-compiling, whereas most software packages that use Autotools and that compile tools to be executed during the build apparently have no trouble using
AX_PROG_CC_FOR_BUILD.I'll withdraw my request for proper cross-compilation support and continue carrying my patch downstream. Thank you for your time and explanation.
- whitslack closed this on Nov 22, 2022
-
real-or-random commented at 11:28 AM on November 22, 2022: contributor
Thanks for your understanding. And thanks for reaching out! We don't typically get feedback from packagers, and I see people applying (sometimes weird) patches without having talked to us.
My interest comes not as a user but as a packager. Gentoo takes a purist approach to open-source software: anything than can be rebuilt from sources ought to be. Standard operating procedure is to discard pregenerated assets and rebuild them from sources wherever possible.
I see. I was a Gentoo user in the past, but that's maybe a little bit too purist for my taste (though there's apparently a "wherever possible" exception). I'm not sure if this helps you, but maybe a simpler patch is to invoke a compiler on the build system manually. We try to make sure that the library is very easy to build manually, even though that's currently not documented.
I had a look at your ebuild at https://gitlab.com/bitcoin/gentoo/-/blob/d783d4d95cc1e7befeecec30a2d1f9e0d3a5ad72/dev-libs/libsecp256k1/libsecp256k1-0.1.0_pre20220803.ebuild#L73-74
$(usex lowmem '--with-ecmult-window=2 --with-ecmult-gen-precision=2' '') $(usex precompute-ecmult '--with-ecmult-window=24 --with-ecmult-gen-precision=8' '')This should really be
$(usex lowmem '--with-ecmult-window=4 --with-ecmult-gen-precision=2' '')(We should really just have a
--with-low-memoption...)And
precompute-ecmultshould probably be removed. I don't see what the purpose of this flag is. We always have precomputed ecmult tables and the only thing the USE flag currently does is increase these tables to a huge size, so huge that performance will suffer for sure (because the tables won't fit in the CPU cache).The defaults are 15 and 4, and they've been selected using careful benchmarks for desktop machines. Maybe you can squeeze out a few percents on your specific machine with something like (14,4) or (16,4) or similar if you really do the benchmarks. But (24,8) is insanely high and has no benefits.
-
whitslack commented at 11:35 AM on November 22, 2022: contributor
And
precompute-ecmultshould probably be removed. @real-or-random: Thanks. I'll make those changes. Those flags originated before I got involved. - gentoo-repo-qa-bot referenced this in commit 6e39601a74 on Nov 23, 2022
- whitslack cross-referenced this on Jan 6, 2023 from issue dev-libs/libsecp256k1 ebuild updates by whitslack
- whitslack referenced this in commit aa83d52772 on Jan 6, 2023
- whitslack referenced this in commit b1b09655f3 on Jan 6, 2023
- whitslack referenced this in commit 863272f631 on Jan 6, 2023
- whitslack referenced this in commit f99a2e5aea on Jan 6, 2023
- whitslack referenced this in commit 5feffcdabc on Jan 7, 2023
- whitslack referenced this in commit 798a0aa948 on Jan 7, 2023
- gentoo-bot referenced this in commit 92347c4b7a on Jan 7, 2023
- gentoo-bot referenced this in commit 691874242c on Jan 7, 2023