[Qemu-devel] Re: [PATCH] target-sh4: get rid of CPU_{Float, Double}U

[Peter Maydell <peter.maydell@linaro.org>]

On 11 April 2011 15:55, Nathan Froyd <froydnj@codesourcery.com> wrote:
> On Sun, Apr 10, 2011 at 09:13:05PM +0200, Aurelien Jarno wrote:
>> SH4 is always using softfloat, so it's possible to have helpers directly
>> taking float32 or float64 value. This allow to get rid of conversions
>> through CPU_{Float,Double}U.
>
> Eh, I think this punning on i32/f32 and i64/f64 values is not healthy.
> But Peter's already said that the floats-as-structs bit of softfloat is
> broken, so maybe it's not worth trying to ensure floats-as-structs works
> (or even making it the default, to discourage people from bit-twiddling
> directly).

I guess I should clarify that about the floats-as-structs thing.

(1) It does compile cleanly for the ARM target. Some other targets
don't compile because they're (buggily) not using the boxing/unboxing
macros when they do bit-twiddling of floats; that should be fixed.
(2) I think most of the value is in whether it compiles OK or not,
rather than trying to actually run with it as a config (which I
agree with Nathan is likely to go wrong if you have a host which
doesn't pass 32/64 bit structs in registers). The compile test catches
cases where the C code is doing bit-twiddling on float32s.
(3) If we did say you shouldn't be passing 'float32' etc into helper
functions, this would make the def-helper.h support for 'f32' and 'f64'
a bit pointless because they could never be used
(4) I think you should be able to write a helper function for an
add as just
 float32 HELPER(my_float_add)(float32 a, float32 b) {
     return float32_add(a, b, status);
 }
and having to add boxing/unboxing macros to this reduces clarity
for no real gain. Using the macros should be a sign you're doing
something wrong, not that you're doing it right :-)

-- PMM