Optimization: avoid neg, shrink lookup when comb>256

Author: sipa

src/ecmult_gen_impl.h

@@ -119,40 +119,60 @@ static void secp256k1_ecmult_gen(const secp256k1_ecmult_gen_context *ctx, secp25
             /* Gather the mask(block)-selected bits of recoded into bits. They're packed
              * together: bit (tooth) of bits = bit
              * ((block*COMB_TEETH + tooth)*COMB_SPACING + comb_off) of recoded. */
-            uint32_t bits = 0, sign, abs, index, tooth;
-            for (tooth = 0; tooth < COMB_TEETH && bit_pos < 256; ++tooth) {
-                uint32_t bit = secp256k1_scalar_get_bits(&recoded, bit_pos, 1);
-                bits |= bit << tooth;
-                bit_pos += COMB_SPACING;
+            uint32_t bits = 0, index, tooth = 0;
+            while (1) {
+                if (EXPECT(tooth == COMB_TEETH, 0)) {
+                    /* All bits have been gathered.
+                     *
+                     * If the top bit of bits is 1, conditionally flip them all (corresponding
+                     * to looking up the negated table value), and remember to negate the
+                     * result in sign. */
+                    uint32_t sign = (bits >> (COMB_TEETH - 1)) & 1;
+                    uint32_t abs = (bits ^ -sign) & (COMB_POINTS - 1);
+                    VERIFY_CHECK(sign == 0 || sign == 1);
+                    VERIFY_CHECK(abs < COMB_POINTS);
+                    /* This uses a conditional move to avoid any secret data in array indexes.
+                     * _Any_ use of secret indexes has been demonstrated to result in timing
+                     * sidechannels, even when the cache-line access patterns are uniform.
+                     * See also:
+                     * - "A word of warning", CHES 2013 Rump Session, by Daniel J. Bernstein and
+                     *   Peter Schwabe (https://cryptojedi.org/peter/data/chesrump-20130822.pdf)
+                     * - "Cache Attacks and Countermeasures: the Case of AES", RSA 2006, by Dag
+                     *   Arne Osvik, Adi Shamir, and Eran Tromer
+                     *   (https://www.tau.ac.il/~tromer/papers/cache.pdf)
+                     */
+                    for (index = 0; index < COMB_POINTS; ++index) {
+                        secp256k1_ge_storage_cmov(&adds, &secp256k1_ecmult_gen_prec_table[block][index], index == abs);
+                    }
+                    /* Set add=adds or add=-adds, in constant time, based on sign. */
+                    secp256k1_ge_from_storage(&add, &adds);
+                    secp256k1_fe_negate(&neg, &add.y, 1);
+                    secp256k1_fe_cmov(&add.y, &neg, sign);
+                    break;
+#if COMB_BITS > 256
+                } else if (EXPECT(bit_pos >= 256, 0)) {
+                    /* Some bit(s) of (mask(block) << comb_off) are outside of [0,256). This means
+                     * we are also done constructing bits, but know its top bit is zero, and no
+                     * flipping/negating is needed. The table lookup can also be done over a
+                     * smaller number of entries. */
+                    VERIFY_CHECK(bits < (1U << tooth));
+                    VERIFY_CHECK(bits < COMB_POINTS);
+                    for (index = 0; (index >> tooth) == 0; ++index) {
+                        secp256k1_ge_storage_cmov(&adds, &secp256k1_ecmult_gen_prec_table[block][index], index == bits);
+                    }
+                    secp256k1_ge_from_storage(&add, &adds);
+                    break;
+#endif
+                } else {
+                    /* Gather another bit. */
+                    uint32_t bit = secp256k1_scalar_get_bits(&recoded, bit_pos, 1);
+                    VERIFY_CHECK(bit_pos < COMB_BITS && bit_pos < 256);
+                    bits |= bit << tooth;
+                    bit_pos += COMB_SPACING;
+                    ++tooth;
+                }
             }
 
-            /* If the top bit of bits is 1, conditionally flip them all (correspoding
-             * to looking up the negated table value), and remember to negate the
-             * result in sign. */
-            sign = (bits >> (COMB_TEETH - 1)) & 1;
-            abs = (bits ^ -sign) & (COMB_POINTS - 1);
-            VERIFY_CHECK(sign == 0 || sign == 1);
-            VERIFY_CHECK(abs < COMB_POINTS);
-
-            /** This uses a conditional move to avoid any secret data in array indexes.
-             *   _Any_ use of secret indexes has been demonstrated to result in timing
-             *   sidechannels, even when the cache-line access patterns are uniform.
-             *  See also:
-             *   "A word of warning", CHES 2013 Rump Session, by Daniel J. Bernstein and Peter Schwabe
-             *    (https://cryptojedi.org/peter/data/chesrump-20130822.pdf) and
-             *   "Cache Attacks and Countermeasures: the Case of AES", RSA 2006,
-             *    by Dag Arne Osvik, Adi Shamir, and Eran Tromer
-             *    (https://www.tau.ac.il/~tromer/papers/cache.pdf)
-             */
-            for (index = 0; index < COMB_POINTS; ++index) {
-                secp256k1_ge_storage_cmov(&adds, &secp256k1_ecmult_gen_prec_table[block][index], index == abs);
-            }
-
-            /* Set add=adds or add=-adds, in constant time, based on sign. */
-            secp256k1_ge_from_storage(&add, &adds);
-            secp256k1_fe_negate(&neg, &add.y, 1);
-            secp256k1_fe_cmov(&add.y, &neg, sign);
-
             /* Add the looked up and conditionally negated value to r. */
             if (EXPECT(first, 0)) {
                 /* If this is the first table lookup, we can skip addition. */