[Qemu-devel] [PATCH v1 04/19] include/fpu/softfloat: implement float16_set_sign helper

Alex Bennée posted 19 patches 8 years, 2 months ago
There is a newer version of this series
[Qemu-devel] [PATCH v1 04/19] include/fpu/softfloat: implement float16_set_sign helper
Posted by Alex Bennée 8 years, 2 months ago
Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
---
 include/fpu/softfloat.h | 5 +++++
 1 file changed, 5 insertions(+)

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.
 *----------------------------------------------------------------------------*/
-- 
2.15.1


Re: [Qemu-devel] [PATCH v1 04/19] include/fpu/softfloat: implement float16_set_sign helper
Posted by Richard Henderson 8 years, 1 month ago
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~

Re: [Qemu-devel] [PATCH v1 04/19] include/fpu/softfloat: implement float16_set_sign helper
Posted by Alex Bennée 8 years, 1 month ago
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

Re: [Qemu-devel] [PATCH v1 04/19] include/fpu/softfloat: implement float16_set_sign helper
Posted by Alex Bennée 8 years, 1 month ago
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

Re: [Qemu-devel] [PATCH v1 04/19] include/fpu/softfloat: implement float16_set_sign helper
Posted by Richard Henderson 8 years, 1 month ago
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~

Re: [Qemu-devel] [PATCH v1 04/19] include/fpu/softfloat: implement float16_set_sign helper
Posted by Philippe Mathieu-Daudé 8 years, 1 month ago
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... :)