| Message ID | 20171211125705.16120-5-alex.bennee@linaro.org |
|---|---|
| State | Superseded |
| Headers | show |
| Series | re-factor softfloat and add fp16 functions | expand |
On 12/11/2017 04:56 AM, Alex Bennée wrote: > +static inline float16 float16_set_sign(float16 a, int sign) > +{ > + return make_float16((float16_val(a) & 0x7fff) | (sign << 15)); > +} > + 1) Do we use this anywhere? 2) While this is probably in line with the other implementations, but going to a more qemu-ish style this should use deposit32. Anyway, Reviewed-by: Richard Henderson <richard.henderson@linaro.org> r~
Richard Henderson <richard.henderson@linaro.org> writes: > On 12/11/2017 04:56 AM, Alex Bennée wrote: >> +static inline float16 float16_set_sign(float16 a, int sign) >> +{ >> + return make_float16((float16_val(a) & 0x7fff) | (sign << 15)); >> +} >> + > > 1) Do we use this anywhere? Yes in the target specific helpers > > 2) While this is probably in line with the other implementations, > but going to a more qemu-ish style this should use deposit32. OK, will do. > > Anyway, > > Reviewed-by: Richard Henderson <richard.henderson@linaro.org> > > > r~ -- Alex Bennée
On 12/18/2017 06:44 PM, Richard Henderson wrote: > On 12/11/2017 04:56 AM, Alex Bennée wrote: >> +static inline float16 float16_set_sign(float16 a, int sign) >> +{ >> + return make_float16((float16_val(a) & 0x7fff) | (sign << 15)); >> +} >> + > > 1) Do we use this anywhere? > > 2) While this is probably in line with the other implementations, > but going to a more qemu-ish style this should use deposit32. Yes, it is easier to read while reviewing (no mask or shift), so safer. That's probably why I'm becoming addict of the "hw/registerfields.h" API... :)
Alex Bennée <alex.bennee@linaro.org> writes: > Richard Henderson <richard.henderson@linaro.org> writes: > >> On 12/11/2017 04:56 AM, Alex Bennée wrote: >>> +static inline float16 float16_set_sign(float16 a, int sign) >>> +{ >>> + return make_float16((float16_val(a) & 0x7fff) | (sign << 15)); >>> +} >>> + >> >> 1) Do we use this anywhere? > > Yes in the target specific helpers > >> >> 2) While this is probably in line with the other implementations, >> but going to a more qemu-ish style this should use deposit32. > > OK, will do. > It turns out doing this unleashes a weird circular dependency at we need qemu/bitops.h but that brings in host-utils.h and bswap.h which tries to include softfloat.h again. CHK version_gen.h CC qga/main.o In file included from /home/alex/lsrc/qemu/qemu.git/include/qemu/bitops.h:16:0, from /home/alex/lsrc/qemu/qemu.git/include/fpu/softfloat.h:85, from /home/alex/lsrc/qemu/qemu.git/include/qemu/bswap.h:4, from qga/main.c:28: /home/alex/lsrc/qemu/qemu.git/include/qemu/host-utils.h: In function ‘revbit16’: /home/alex/lsrc/qemu/qemu.git/include/qemu/host-utils.h:293:9: error: implicit declaration of function ‘bswap16’ [-Werror=implicit-function-declaration] x = bswap16(x); ^ /home/alex/lsrc/qemu/qemu.git/include/qemu/host-utils.h:293:5: error: nested extern declaration of ‘bswap16’ [-Werror=nested-externs] x = bswap16(x); ^ /home/alex/lsrc/qemu/qemu.git/include/qemu/host-utils.h: In function ‘revbit32’: /home/alex/lsrc/qemu/qemu.git/include/qemu/host-utils.h:312:9: error: implicit declaration of function ‘bswap32’ [-Werror=implicit-function-declaration] x = bswap32(x); ^ /home/alex/lsrc/qemu/qemu.git/include/qemu/host-utils.h:312:5: error: nested extern declaration of ‘bswap32’ [-Werror=nested-externs] x = bswap32(x); ^ /home/alex/lsrc/qemu/qemu.git/include/qemu/host-utils.h: In function ‘revbit64’: /home/alex/lsrc/qemu/qemu.git/include/qemu/host-utils.h:331:9: error: implicit declaration of function ‘bswap64’ [-Werror=implicit-function-declaration] x = bswap64(x); ^ /home/alex/lsrc/qemu/qemu.git/include/qemu/host-utils.h:331:5: error: nested extern declaration of ‘bswap64’ [-Werror=nested-externs] x = bswap64(x); ^ In file included from qga/main.c:28:0: /home/alex/lsrc/qemu/qemu.git/include/qemu/bswap.h: At top level: /home/alex/lsrc/qemu/qemu.git/include/qemu/bswap.h:14:24: error: conflicting types for ‘bswap16’ static inline uint16_t bswap16(uint16_t x) ^ In file included from /home/alex/lsrc/qemu/qemu.git/include/qemu/bitops.h:16:0, from /home/alex/lsrc/qemu/qemu.git/include/fpu/softfloat.h:85, from /home/alex/lsrc/qemu/qemu.git/include/qemu/bswap.h:4, from qga/main.c:28: /home/alex/lsrc/qemu/qemu.git/include/qemu/host-utils.h:293:9: note: previous implicit declaration of ‘bswap16’ was here x = bswap16(x); ^ In file included from qga/main.c:28:0: /home/alex/lsrc/qemu/qemu.git/include/qemu/bswap.h:19:24: error: conflicting types for ‘bswap32’ static inline uint32_t bswap32(uint32_t x) ^ In file included from /home/alex/lsrc/qemu/qemu.git/include/qemu/bitops.h:16:0, from /home/alex/lsrc/qemu/qemu.git/include/fpu/softfloat.h:85, from /home/alex/lsrc/qemu/qemu.git/include/qemu/bswap.h:4, from qga/main.c:28: /home/alex/lsrc/qemu/qemu.git/include/qemu/host-utils.h:312:9: note: previous implicit declaration of ‘bswap32’ was here x = bswap32(x); ^ In file included from qga/main.c:28:0: /home/alex/lsrc/qemu/qemu.git/include/qemu/bswap.h:24:24: error: conflicting types for ‘bswap64’ static inline uint64_t bswap64(uint64_t x) ^ In file included from /home/alex/lsrc/qemu/qemu.git/include/qemu/bitops.h:16:0, from /home/alex/lsrc/qemu/qemu.git/include/fpu/softfloat.h:85, from /home/alex/lsrc/qemu/qemu.git/include/qemu/bswap.h:4, from qga/main.c:28: /home/alex/lsrc/qemu/qemu.git/include/qemu/host-utils.h:331:9: note: previous implicit declaration of ‘bswap64’ was here x = bswap64(x); ^ cc1: all warnings being treated as errors /home/alex/lsrc/qemu/qemu.git/rules.mak:66: recipe for target 'qga/main.o' failed make: *** [qga/main.o] Error 1 Compilation exited abnormally with code 2 at Mon Jan 8 12:57:41 -- Alex Bennée
On 01/08/2018 04:58 AM, Alex Bennée wrote: > > Alex Bennée <alex.bennee@linaro.org> writes: > >> Richard Henderson <richard.henderson@linaro.org> writes: >> >>> On 12/11/2017 04:56 AM, Alex Bennée wrote: >>>> +static inline float16 float16_set_sign(float16 a, int sign) >>>> +{ >>>> + return make_float16((float16_val(a) & 0x7fff) | (sign << 15)); >>>> +} >>>> + >>> >>> 1) Do we use this anywhere? >> >> Yes in the target specific helpers >> >>> >>> 2) While this is probably in line with the other implementations, >>> but going to a more qemu-ish style this should use deposit32. >> >> OK, will do. >> > > It turns out doing this unleashes a weird circular dependency at we need > qemu/bitops.h but that brings in host-utils.h and bswap.h which tries > to include softfloat.h again. Bah. Just ignore this request for now then. For future cleanup, I'm sure that bswap.h includes softfloat.h for the float32/float64 typedefs. We should move those out somewhere else -- probably qemu/typedefs.h. Which probably drops the number of objects that depend on softfloat.h by a factor of 100. r~
diff --git a/include/fpu/softfloat.h b/include/fpu/softfloat.h index 32036382c6..17dfe60dbd 100644 --- a/include/fpu/softfloat.h +++ b/include/fpu/softfloat.h @@ -390,6 +390,11 @@ static inline float16 float16_chs(float16 a) return make_float16(float16_val(a) ^ 0x8000); } +static inline float16 float16_set_sign(float16 a, int sign) +{ + return make_float16((float16_val(a) & 0x7fff) | (sign << 15)); +} + /*---------------------------------------------------------------------------- | The pattern for a default generated half-precision NaN. *----------------------------------------------------------------------------*/
Signed-off-by: Alex Bennée <alex.bennee@linaro.org> --- include/fpu/softfloat.h | 5 +++++ 1 file changed, 5 insertions(+) -- 2.15.1