| Message ID | 1388496958-3542-4-git-send-email-peter.maydell@linaro.org |
|---|---|
| State | New |
| Headers | show |
On 12/31/2013 05:35 AM, Peter Maydell wrote: > +/* We provide the int16 versions for symmetry of API with float-to-int */ > +INLINE float32 int16_to_float32(int_fast16_t v STATUS_PARAM) > +{ > + return int32_to_float32(v STATUS_VAR); > +} If you're going to have int16 versions, I don't think you should use int_fast16_t, but rather int16_t so that we will properly truncate the incoming value. Otherwise there's not much point in having these, IMO. And please add blank lines between the functions. r~
On 31 December 2013 14:21, Richard Henderson <rth@twiddle.net> wrote: > On 12/31/2013 05:35 AM, Peter Maydell wrote: >> +/* We provide the int16 versions for symmetry of API with float-to-int */ >> +INLINE float32 int16_to_float32(int_fast16_t v STATUS_PARAM) >> +{ >> + return int32_to_float32(v STATUS_VAR); >> +} > > If you're going to have int16 versions, I don't think you should use > int_fast16_t, but rather int16_t so that we will properly truncate the incoming > value. Otherwise there's not much point in having these, IMO. I was going for consistency with the convert-to-int16, which return int_fast16_t. The benefit of having these functions is that the caller doesn't have to effectively have an extra bit of logic in its macros to say "conversion from 32 bit int is int32_to_float32, conversion from 64 bit int is int64_to_float32, but conversion from 16 bit int is not int16_to_float32". The existing int32_to_float* functions take int32, not int32_t, so this is the same semantics. You could argue that it would be better for all of them to take the exact type rather than the at-least-this-big type (it would let me drop a cast in the ARM code that calls these), I suppose. > And please add blank lines between the functions. Sure. thanks -- PMM
On 12/31/2013 06:27 AM, Peter Maydell wrote: > I was going for consistency with the convert-to-int16, which return > int_fast16_t. Perhaps, but in that case we've properly bounded the result; it's guaranteed to be in range of int16_t. > The existing int32_to_float* functions take int32, not int32_t, > so this is the same semantics. You could argue that it would > be better for all of them to take the exact type rather than > the at-least-this-big type (it would let me drop a cast in the > ARM code that calls these), I suppose. Yes, that's what I'm arguing. Of course, I'd forgotten that we already have that problem, since my eyes refuse to see the lack of "_t" there. But is that existing bug any reason to extend the problem? One of these days we should just clean up all the crap formatting, bogus types, and stupid STATUS_* macros... r~
On 31 December 2013 14:35, Richard Henderson <rth@twiddle.net> wrote: > On 12/31/2013 06:27 AM, Peter Maydell wrote: >> The existing int32_to_float* functions take int32, not int32_t, >> so this is the same semantics. You could argue that it would >> be better for all of them to take the exact type rather than >> the at-least-this-big type (it would let me drop a cast in the >> ARM code that calls these), I suppose. > > Yes, that's what I'm arguing. Of course, I'd forgotten that we already have > that problem, since my eyes refuse to see the lack of "_t" there. But is that > existing bug any reason to extend the problem? OK, let's make this one return int16_t. I may throw in an extra patch making the other int-to-float routines take the precise types too. > One of these days we should just clean up all the crap formatting, bogus types, > and stupid STATUS_* macros... Yes, the STATUS_ stuff is definitely on my list to zap, once we get all this stuff reviewed and committed (since it would otherwise cause conflicts all over the place). I think the standing question about the types is whether we should be converting int32 to int32_t or int_fast32_t (and indeed whether it makes much performance difference). That's why we have the current setup where some of the intfoo got changed to int_fastfoo_t and some didn't. My other hope for this year is that we can clear up the relicensing stuff so I don't have to keep prodding people about specifying the 2a-or-2b stuff on new patches... thanks -- PMM
diff --git a/include/fpu/softfloat.h b/include/fpu/softfloat.h index a9b8cd9..3790b10 100644 --- a/include/fpu/softfloat.h +++ b/include/fpu/softfloat.h @@ -238,6 +238,23 @@ float64 uint64_to_float64( uint64 STATUS_PARAM ); floatx80 int64_to_floatx80( int64 STATUS_PARAM ); float128 int64_to_float128( int64 STATUS_PARAM ); float128 uint64_to_float128( uint64 STATUS_PARAM ); +/* We provide the int16 versions for symmetry of API with float-to-int */ +INLINE float32 int16_to_float32(int_fast16_t v STATUS_PARAM) +{ + return int32_to_float32(v STATUS_VAR); +} +INLINE float32 uint16_to_float32(uint_fast16_t v STATUS_PARAM) +{ + return uint32_to_float32(v STATUS_VAR); +} +INLINE float64 int16_to_float64(int_fast16_t v STATUS_PARAM) +{ + return int32_to_float64(v STATUS_VAR); +} +INLINE float64 uint16_to_float64(uint_fast16_t v STATUS_PARAM) +{ + return uint32_to_float64(v STATUS_VAR); +} /*---------------------------------------------------------------------------- | Software half-precision conversion routines.
Add the float to 16 bit integer conversion routines. These can be trivially implemented in terms of the int32_to_float* routines, but providing them makes our API more symmetrical and can simplify callers. Signed-off-by: Peter Maydell <peter.maydell@linaro.org> --- include/fpu/softfloat.h | 17 +++++++++++++++++ 1 file changed, 17 insertions(+)