On 9/24/19 8:35 AM, Mark Cave-Ayland wrote: > +static void get_dfp64(uint64_t *dst, uint64_t *dfp) > +{ > + dst[0] = dfp[0]; > +} > + > +static void get_dfp128(uint64_t *dst, uint64_t *dfp) > +{ > + dst[0] = dfp[HI_IDX]; > + dst[1] = dfp[LO_IDX]; > +}
I'm not keen on this. I would prefer some type difference that prevents you from getting the arguments the wrong way around. I'm thinking that a combination helper like static void get_dfp64(decNumber *out, uint64_t *in) { union { uint64_t i; decimal64 d; } u; u.i = *in; decimal64ToNumber(&u.d, out); } > @@ -129,7 +140,7 @@ static void dfp_prepare_decimal64(struct PPC_DFP *dfp, > uint64_t *a, > dfp->env = env; > > if (a) { > - dfp->a64[0] = *a; > + get_dfp64(dfp->a64, a); > decimal64ToNumber((decimal64 *)dfp->a64, &dfp->a); > } else { > dfp->a64[0] = 0; becomes get_dfp64(&dfp->a, a); and that might clean up a lot of the code. > @@ -617,10 +626,12 @@ uint32_t helper_##op(CPUPPCState *env, uint64_t *a, > uint64_t *b) \ > { \ > struct PPC_DFP dfp; \ > unsigned k; \ > + uint64_t a64; \ > \ > dfp_prepare_decimal##size(&dfp, 0, b, env); \ > \ > - k = *a & 0x3F; \ > + get_dfp64(&a64, a); \ > + k = a64 & 0x3F; \ > \ > if (unlikely(decNumberIsSpecial(&dfp.b))) { \ > dfp.crbf = 1; \ Whereas cases like this, where a scalar is being passed in, I don't think that re-using the same helper is appropriate. Ideally, they would be passed in by value, and not by reference at all. That, of course, requires changes to the translator beyond the scope of this patch. > void helper_dctqpq(CPUPPCState *env, uint64_t *t, uint64_t *b) > { > struct PPC_DFP dfp; > + uint64_t b64; > dfp_prepare_decimal128(&dfp, 0, 0, env); > - decimal64ToNumber((decimal64 *)b, &dfp.t); > + get_dfp64(&b64, b); > + decimal64ToNumber((decimal64 *)&b64, &dfp.t); This would become get_dfp64(&dfp.t, b); with no intermediate b64 temp. r~