gmpxx: bug in declaration of __gmp_set_expr

[Marc Glisse]

On Tue, 12 Aug 2008, Torbjorn Granlund wrote:

>  - small temporaries on stack (I skipped hypot)
>
> Like the stuff I did recently?

Exactly.

>  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.

Actually, no, it can't. What the patch does is check if the size is 
smaller than N and in this case realloc _N+1_ (not just N), and then only 
when needed check whether the size is smaller than N+1 and in this case 
realloc N+1. So if the first allocation happens, the second one cannot 
happen. The idea is just that often, particularly for in-place operations, 
N should be enough. But when we reallocate, we might as well get the extra 
limb to make sure we have enough space for a potential carry.

> *If* it is a good idea, the same trick applies to dozens of other places 
> in GMP.

I don't know if it is a *good* idea, just an idea I had and felt like 
mentionning.

> 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
> just mpz/aors.

I don't mind allocating extra space when we allocate, I just mind 
reallocating when it is not needed. But then again, I am just mentionning 
something, it might be a bad idea. Allocating an extra limb for 
multiplication might be better in practice, or the current code might be 
best...

>  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.

#if COMPILERHASBUILTINCONSTANTP
if(__builtin_constant_p(r==s)&&(r==s));else
#endif
mpqset();

(I didn't check just now, but I believe that is what I tried the other 
day and g++-4.1 handled it fine for q+=5)

>  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
>  instead.
>
> 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.

I thougt the function name was __gmpz_*, and mpz_* was just a macro, that 
for now is equal to __gmpz_*, but that it would not matter if it contained 
a bit more code (anyway removed at compile-time) before calling __gmpz_*.


-- 
Marc Glisse