Re: [Qemu-devel] [PATCH v4 2/4] target-tilegx: Add single floating point implementation

[Richard Henderson]

On 12/23/2015 01:48 PM, cheng...@emindsoft.com.cn wrote:

+static float32 sfmt_to_float32(uint64_t sfmt)
+{
+uint32_t sign = get_fsingle_sign(sfmt);
+uint32_t man = get_fsingle_man(sfmt);
+uint32_t exp = get_fsingle_exp(sfmt);
+float32 f;
+
+if (get_fsingle_calc(sfmt) == TILEGX_F_CALC_CVT) {
+if (sign) {
+f = int32_to_float32(0 - man, &fp_status);
+} else {
+f = uint32_to_float32(man, &fp_status);
+}
+exp += get_f32_exp(f) - 0x9e;
+if ((int32_t) exp < 0) {
+return float32_infinity | float32_set_sign(float32_zero, sign);
+} else if (exp >= 0xff) {
+return float32_zero | float32_set_sign(float32_zero, sign);
+} else {
+set_f32_exp(&f, exp);
+}



What in the world are you attempting to do here?
This is not normalization.  This is not even remotely
correct with respect to zero or infinity.

Moreover, I thought we agreed to do away with that CALC bit.


r~


+} else {
+f = create_f32_man(man >> 8);
+set_f32_exp(&f, exp);
+}
+
+f = float32_set_sign(f, sign);
+return f;
+}
+
+uint64_t helper_fsingle_pack2(uint64_t srca)
+{
+return float32_val(sfmt_to_float32(srca));
+}
+
+static uint64_t main_calc(float32 fsrca, float32 fsrcb,
+  float32 (*calc)(float32, float32, float_status *))
+{
+uint64_t sfmt = float32_to_sfmt(calc(fsrca, fsrcb, &fp_status));
+
+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();
+}
+
+set_fsingle_calc(&sfmt, TILEGX_F_CALC_NCVT);
+return sfmt;
+}
+
+uint64_t helper_fsingle_add1(uint64_t srca, uint64_t srcb)
+{
+return main_calc(make_float32(srca), make_float32(srcb), float32_add);
+}
+
+uint64_t helper_fsingle_sub1(uint64_t srca, uint64_t srcb)
+{
+return main_calc(make_float32(srca), make_float32(srcb), float32_sub);
+}
+
+uint64_t helper_fsingle_mul1(uint64_t srca, uint64_t srcb)
+{
+return main_calc(make_float32(srca), make_float32(srcb), float32_mul);
+}

Re: [Qemu-devel] [PATCH v4 2/4] target-tilegx: Add single floating point implementation

[Chen Gang]

On 12/24/15 07:07, Richard Henderson wrote:
> On 12/23/2015 01:48 PM, cheng...@emindsoft.com.cn wrote:
>> +static float32 sfmt_to_float32(uint64_t sfmt)
>> +{
>> +uint32_t sign = get_fsingle_sign(sfmt);
>> +uint32_t man = get_fsingle_man(sfmt);
>> +uint32_t exp = get_fsingle_exp(sfmt);
>> +float32 f;
>> +
>> +if (get_fsingle_calc(sfmt) == TILEGX_F_CALC_CVT) {
>> +if (sign) {
>> +f = int32_to_float32(0 - man, &fp_status);
>> +} else {
>> +f = uint32_to_float32(man, &fp_status);
>> +}
>> +exp += get_f32_exp(f) - 0x9e;
>> +if ((int32_t) exp < 0) {
>> +return float32_infinity | float32_set_sign(float32_zero, sign);
>> +} else if (exp >= 0xff) {
>> +return float32_zero | float32_set_sign(float32_zero, sign);
>> +} else {
>> +set_f32_exp(&f, exp);
>> +}
> 
> 
> What in the world are you attempting to do here?
> This is not normalization.  This is not even remotely
> correct with respect to zero or infinity.
> 

For fdouble, I use almost the same way, but can get the correct result (
pass gcc testsuite for fdouble, include inf and zero tests). But tests
will never enough, we can not say the fdouble implementation must be OK.

So please help check the fdouble implementation in details, again, when
you have time, it may still have issues.

> Moreover, I thought we agreed to do away with that CALC bit.
> 

OK, I will try, next.

I will copy and reconstruct related code from qemu fpu implementation
instead of (u)int32/64_to_float32/64 functions (just like you said, I
guess).

Hope I can finish within 2015-12-31.

Thanks.
-- 
Chen Gang (陈刚)

Open, share, and attitude like air, water, and life which God blessed

Re: [Qemu-devel] [PATCH v4 2/4] target-tilegx: Add single floating point implementation

[Chen Gang]

On 12/24/15 23:52, Chen Gang wrote:
> On 12/24/15 07:07, Richard Henderson wrote:
> 
>> Moreover, I thought we agreed to do away with that CALC bit.
>>

After check again, I guess, we can stil reserve CALC bit:

 - Then we can remove float32_to_sfmt (use high 32-bit to save float32
   directly). And in helper_fsingle_pack2, for CALC, we can return high
   32-bit directly.  Only for NCALC, we need process it in details.

 - I guess, most cases are for CALC, so it will let the performance a
   little better (need float32_to_sfmt, then sfmt_to_float32 again).

Then we can only focus on NCAL in helper_fsingle_pack2.

Thanks.

> 
> OK, I will try, next.
> 
> I will copy and reconstruct related code from qemu fpu implementation
> instead of (u)int32/64_to_float32/64 functions (just like you said, I
> guess).
> 
> Hope I can finish within 2015-12-31.
> 
> Thanks.
> 

-- 
Chen Gang (陈刚)

Open, share, and attitude like air, water, and life which God blessed