Closes #1324.
As the __shiftright128 docs state:
The
Shiftvalue is always modulo 64...
it is not applicable for the n >= 64 branch.
Use `__shiftright128` intrinsic in `secp256k1_u128_rshift` on MSVC #1336
pull hebasto wants to merge 1 commits into bitcoin-core:master from hebasto:230601-intrinsic changing 1 files +5 −0-
hebasto commented at 12:28 PM on June 1, 2023: member
-
real-or-random commented at 8:01 PM on June 1, 2023: contributor
Thanks for working on this.
it is not applicable for the
n >= 64branch.Good point, can you add a
VERIFY_CHECK(n < 64);over the intrinsic line to document this?This #ifdef should be active on CI, right? So this is tested on CI? It may make sense to add a few simple (fixed-input) tests for that function.
- hebasto force-pushed on Jun 4, 2023
-
hebasto commented at 3:21 PM on June 4, 2023: member
it is not applicable for the
n >= 64branch.Good point, can you add a
VERIFY_CHECK(n < 64);over the intrinsic line to document this?Added.
This #ifdef should be active on CI, right?
Yes.
So this is tested on CI?
It is.
It may make sense to add a few simple (fixed-input) tests for that function.
In addition to https://github.com/bitcoin-core/secp256k1/blob/60556c9f49a9384efd7f16b734820ae19108f053/src/tests.c#L1990-L1995 ?
-
Use `__shiftright128` intrinsic in `secp256k1_u128_rshift` on MSVC 5b7bf2e9d4
- hebasto force-pushed on Jun 4, 2023
-
hebasto commented at 5:04 PM on June 4, 2023: member
Rebased to avoid CI failures.
- real-or-random approved
-
real-or-random commented at 6:53 AM on June 5, 2023: contributor
utACK 5b7bf2e9d4ee02cbec1105ad6e890c34a4da1beb
In addition to ...
Sorry, I missed the existing test.
- real-or-random added the label performance on Jun 5, 2023
-
sipa commented at 8:14 PM on June 23, 2023: contributor
utACK 5b7bf2e9d4ee02cbec1105ad6e890c34a4da1beb
- real-or-random approved
-
real-or-random commented at 8:15 AM on June 24, 2023: contributor
ACK 5b7bf2e9d4ee02cbec1105ad6e890c34a4da1beb tested with MSVC x64
- real-or-random merged this on Jun 24, 2023
- real-or-random closed this on Jun 24, 2023
- hebasto deleted the branch on Jun 24, 2023
- vmta referenced this in commit 8f03457eed on Jul 1, 2023
- fanquake referenced this in commit 56c05c5ec4 on Jul 17, 2023
- fanquake referenced this in commit ff061fde18 on Jul 18, 2023
- hebasto referenced this in commit 270d2b37b8 on Jul 21, 2023
- delta1 referenced this in commit 3f32c20932 on Aug 8, 2023
- delta1 referenced this in commit 31ac0c1081 on Aug 31, 2023
- janus referenced this in commit 476a2176e7 on Sep 11, 2023
- div72 referenced this in commit af627d47c3 on Apr 12, 2025
- str4d referenced this in commit 3b49801869 on Jun 4, 2025
