gmpxx: bug in declaration of __gmp_set_expr
tg at swox.com
Tue Aug 12 16:33:45 CEST 2008
Marc Glisse <marc.glisse at normalesup.org> writes:
etc with all possible combinations of mpz/mpq/mpf and
First, one can notice in the rest of the code that __gmp_set_expr is
mostly called on compound expressions, and can only be called on
mp*_class if it is not the same as the first argument (mpz_t,mpz_class
is never called, but mpz_t,mpq_class can be). This means three of
these functions are useless, but they don't hurt and might become
useful again soon. Now if we actually call
__gmp_set_expr(mpz_t,mpq_class), we notice that it does not call the
version of the function that we would expect, but the generic
__gmp_set_expr(mpz_t,__gmp_expr(mpq_t,T)). The reason appears to be
the empty "template<>" at the beginning. It looks like a confusion
between functions (which get overloads) and classes (which get partial
specializations). Removing the "template<>" seems to solve the
issue. But then it is surprising the compiler didn't print a warning
if writing it was wrong. Also, the early declarations are apparently
useless (but if we want them we should probably declare all
It would be nice if someone familiar with C++ could comment on this...
By the way I am attaching a version of gmpxx.h where I played with
- small temporaries on stack (I skipped hypot)
Like the stuff I did recently?
- no need to write the code 4 times for all comparison operators
- do some unary operations in-place (except for sqrt for which it
I am not too happy with the commute thing.
Don't take any of it as-is, it is just there as a list of ideas, even
though I made sure it still passed the testsuite.
The tests for gmpxx.h in the GMP testsuite is far inferior to the rest
of the testsuite, unfortunately.
I am also attaching
a patch to mpz/aors to make reallocation less likely, but I am not
sure it really improves anything with a usual malloc implementation
where realloc(size+4) is usually cheap.
I am not sure this is a good idea, since it might lead to two
reallocations. *If* it is a good idea, the same trick applies to
dozens of other places in GMP.
It is an intensional design to end up with an extra limb, to simplify
bookkeeping. It might be a bad design decision for applications with
a large number of small mpz_t variables, so we might want to evaluate
it. But then we need to look at the cost of it in many functions, not
In gmpxx, it sometimes happens that mpq_set(r,s) is called with r==s
in addition and substraction (for instance for q+=3), but I noticed
that g++ -O1 is able to detect it with __builtin_constant_p. We could
replace mpq_set by if(r!=s)mpqset, but if we decide there is too much
overhead this way, we can still do it at compile time only with gcc.
How do you use __builtin_constant_p for that?
I don't like if(r!=s)mpqset much.
There are actually quite a few things (even in the C version) that
could benefit from the use of gcc's __builtin_constant_p. mul_ui could
check whether its argument is a power of 2 and call mul_2exp
That would require to make it a macro in gmp-h, of course, and rename
the function to something else. Care need to be taken to put a
function named mpz_mul_ui in the (shared) library, otherwise we would
break binary compatibility.
But if that is taken care of, it could be a nice optimization.
Oh and gmpxx seems to be missing a pow function.
More information about the gmp-bugs