mul_fft, cleaning up some details of the code
Marco Bodrato
bodrato at mail.dm.unipi.it
Sun Mar 6 11:14:49 CET 2022
Ciao,
I looked into the code published by Samuel Vivien.
I realised that in mul_fft there are a lot of _add_1 and _sub_1. At
least some of them can be easily replaced by _INCR_U or _DECR_U...
Specifically I'd focus into a suspicious piece of code, shared by both
our current code and Vivien's.
The function mpn_mul_fft_decompose has a branch "if (dif > Kl)", that
ends with the following lines:
if (cy >= 0)
cy = mpn_add_1 (tmp, tmp, Kl, cy);
else
cy = mpn_sub_1 (tmp, tmp, Kl, -cy);
Can those lines be correct? Can the value in cy be used after this code
both if it contains the carry or if it contains the borrow of the
operation on tmp?
I believe this is an error, I'd change the last line into
cy = 1 - mpn_sub_1 (tmp, tmp, Kl, -cy - 1);
and I propose the attached patch changing this and some other _1
functions.
This is not really needed to solve a bug.
The comment before the mpn_mul_fft_decompose function says "We must have
nl <= 2*K*l", this means that we should never have ((dif = nl - Kl) >
Kl), and the code in that branch should never be used.
According to the coverage analisys, the code is not explored by the
tests:
https://gmplib.org/devel/lcov/shell/var/tmp/lcov/gmp/mpn/mul_fft.c.gcov#691
The "bug" is never triggered.
Maybe the branch could be used if someone re-enables mpn_mul_fft_full
and uses it for very unbalanced (more than 4x1) operands?
Ĝis,
m
-------------- next part --------------
A non-text attachment was scrubbed...
Name: mul_fft.diff
Type: text/x-diff
Size: 4103 bytes
Desc: not available
URL: <https://gmplib.org/list-archives/gmp-devel/attachments/20220306/a234224d/attachment.bin>
More information about the gmp-devel
mailing list