Skip to content

Commit 5f7c1c9

Browse files
committed
Simplify divmod handling of "top dividend word exceeds top divisor word" case
Originally we computed q by dividing the top words of the dividend and divisor, then corrected it by at most 2. However, the ratio of the top words can only ever be 1, so rather than having three cases (original correct; need to subtract 1; need to subtract 2), we actually only have two (1 is correct; 0 is correct). This simplifies the algorithm a little bit and gets us to full test coverage, which we didn't have before since the "correct by 2" case was impossible to hit.
1 parent 953894a commit 5f7c1c9

File tree

1 file changed

+19
-16
lines changed

1 file changed

+19
-16
lines changed

src/num_native_impl.h

Lines changed: 19 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -383,25 +383,28 @@ static int secp256k1_num_div_mod(secp256k1_num *rq, secp256k1_num *rr, const sec
383383
* exceeded our base). This can only happen on the first iteration,
384384
* so we special-case it here before starting the real algorithm. */
385385
if (rem_high >= div_high) {
386+
/* Our shifting above guarantees us that div_high >= B/2, where B is our
387+
* word base. By definitien rem_high < B, as rem_high is one word. So we
388+
* have inside this if block that:
389+
* B > rem_high >= div_high >= B/2
390+
*
391+
* The inner inequality forces q = rem_high/div_high >= 1; then the
392+
* outer inequalities force q <= 1. So q always equals 1 here.
393+
*/
386394
secp256k1_num sub;
387-
secp256k1_num_word q = rem_high / div_high;
395+
secp256k1_num_word q = 1;
388396
secp256k1_num_mul_word_shift(&sub, &div, q, output_idx);
389-
/* Correct for error in the quotient. This was a while loop, but as it is
390-
* mathematically guaranteed to iterate at most twice, we can unroll it
391-
* into a pair of nested ifs. Note that we are guaranteed q > 0 here by
392-
* virtue of being in this if block. */
397+
/* This said, rem_high/div_high = 1 does not necessarily imply that the
398+
* actual quotient is 1. As described in the above block comment, this
399+
* is an overshoot of at most 2, i.e. maybe the actual quotient is zero.
400+
* In this case our reduction is a no-op! So rather than correcting q,
401+
* as we do in the analogous check in the loop below, we do nothing. */
393402
if (secp256k1_num_cmp (&sub, rr) > 0) {
394-
secp256k1_num_sub_shift_word (&sub, &sub, &div, output_idx, i + 1);
395-
--q;
396-
if (secp256k1_num_cmp (&sub, rr) >= 0) {
397-
secp256k1_num_sub_shift_word (&sub, &sub, &div, output_idx, i + 1);
398-
--q;
399-
}
400-
}
401-
/* Reduce remainder */
402-
secp256k1_num_sub_shift_word(rr, rr, &sub, 0, i + 1);
403-
rq->data[output_idx] = q;
404-
if (q != 0) {
403+
/* Do nothing. */
404+
} else {
405+
/* Reduce remainder */
406+
secp256k1_num_sub_shift_word(rr, rr, &sub, 0, i + 1);
407+
rq->data[output_idx] = q;
405408
ret = output_idx;
406409
}
407410
}

0 commit comments

Comments
 (0)