Re: [PATCH] Use long for mantissa for generic mp code

[Siddhesh Poyarekar <siddhesh dot poyarekar at gmail dot com>]

On 27 February 2013 00:23, Richard Henderson <rth@twiddle.net> wrote:
> On 02/26/2013 07:42 AM, Siddhesh Poyarekar wrote:
>> +typedef long mantissa_t;
>> +typedef int64_t mantissa_store_t;
>
> Is there any need for these to be signed?  Unsigned greatly
> simplifies the code generated for DIV_RADIX.

Subtraction borrows from earlier mantissa digits, making them negative
momentarily.  If I can think of a way to avoid that, we could make it
unsigned later.

>> +#define TWOPOW(i) (1 << i)
>
> Wrong type.  1L (or 1UL, see above).

The largest number I make like this is 2^24, but I guess 1L should
make it consistent by avoiding 32-bit operations.

>> +/* Truncate IN to a multiple of F, where F is a power of two.  */
>> +#define TRUNCATE_TO_MUL(in, f) ((in) & ~(f))
>
> Err, ~f?  Either this doesn't work at all, or the comment is wrong.

Ugh, it should be ~(f - 1).

>> +++ b/sysdeps/powerpc/powerpc32/power4/fpu/mpa-arch.h
>> +++ b/sysdeps/powerpc/powerpc64/power4/fpu/mpa-arch.h
>
> We should not have two copies of this file.
>
> Is there, or could there be, a common directory, perhaps
> sysdeps/powerpc/power4/fpu/, that both targets share?
> Alternately, one file should include the other explicitly.

I'm not sure how the sysdeps structure works in this case, but I think
it might not be possible.  I'll try this (or something else) out to
avoid duplicating the files.  Thanks for the review.

Siddhesh
-- 
http://siddhesh.in