On 12/11/15 01:15, Richard Henderson wrote:
> On 12/10/2015 06:15 AM, Chen Gang wrote:
>> +#define TILEGX_F_CALC_CVT 0 /* convert int to fsingle */
>> +#define TILEGX_F_CALC_NCVT 1 /* Not convertion */
>> +
>> +static uint32_t get_f32_exp(float32 f)
>> +{
>> + return extract32(float32_val(f), 23, 8);
>> +}
>> +
>> +static void set_f32_exp(float32 *f, uint32_t exp)
>> +{
>> + *f = make_float32(deposit32(float32_val(*f), 23, 8, exp));
>> +}
>
> Why take a pointer instead of returning the new value?
>
I referenced set_* functions' declarations in "include/fpu/softfloat.h",
originally.
>> +static inline uint32_t get_fsingle_sign(uint64_t n)
>> +{
>> + return test_bit(10, &n);
>> +}
>> +
>> +static inline void set_fsingle_sign(uint64_t *n)
>> +{
>> + set_bit(10, n);
>> +}
>
> Why are you using test_bit and set_bit here, rather than continuing to use
> deposit and extract?
>
It is really only for one bit test and set, so test_bit/set_bit are
simpler and clearer than deposit/extract.
>> +static float32 sfmt_to_float32(uint64_t sfmt, float_status *fp_status)
>> +{
>> + float32 f;
>> + uint32_t sign = get_fsingle_sign(sfmt);
>> + uint32_t man = get_fsingle_man(sfmt);
>> +
>> + if (get_fsingle_calc(sfmt) == TILEGX_F_CALC_CVT) {
>> + if (sign) {
>> + return int32_to_float32(0 - man, fp_status);
>> + } else {
>> + return uint32_to_float32(man, fp_status);
>> + }
>> + } else {
>> + f = float32_set_sign(float32_zero, sign);
>> + f |= create_f32_man(man >> 8);
>> + set_f32_exp(&f, get_fsingle_exp(sfmt));
>> + }
>
> I'm not especially keen on this calc bit. I'd much rather that we always pack
> and round properly.
>
OK.
> In particular, if gcc decided to optimize fractional fixed-point types, it
> would do something very similar to the current floatsisf2 code sequence,
> except
> that it wouldn't use 0x9e as the exponent; it would use something smaller, so
> that some number of low bits of the mantessa would be below the radix point.
>
Oh, really.
> Therefore, I think that fsingle_pack2 should do the following: Take the
> (sign,exp,man) tuple and slot them into a double -- recall that a single only
> has 23 bits in its mantessa, and this temp format has 32 -- then convert the
> double to a single. Pre-rounded single results from fsingle_* will be
> unchanged, while integer data that gcc has constructed will be properly
> rounded.
>
> E.g.
>
> uint32_t sign = get_fsingle_sign(sfmt);
> uint32_t exp = get_fsingle_exp(sfmt);
> uint32_t man = get_fsingle_man(sfmt);
> uint64_t d;
>
> /* Adjust the exponent for double precision, preserving Inf/NaN. */
> if (exp == 0xff) {
> exp = 0x7ff;
> } else {
> exp += 1023 - 127;
> }
>
> d = (uint64_t)sign << 63;
> d = deposit64(d, 53, 11, exp);
> d = deposit64(d, 21, 32, man);
> return float64_to_float32(d, fp_status);
>
> Note that this does require float32_to_sfmt to store the mantissa
> left-justified. That is, not in bits [54-32] as you're doing now, but in bits
> [63-41].
>
For me, it is a good idea! :-)
>> +static void ana_bits(float_status *fp_status,
>> + float32 fsrca, float32 fsrcb, uint64_t *sfmt)
>
> Is "ana" supposed to be short for "analyze"?
>
Yes.
>> +{
>> + if (float32_eq(fsrca, fsrcb, fp_status)) {
>> + *sfmt |= create_fsfd_flag_eq();
>> + } else {
>> + *sfmt |= create_fsfd_flag_ne();
>> + }
>> +
>> + if (float32_lt(fsrca, fsrcb, fp_status)) {
>> + *sfmt |= create_fsfd_flag_lt();
>> + }
>> + if (float32_le(fsrca, fsrcb, fp_status)) {
>> + *sfmt |= create_fsfd_flag_le();
>> + }
>> +
>> + if (float32_lt(fsrcb, fsrca, fp_status)) {
>> + *sfmt |= create_fsfd_flag_gt();
>> + }
>> + if (float32_le(fsrcb, fsrca, fp_status)) {
>> + *sfmt |= create_fsfd_flag_ge();
>> + }
>> +
>> + if (float32_unordered(fsrca, fsrcb, fp_status)) {
>> + *sfmt |= create_fsfd_flag_un();
>> + }
>> +}
>
> Again, I think it's better to return the new sfmt value than modify a pointer.
>
Oh, I guess, we can inline ana_bits() to main_calc(), for they are both
simple short functions, and ana_bits() is only called by main_calc().
Thanks.
--
Chen Gang (陈刚)
Open, share, and attitude like air, water, and life which God blessed
