It's not actively built and when enabled things fail to compile. I'm
not sure the type-checking is really helping here. Seeing as we "own"
our softfloat now lets remove the cruft.

Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
---
 include/fpu/softfloat.h | 27 ---------------------------
 1 file changed, 27 deletions(-)

diff --git a/include/fpu/softfloat.h b/include/fpu/softfloat.h
index d5e99667b6..52af1412de 100644
--- a/include/fpu/softfloat.h
+++ b/include/fpu/softfloat.h
@@ -103,32 +103,6 @@ enum {
 /*----------------------------------------------------------------------------
 | Software IEC/IEEE floating-point types.
 *----------------------------------------------------------------------------*/
-/* Use structures for soft-float types.  This prevents accidentally mixing
-   them with native int/float types.  A sufficiently clever compiler and
-   sane ABI should be able to see though these structs.  However
-   x86/gcc 3.x seems to struggle a bit, so leave them disabled by default.  */
-//#define USE_SOFTFLOAT_STRUCT_TYPES
-#ifdef USE_SOFTFLOAT_STRUCT_TYPES
-typedef struct {
-    uint16_t v;
-} float16;
-#define float16_val(x) (((float16)(x)).v)
-#define make_float16(x) __extension__ ({ float16 f16_val = {x}; f16_val; })
-#define const_float16(x) { x }
-typedef struct {
-    uint32_t v;
-} float32;
-/* The cast ensures an error if the wrong type is passed.  */
-#define float32_val(x) (((float32)(x)).v)
-#define make_float32(x) __extension__ ({ float32 f32_val = {x}; f32_val; })
-#define const_float32(x) { x }
-typedef struct {
-    uint64_t v;
-} float64;
-#define float64_val(x) (((float64)(x)).v)
-#define make_float64(x) __extension__ ({ float64 f64_val = {x}; f64_val; })
-#define const_float64(x) { x }
-#else
 typedef uint16_t float16;
 typedef uint32_t float32;
 typedef uint64_t float64;
@@ -141,7 +115,6 @@ typedef uint64_t float64;
 #define const_float16(x) (x)
 #define const_float32(x) (x)
 #define const_float64(x) (x)
-#endif
 typedef struct {
     uint64_t low;
     uint16_t high;
-- 
2.15.1

Le 09/01/2018 à 13:22, Alex Bennée a écrit :
> It's not actively built and when enabled things fail to compile. I'm
> not sure the type-checking is really helping here. Seeing as we "own"
> our softfloat now lets remove the cruft.

I think it would be better to fix the build break than to remove the
type-checking tool.

but that's only my opinion...

Thanks,
Laurent

On 2018-01-09 13:27, Laurent Vivier wrote:
> Le 09/01/2018 à 13:22, Alex Bennée a écrit :
> > It's not actively built and when enabled things fail to compile. I'm
> > not sure the type-checking is really helping here. Seeing as we "own"
> > our softfloat now lets remove the cruft.
> 
> I think it would be better to fix the build break than to remove the
> type-checking tool.
> 
> but that's only my opinion...

I agree with that. Those checks are useful for targets which call host
floating point functions for some instructions. This is ugly, but that's
what is still done for at least x86 for the trigonometrical functions.
The check prevents assigning a float or double value to a softfloat type
without calling the conversion function.

Now, when we make sure that those ugly things are removed, I think these
type-checking might be removed.

Aurelien

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

On 9 January 2018 at 14:12, Aurelien Jarno <aurelien@aurel32.net> wrote:
> On 2018-01-09 13:27, Laurent Vivier wrote:
>> Le 09/01/2018 à 13:22, Alex Bennée a écrit :
>> > It's not actively built and when enabled things fail to compile. I'm
>> > not sure the type-checking is really helping here. Seeing as we "own"
>> > our softfloat now lets remove the cruft.
>>
>> I think it would be better to fix the build break than to remove the
>> type-checking tool.
>>
>> but that's only my opinion...
>
> I agree with that. Those checks are useful for targets which call host
> floating point functions for some instructions. This is ugly, but that's
> what is still done for at least x86 for the trigonometrical functions.
> The check prevents assigning a float or double value to a softfloat type
> without calling the conversion function.

Is gcc's codegen still bad enough that we have to default to not
using the type-checking versions? If so, maybe we could at least
enable the type-checking on an --enable-debug build, so it doesn't
bitrot all the time.

thanks
-- PMM

Le 09/01/2018 à 15:14, Peter Maydell a écrit :
> On 9 January 2018 at 14:12, Aurelien Jarno <aurelien@aurel32.net> wrote:
>> On 2018-01-09 13:27, Laurent Vivier wrote:
>>> Le 09/01/2018 à 13:22, Alex Bennée a écrit :
>>>> It's not actively built and when enabled things fail to compile. I'm
>>>> not sure the type-checking is really helping here. Seeing as we "own"
>>>> our softfloat now lets remove the cruft.
>>>
>>> I think it would be better to fix the build break than to remove the
>>> type-checking tool.
>>>
>>> but that's only my opinion...
>>
>> I agree with that. Those checks are useful for targets which call host
>> floating point functions for some instructions. This is ugly, but that's
>> what is still done for at least x86 for the trigonometrical functions.
>> The check prevents assigning a float or double value to a softfloat type
>> without calling the conversion function.
> 
> Is gcc's codegen still bad enough that we have to default to not
> using the type-checking versions? If so, maybe we could at least
> enable the type-checking on an --enable-debug build, so it doesn't
> bitrot all the time.

What means "bad enough"? for some targets it works fine.

The problem with that is if it is not enabled all the time it becomes
broken really quick...

BTW, if it doesn't break Alex's work I'm volunteer to fix
USE_SOFTFLOAT_STRUCT_TYPES build.

Thanks,
Laurent

On 9 January 2018 at 14:20, Laurent Vivier <laurent@vivier.eu> wrote:
> Le 09/01/2018 à 15:14, Peter Maydell a écrit :
>> Is gcc's codegen still bad enough that we have to default to not
>> using the type-checking versions? If so, maybe we could at least
>> enable the type-checking on an --enable-debug build, so it doesn't
>> bitrot all the time.
>
> What means "bad enough"? for some targets it works fine.

I mean whatever the problem was that made us write the comment
   A sufficiently clever compiler and
   sane ABI should be able to see though these structs.  However
   x86/gcc 3.x seems to struggle a bit, so leave them disabled by default.

In theory the code generated should be no worse than without structs...

> The problem with that is if it is not enabled all the time it becomes
> broken really quick...

Yes, that's why I'd like it at least default-enabled for --enable-debug,
if we can't enable it always.

thanks
-- PMM

On 01/09/2018 06:43 AM, Peter Maydell wrote:
> On 9 January 2018 at 14:20, Laurent Vivier <laurent@vivier.eu> wrote:
>> Le 09/01/2018 à 15:14, Peter Maydell a écrit :
>>> Is gcc's codegen still bad enough that we have to default to not
>>> using the type-checking versions? If so, maybe we could at least
>>> enable the type-checking on an --enable-debug build, so it doesn't
>>> bitrot all the time.
>>
>> What means "bad enough"? for some targets it works fine.
> 
> I mean whatever the problem was that made us write the comment
>    A sufficiently clever compiler and
>    sane ABI should be able to see though these structs.  However
>    x86/gcc 3.x seems to struggle a bit, so leave them disabled by default.

E.g. the i386 ABI is *not* sane in this respect.  Nor is sparcv8plus.  I'd have
to check on the others.

If we enable USE_SOFTFLOAT_STRUCT_TYPES, then we *must* remove the markup for
f32 and f64 from include/exec/helper-head.h, because structures are passed
differently from integers as parameters and/or return values.

I personally do not think USE_SOFTFLOAT_STRUCT_TYPES is worth the headache.


r~

Laurent Vivier <laurent@vivier.eu> writes:

> Le 09/01/2018 à 15:14, Peter Maydell a écrit:
>> On 9 January 2018 at 14:12, Aurelien Jarno <aurelien@aurel32.net> wrote:
>>> On 2018-01-09 13:27, Laurent Vivier wrote:
>>>> Le 09/01/2018 à 13:22, Alex Bennée a écrit :
>>>>> It's not actively built and when enabled things fail to compile. I'm
>>>>> not sure the type-checking is really helping here. Seeing as we "own"
>>>>> our softfloat now lets remove the cruft.
>>>>
>>>> I think it would be better to fix the build break than to remove the
>>>> type-checking tool.
>>>>
>>>> but that's only my opinion...
>>>
>>> I agree with that. Those checks are useful for targets which call host
>>> floating point functions for some instructions. This is ugly, but that's
>>> what is still done for at least x86 for the trigonometrical functions.
>>> The check prevents assigning a float or double value to a softfloat type
>>> without calling the conversion function.
>>
>> Is gcc's codegen still bad enough that we have to default to not
>> using the type-checking versions? If so, maybe we could at least
>> enable the type-checking on an --enable-debug build, so it doesn't
>> bitrot all the time.
>
> What means "bad enough"? for some targets it works fine.
>
> The problem with that is if it is not enabled all the time it becomes
> broken really quick...
>
> BTW, if it doesn't break Alex's work I'm volunteer to fix
> USE_SOFTFLOAT_STRUCT_TYPES build.

Be my guest - I suspect getting that merged would be on a faster path
than the rest of the softfloat re-factoring patch series (unless the
relative silence means everyone is happy with it ;-).

By the way I mentioned in my header mail that the types are included
from bswap.h so it would be nice to move the softfloat types somewhere
where they could be more easily included to avoid triggering a re-build
every time softfloat.h is touched.

--
Alex Bennée