On Mon, 8 Feb 2021 at 09:32, Philippe Mathieu-Daudé <f4...@amsat.org> wrote:
>
> +Eduardo/Richard.
>
> On 2/7/21 8:43 PM, Peter Maydell wrote:
> > On Sun, 7 Feb 2021 at 17:10, Philippe Mathieu-Daudé <f4...@amsat.org> wrote:
> >>
> >> On 10/22/20 2:08 PM, Peter Maydell wrote:
> >>> Commit ef96e3ae9698d6 in January 2019 removed the last user of the
> >>> VMSTATE_FLOAT64* macros. These were used by targets which defined
> >>> their floating point register file as an array of 'float64'.
> >>
> >> Similar candidate: VMSTATE_CPUDOUBLE_ARRAY()
> >
> > Isn't that still used by target/sparc ?
>
> Sorry, I meant CPU_DoubleU could be a similar to the removal of float64
> in commit ef96e3ae969 ("target/ppc: move FP and VMX registers into
> aligned vsr register array"), so we can remove VMSTATE_CPUDOUBLE_ARRAY()
> later.
>
> But I could be wrong and this is a legit use, as CPU_DoubleU contains
> either a float64 or a uint64_t.
CPU_DoubleU and friends are there to provide host-endian-independent
access to the different parts of a larger-than-word-size value (which is
why they're defined in bswap.h). They happen to use 'float64 d' as one
of the union subtypes as well as 'uint64_t ll', because they pre-date
the decision that we shouldn't really care about the distinction
between float64 and uint64_t except for documentation purposes in
helper function prototypes. But the core reason they're present is
to provide conversion between 'd' or 'll' and ('upper', 'lower').
There is some cleanup that could be done of these types now that we
don't try to distinguish float64 and uint64_t:
(1) CPU_FloatU is now entirely unnecessary, and uses like this (alpha):
static inline uint64_t float32_to_s(float32 fa)
{
CPU_FloatU a;
a.f = fa;
return float32_to_s_int(a.l);
}
could just be written:
static inline uint64_t float32_to_s(float32 fa)
{
return float32_to_s_int(fa);
}
(and then rename float32_to_s_int() to float32_to_s() and drop
the wrapper entirely)
(2) Where CPU_DoubleU is being used only for transitions between 'd'
and 'll', like this (ppc):
uint64_t helper_frsp(CPUPPCState *env, uint64_t arg)
{
CPU_DoubleU farg;
float32 f32;
farg.ll = arg;
if (unlikely(float64_is_signaling_nan(farg.d, &env->fp_status))) {
float_invalid_op_vxsnan(env, GETPC());
}
f32 = float64_to_float32(farg.d, &env->fp_status);
farg.d = float32_to_float64(f32, &env->fp_status);
return farg.ll;
}
we can drop the use of CPU_DoubleU and just pass 'arg'
directly where 'farg.d' was being passed, and similarly
just return the result of float32_to_float64() without
passing it through the union.
But at least some uses of these types will remain after that, I think.
(We could look again at what those remaining uses are after the
first round of cleanup and whether there are better ways to write
that code; personally I find these unions a bit ugly and wouldn't be
sorry to see them go away.)
thanks
-- PMM
