Re: [Qemu-devel] [PATCH 2/5] softfloat: Fix warn about implicit conversion from int to int8_t

[Aurelien Jarno <aurelien@aurel32.net>]

On 2016-08-09 22:12, Peter Maydell wrote:
> On 9 August 2016 at 20:16, Aurelien Jarno <aurelien@aurel32.net> wrote:
> > On 2016-08-09 15:02, Pranith Kumar wrote:
> >> Change the flag type to 'int' to fix the implicit conversion error.
> >>
> >> Suggested-by: Peter Maydell <peter.maydell@linaro.org>
> >> Signed-off-by: Pranith Kumar <bobby.prani@gmail.com>
> >> ---
> >>  fpu/softfloat-specialize.h | 2 +-
> >>  include/fpu/softfloat.h    | 4 ++--
> >>  2 files changed, 3 insertions(+), 3 deletions(-)
> >>
> >> diff --git a/fpu/softfloat-specialize.h b/fpu/softfloat-specialize.h
> >> index 43d0890..46b4091 100644
> >> --- a/fpu/softfloat-specialize.h
> >> +++ b/fpu/softfloat-specialize.h
> >> @@ -197,7 +197,7 @@ float128 float128_default_nan(float_status *status)
> >>  | should be simply `float_exception_flags |= flags;'.
> >>  *----------------------------------------------------------------------------*/
> >>
> >> -void float_raise(int8_t flags, float_status *status)
> >> +void float_raise(int flags, float_status *status)
> >>  {
> >>      status->float_exception_flags |= flags;
> >>  }
> >> diff --git a/include/fpu/softfloat.h b/include/fpu/softfloat.h
> >> index 0e57ee5..416cf7a 100644
> >> --- a/include/fpu/softfloat.h
> >> +++ b/include/fpu/softfloat.h
> >> @@ -196,9 +196,9 @@ enum {
> >>  };
> >>
> >>  typedef struct float_status {
> >> +    int         float_exception_flags;
> >>      signed char float_detect_tininess;
> >>      signed char float_rounding_mode;
> >> -    signed char float_exception_flags;
> >>      signed char floatx80_rounding_precision;
> >>      /* should denormalised results go to zero and set the inexact flag? */
> >>      flag flush_to_zero;
> >
> > This changes the size of the structure, and thus of the CPU*State
> > structures. I don't think it's something we want to do, especially given
> > we currently only use 7 flags, so 7 bits and that fits in a char.
> 
> It does, but only by four bytes, which I didn't think was that
> big a deal. If we want to keep it to one byte then I think

Indeed it's not a lot, but if we do that with everything that goes into
the CPU*state structures, it has a significant impact.

> making it a uint8_t is probably better than leaving it as
> signed char, given we're definitely not treating it as a
> signed value.

I agree the signed char type is very strange here, it is probably there
for historical reasons. I guess using a uint8_t would indeed be the
correct short term fix.

The long term fix is probably to change this whole structure as a
bitfield of unsigned int, like we already do in the tcg code.

Aurelien

-- 
Aurelien Jarno                          GPG: 4096R/1DDD8C9B
aurelien@aurel32.net                 http://www.aurel32.net