| Message ID | COL130-W35744AB7E5D577C3EDB6CDB9480@phx.gbl |
|---|---|
| State | New |
| Headers | show |
On 5 October 2015 at 12:21, Chen Gang <xili_gchen_5257@hotmail.com> wrote: > From 6bb2ed5b7046cda545f6a12721b773fde40f07f1 Mon Sep 17 00:00:00 2001 > From: Chen Gang <gang.chen.5i5j@gmail.com> > Date: Mon, 5 Oct 2015 19:12:07 +0800 > Subject: [PATCH] temp-floating-point: Use float32_to_t and t_to_float32 for > the input register value > > Original implementation use int*_to_float32 and float32_to_int*, which > will generate incorrect result. > > Signed-off-by: Chen Gang <gang.chen.5i5j@gmail.com> > --- > target-tilegx/fpu_helper.c | 51 +++++++++++++++++++++------------------------- > 1 file changed, 23 insertions(+), 28 deletions(-) > > diff --git a/target-tilegx/fpu_helper.c b/target-tilegx/fpu_helper.c > index daae570..2707f30 100644 > --- a/target-tilegx/fpu_helper.c > +++ b/target-tilegx/fpu_helper.c > @@ -68,6 +68,20 @@ static uint64_t float64_to_t(float64 fa) > return r.ll; > } > > +static float32 t_to_float32 (uint32_t a) > +{ > + CPU_FloatU r; > + r.l = a; > + return r.f; > +} This appears to be reimplementing make_float32(). > + > +static uint32_t float32_to_t(float32 a) > +{ > + CPU_FloatU r; > + r.f = a; > + return r.l; > +} And this is just float32_val(). > uint64_t helper_fsingle_add1(CPUTLGState *env, uint64_t rsrc, uint64_t rsrcb) > { > - FPUTLGState *fpu = &env->fpu; > - return fsingle_calc(fpu, int64_to_float32(rsrc, &FP_STATUS), > - int64_to_float32(rsrcb, &FP_STATUS), > - float32_add); > + return fsingle_calc(&env->fpu, t_to_float32((uint32_t)rsrc), > + t_to_float32((uint32_t)rsrcb), float32_add); > } Why is the helper for a single-precision operation taking a 64-bit argument anyway? thanks -- PMM
> On 5 October 2015 at 12:21, Chen Gang <xili_gchen_5257@hotmail.com> wrote: >> +static float32 t_to_float32 (uint32_t a) >> +{ >> + CPU_FloatU r; >> + r.l = a; >> + return r.f; >> +} > > This appears to be reimplementing make_float32(). > OK, thanks. >> + >> +static uint32_t float32_to_t(float32 a) >> +{ >> + CPU_FloatU r; >> + r.f = a; >> + return r.l; >> +} > > And this is just float32_val(). > OK, thanks. >> uint64_t helper_fsingle_add1(CPUTLGState *env, uint64_t rsrc, uint64_t rsrcb) >> { >> - FPUTLGState *fpu = &env->fpu; >> - return fsingle_calc(fpu, int64_to_float32(rsrc, &FP_STATUS), >> - int64_to_float32(rsrcb, &FP_STATUS), >> - float32_add); >> + return fsingle_calc(&env->fpu, t_to_float32((uint32_t)rsrc), >> + t_to_float32((uint32_t)rsrcb), float32_add); >> } > > Why is the helper for a single-precision operation taking a 64-bit > argument anyway? > Oh, the register are uint64_t, so I guess the input register value are 64-bit, too. Thanks. -- Chen Gang Open, share, and attitude like air, water, and life which God blessed
On 5 October 2015 at 12:54, Chen Gang <xili_gchen_5257@hotmail.com> wrote: >> On 5 October 2015 at 12:21, Chen Gang <xili_gchen_5257@hotmail.com> wrote: >> Why is the helper for a single-precision operation taking a 64-bit >> argument anyway? >> > > Oh, the register are uint64_t, so I guess the input register value are 64-bit, too. Usually for single precision the generated code is also working with 32-bit values. What you have is not necessarily wrong, but it is odd. I'd have to check the architecture manual and the translate.c code to be sure. thanks -- PMM
> From: peter.maydell@linaro.org > On 5 October 2015 at 12:54, Chen Gang <xili_gchen_5257@hotmail.com> wrote: >>> On 5 October 2015 at 12:21, Chen Gang <xili_gchen_5257@hotmail.com> wrote: >>> Why is the helper for a single-precision operation taking a 64-bit >>> argument anyway? >>> >> >> Oh, the register are uint64_t, so I guess the input register value are 64-bit, too. > > Usually for single precision the generated code is also working > with 32-bit values. What you have is not necessarily > wrong, but it is odd. I'd have to check the architecture > manual and the translate.c code to be sure. > OK, thanks. Originally, I referenced alpha and s390x for it. -- Chen Gang Open, share, and attitude like air, water, and life which God blessed
diff --git a/target-tilegx/fpu_helper.c b/target-tilegx/fpu_helper.c index daae570..2707f30 100644 --- a/target-tilegx/fpu_helper.c +++ b/target-tilegx/fpu_helper.c @@ -68,6 +68,20 @@ static uint64_t float64_to_t(float64 fa) return r.ll; } +static float32 t_to_float32 (uint32_t a) +{ + CPU_FloatU r; + r.l = a; + return r.f; +} + +static uint32_t float32_to_t(float32 a) +{ + CPU_FloatU r; + r.f = a; + return r.l; +} + static uint64_t ctx_to_uint64(TileGXFPCtx a) { union { @@ -199,36 +213,20 @@ static uint64_t fsingle_calc(FPUTLGState *fpu, uint64_t helper_fsingle_add1(CPUTLGState *env, uint64_t rsrc, uint64_t rsrcb) { - FPUTLGState *fpu = &env->fpu; - return fsingle_calc(fpu, int64_to_float32(rsrc, &FP_STATUS), - int64_to_float32(rsrcb, &FP_STATUS), - float32_add); + return fsingle_calc(&env->fpu, t_to_float32((uint32_t)rsrc), + t_to_float32((uint32_t)rsrcb), float32_add); } uint64_t helper_fsingle_sub1(CPUTLGState *env, uint64_t rsrc, uint64_t rsrcb) { - FPUTLGState *fpu = &env->fpu; - return fsingle_calc(fpu, int64_to_float32(rsrc, &FP_STATUS), - int64_to_float32(rsrcb, &FP_STATUS), - float32_sub); + return fsingle_calc(&env->fpu, t_to_float32((uint32_t)rsrc), + t_to_float32((uint32_t)rsrcb), float32_sub); } uint64_t helper_fsingle_mul1(CPUTLGState *env, uint64_t rsrc, uint64_t rsrcb) { - FPUTLGState *fpu = &env->fpu; -#if 0 - { - float32 v; - fprintf(stderr, "\ncall helper_fsingle_mul1(), %lx, %lx\n", rsrc, rsrcb); - v = float32_mul(int64_to_float32(rsrc, &FP_STATUS), - int64_to_float32(rsrcb, &FP_STATUS), - &FP_STATUS); - fprintf(stderr, "result: %lx.\n", float32_to_int64(v, &FP_STATUS)); - } -#endif - return fsingle_calc(fpu, int64_to_float32(rsrc, &FP_STATUS), - int64_to_float32(rsrcb, &FP_STATUS), - float32_mul); + return fsingle_calc(&env->fpu, t_to_float32((uint32_t)rsrc), + t_to_float32((uint32_t)rsrcb), float32_mul); } uint64_t helper_fsingle_pack2(CPUTLGState *env, uint64_t rsrc) @@ -240,17 +238,14 @@ uint64_t helper_fsingle_pack2(CPUTLGState *env, uint64_t rsrc) if (ctx.data>= TILEGX_F_COUNT) { helper_exception(env, TILEGX_EXCP_OPCODE_INVALID_VALUE); } - return float32_to_int32(fpu->val32s[ctx.data], &FP_STATUS); + return float32_to_t(fpu->val32s[ctx.data]); } switch (ctx.exp) { case 0x9e: - return float32_to_int64(uint32_to_float32(ctx.data, &FP_STATUS), - &FP_STATUS); + return float32_to_t(uint32_to_float32(ctx.data, &FP_STATUS)); case 0x49e: - return float32_to_int64(int32_to_float32(ctx.data | SIGNBIT32, - &FP_STATUS), - &FP_STATUS); + return float32_to_t(int32_to_float32(ctx.data | SIGNBIT32, &FP_STATUS)); default: fprintf(stderr, "\nUIMP: in helper_fsingle_pack2().\n"); helper_exception(env, TILEGX_EXCP_OPCODE_UNIMPLEMENTED);