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?
> +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?
> +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.
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.
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].
> +static void ana_bits(float_status *fp_status,
> + float32 fsrca, float32 fsrcb, uint64_t *sfmt)
Is "ana" supposed to be short for "analyze"?
> +{
> + 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.
r~
