[PATCH 8/9] Negative numbers added for sreal class.

[Martin Liška]

On 11/21/2014 04:21 PM, Martin Liška wrote:
> On 11/21/2014 04:02 PM, Richard Biener wrote:
>> On Fri, Nov 21, 2014 at 3:39 PM, Martin Liška <mliska@suse.cz> wrote:
>>
>>> Hello.
>>>
>>> Ok, this is simplified, one can use sreal a = 12345 and it works ;)
>>>
>>>> that's a  new API, right?  There is no max () and I think that using
>>>> LONG_MIN here is asking for trouble (host dependence).  The
>>>> comment in the file says the max should be
>>>> sreal (SREAL_MAX_SIG, SREAL_MAX_EXP) and the min
>>>> sreal (-SREAL_MAX_SIG, SREAL_MAX_EXP)?
>>>>
>>>
>>> Sure, sreal can store much bigger(smaller) numbers :)
>>>
>>>> Where do you need sreal::to_double?  The host shouldn't perform
>>>> double calculations so it can be only for dumping?  In which case
>>>> the user should have used sreal::dump (), maybe with extra
>>>> arguments.
>>>>
>>>
>>> That new function was request from Honza, only for debugging purpose.
>>> I agree that dump should this kind of job.
>>>
>>> If no other problem, I will run tests once more and commit it.
>>> Thanks,
>>> Martin
>>
>> -#define SREAL_MAX_EXP (INT_MAX / 4)
>> +#define SREAL_MAX_EXP (INT_MAX / 8)
>>
>> this change doesn't look necessary anymore?
>>
>> Btw, it's also odd that...
>>
>>   #define SREAL_PART_BITS 32
>> ...
>>   #define SREAL_MIN_SIG ((uint64_t) 1 << (SREAL_PART_BITS - 1))
>>   #define SREAL_MAX_SIG (((uint64_t) 1 << SREAL_PART_BITS) - 1)
>>
>> thus all m_sig values fit in 32bits but we still use a uint64_t m_sig ...
>> (the implementation uses 64bit for internal computations, but still
>> the storage is wasteful?)
>>
>> Of course the way normalize() works requires that storage to be
>> 64bits to store unnormalized values.
>>
>> I'd say ok with the SREAL_MAX_EXP change reverted.
>>
>
> Hi.
>
> You are right, this change was done because I used one bit for m_negative (bitfield), not needed any more.
>
> Final version attached.
>
> Thank you,
> Martin
>
>> Thanks,
>> Richard.
>>
>>
>>>
>>>> Otherwise looks good to me and sorry for not noticing the above
>>>> earlier.
>>>>
>>>> Thanks,
>>>> Richard.
>>>>
>>>>> Thanks,
>>>>> Martin
>>>>>
>>>>>
>>>>>>>     };
>>>>>>>
>>>>>>>     extern void debug (sreal &ref);
>>>>>>> @@ -76,12 +133,12 @@ inline sreal &operator+= (sreal &a, const sreal
>>>>>>> &b)
>>>>>>>
>>>>>>>     inline sreal &operator-= (sreal &a, const sreal &b)
>>>>>>>     {
>>>>>>> -return a = a - b;
>>>>>>> +  return a = a - b;
>>>>>>>     }
>>>>>>>
>>>>>>>     inline sreal &operator/= (sreal &a, const sreal &b)
>>>>>>>     {
>>>>>>> -return a = a / b;
>>>>>>> +  return a = a / b;
>>>>>>>     }
>>>>>>>
>>>>>>>     inline sreal &operator*= (sreal &a, const sreal &b)
>>>>>>> --
>>>>>>> 2.1.2
>>>>>>>
>>>>>>>
>>>>>
>>>
>

Hello.

After IRC discussions, I decided to give sreal another refactoring where I
use int64_t for m_sig.

This approach looks much easier and straightforward. I would like to
ask folk for comments?

I am able to run profiled bootstrap on x86_64-linux-pc and ppc64-linux-pc
and new regression is introduced.

Thanks,
Martin


-------------- next part --------------
A non-text attachment was scrubbed...
Name: 0001-New-sreal-implementation-which-uses-int64_t-as-m_sig.patch
Type: text/x-patch
Size: 9480 bytes
Desc: not available
URL: <http://gcc.gnu.org/pipermail/gcc-patches/attachments/20141127/babecf87/attachment.bin>