Rip out non-endomorphism code #826
pull sipa wants to merge 1 commits into bitcoin-core:master from sipa:202009_endo_endo_endo changing 17 files +20 −230-
sipa commented at 3:07 am on September 26, 2020: contributorAs the patent on the GLV optimization has expired, there is no need to keep the slower non-endomorphism code around anymore.
-
sipa force-pushed on Sep 26, 2020
-
sipa force-pushed on Sep 26, 2020
-
elichai commented at 8:51 am on September 26, 2020: contributorThere’s nothing I love more than a PR that only deletes code :)
-
sipa commented at 8:51 am on September 26, 2020: contributor
There’s nothing I love more than a PR that only deletes code :)
Hey it also improves some comments.
-
in .travis.yml:42 in a523e2c996 outdated
45 fast_finish: true 46 include: 47 - compiler: clang 48 os: linux 49- env: HOST=i686-linux-gnu ENDOMORPHISM=yes 50+ env: HOST=i686-linux-gnu
elichai commented at 8:55 am on September 26, 2020:This job can be completely removed, it’s exactly the same as the one below
sipa commented at 8:57 am on September 26, 2020:One has gmp, the other doesn’t. (same comment applies below).
elichai commented at 9:01 am on September 26, 2020:Oh you’re right, my mistake.in .travis.yml:66 in a523e2c996 outdated
58@@ -63,7 +59,7 @@ matrix: 59 - libtool-bin 60 - libc6-dbg:i386 61 - compiler: gcc 62- env: HOST=i686-linux-gnu ENDOMORPHISM=yes
elichai commented at 8:55 am on September 26, 2020:This job can be completely removed, it’s exactly the same as the one belowelichai commented at 9:04 am on September 26, 2020: contributortACK a523e2c9965ff43939abad555fcb8a8aed684d8e
two nits:
- Can you also remove from here: https://github.com/bitcoin-core/secp256k1/blob/master/contrib/travis.sh#L16
- Can you update the README to remove the “optionally”:
Optionally (off by default) use secp256k1’s efficiently-computable endomorphism to split the P multiplicand into 2 half-sized ones.
practicalswift commented at 3:21 pm on September 26, 2020: contributorACK a523e2c9965ff43939abad555fcb8a8aed684d8e (modulo merge conflict): diff looks correctsipa cross-referenced this on Sep 26, 2020 from issue Enable endomorphism by default by paulmillrRip out non-endomorphism code 46f9e0e745sipa force-pushed on Sep 26, 2020sipa commented at 6:14 pm on September 26, 2020: contributorRebased, and:
- Can you also remove from here: https://github.com/bitcoin-core/secp256k1/blob/master/contrib/travis.sh#L16
- Can you update the README to remove the “optionally”:
Done.
gmaxwell commented at 9:08 pm on September 26, 2020: contributorLooks good to me, endo only increases a stripped Os build for me by 456 bytes– so insignificant esp with respect to the performance difference, so I don’t see any reason to keep the other code around even for code size constrained applications (you can get 456 bytes of savings in other ways with less performance impact).tarcieri cross-referenced this on Sep 28, 2020 from issue k256: enable endomorphism optimizations by default; remove non-endomorphism code by tarcieristr4d cross-referenced this on Sep 28, 2020 from issue Use the endomorphism optimization for secp256k1 by str4djonatack commented at 3:00 pm on September 28, 2020: none46f9e0e745accde97742cb542d3c779f5b99b4a8 patch diff LGTMtarcieri referenced this in commit 4858cb0b9d on Sep 28, 2020tarcieri cross-referenced this on Sep 28, 2020 from issue k256: enable endomorphisms by default by tarcieritarcieri referenced this in commit 3ae2dd112c on Sep 28, 2020practicalswift commented at 7:33 pm on September 28, 2020: contributorre-ACK 46f9e0e745accde97742cb542d3c779f5b99b4a8laanwj commented at 8:47 pm on September 28, 2020: memberACK 46f9e0e745accde97742cb542d3c779f5b99b4a8
Looks good to me, endo only increases a stripped Os build for me by 456 bytes– so insignificant esp with respect to the performance difference
I agree. I also looked at it from that angle, but this is a negligible increase in code size. Not worth keeping an option around for. (on RV64 there is no size difference for the stripped
libsecp256k1.so.0.0.0
between master and master with this patch merged)real-or-random cross-referenced this on Sep 29, 2020 from issue Enable GLV optimization? by oleibaelichai commented at 9:58 am on September 29, 2020: contributorre-ACK 46f9e0e745accde97742cb542d3c779f5b99b4a8tarcieri referenced this in commit 0db4e7e945 on Sep 29, 2020tarcieri cross-referenced this on Sep 29, 2020 from issue k256: remove non-endomorphism code by tarcieritarcieri referenced this in commit 2b8ab1c2cb on Sep 30, 2020sipa cross-referenced this on Oct 5, 2020 from issue Increase precision of g1 and g2. by roconnor-blockstreambenthecarman commented at 4:07 pm on October 9, 2020: noneACK 46f9e0esipa cross-referenced this on Oct 11, 2020 from issue Rip out non-endomorphism code + dependencies by sipasipa cross-referenced this on Oct 11, 2020 from issue Switch to our own memcmp function by real-or-randomsipa closed this on Oct 11, 2020
sipa referenced this in commit c6b6b8f1bb on Oct 14, 2020azuchi cross-referenced this on Jan 6, 2022 from issue GLV optimization by default by azuchi
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: 2024-10-30 03:15 UTC
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: 2024-10-30 03:15 UTC
This site is hosted by @0xB10C
More mirrored repositories can be found on mirror.b10c.me
More mirrored repositories can be found on mirror.b10c.me