Patchwork softfloat: make USE_SOFTFLOAT_STRUCT_TYPES compile

login
register
mail settings
Submitter Juan Quintela
Date March 20, 2012, 3:24 p.m.
Message ID <1332257065-31184-1-git-send-email-quintela@redhat.com>
Download mbox | patch
Permalink /patch/147810/
State New
Headers show

Comments

Juan Quintela - March 20, 2012, 3:24 p.m.
This change makes it compile and return the same value than the #undef one.

Signed-off-by: Juan Quintela <quintela@redhat.com>
---
 fpu/softfloat.c |    4 ++--
 1 files changed, 2 insertions(+), 2 deletions(-)
Peter Maydell - March 20, 2012, 3:36 p.m.
On 20 March 2012 15:24, Juan Quintela <quintela@redhat.com> wrote:
> This change makes it compile and return the same value than the #undef one.
>
> Signed-off-by: Juan Quintela <quintela@redhat.com>

Oops, that would have been my fault when I put in the fused multiply
accumulate support.

Reviewed-by: Peter Maydell <peter.maydell@linaro.org>

-- PMM
Andreas Färber - March 20, 2012, 3:41 p.m.
Am 20.03.2012 16:24, schrieb Juan Quintela:
> This change makes it compile and return the same value than the #undef one.
> 
> Signed-off-by: Juan Quintela <quintela@redhat.com>

Cool,

Acked-by: Andreas Färber <afaerber@suse.de>

Tested the default, non-struct version only.

Juan, would it make sense to add a configure option for this #define,
similar to --enable-debug-tcg?

Andreas

> ---
>  fpu/softfloat.c |    4 ++--
>  1 files changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/fpu/softfloat.c b/fpu/softfloat.c
> index 81a7d1a..a1f4527 100644
> --- a/fpu/softfloat.c
> +++ b/fpu/softfloat.c
> @@ -2219,7 +2219,7 @@ float32 float32_muladd(float32 a, float32 b, float32 c, int flags STATUS_PARAM)
>              }
>          }
>          /* Zero plus something non-zero : just return the something */
> -        return c ^ (signflip << 31);
> +        return make_float32(float32_val(c) ^ (signflip << 31));
>      }
> 
>      if (aExp == 0) {
> @@ -3772,7 +3772,7 @@ float64 float64_muladd(float64 a, float64 b, float64 c, int flags STATUS_PARAM)
>              }
>          }
>          /* Zero plus something non-zero : just return the something */
> -        return c ^ ((uint64_t)signflip << 63);
> +        return make_float64(float64_val(c) ^ ((uint64_t)signflip << 63));
>      }
> 
>      if (aExp == 0) {
Juan Quintela - March 20, 2012, 3:49 p.m.
Andreas Färber <afaerber@suse.de> wrote:
> Am 20.03.2012 16:24, schrieb Juan Quintela:
>> This change makes it compile and return the same value than the #undef one.
>> 
>> Signed-off-by: Juan Quintela <quintela@redhat.com>
>
> Cool,
>
> Acked-by: Andreas Färber <afaerber@suse.de>
>
> Tested the default, non-struct version only.
>
> Juan, would it make sense to add a configure option for this #define,
> similar to --enable-debug-tcg?

It depends of its users, clearly it was not used for quite long.  Put in
under --enabled-debug?  Notice that I have only used this _once_, so I
am not the best person to decide.

Later, Juan.
Peter Maydell - March 20, 2012, 4:11 p.m.
On 20 March 2012 15:41, Andreas Färber <afaerber@suse.de> wrote:
> Tested the default, non-struct version only.
>
> Juan, would it make sense to add a configure option for this #define,
> similar to --enable-debug-tcg?

We'd need to fix some of the other targets:
target-alpha/machine.c:25: error: invalid operands to binary - (have
‘uint64_t (*)[31]’ and ‘struct float64 (*)[31]’)
target-m68k/helper.c:540: error: conflicting types for ‘helper_f64_to_i32’
target-microblaze/op_helper.c:447: error: incompatible types when
assigning to type ‘uint32_t’ from type ‘float32’
target-mips/op_helper.c:2487: error: incompatible type for argument 1
of ‘float64_sqrt’
target-ppc/op_helper.c:1236: error: incompatible types when assigning
to type ‘uint64_t’ from type ‘float64’
gdbstub.c:1177: error: incompatible type for argument 2 of ‘stl_le_p’
target-s390x/op_helper.c:1056: error: incompatible types when
assigning to type ‘uint32_t’ from type ‘float32’
target-unicore32/translate.c:2094: error: aggregate value used where
an integer was expected

(one sample error message only from each; eg mips has tons)

A quick look at gcc object files suggests that we do generate
slightly worse code sometimes (a few extra insns here and there,
esp. for float16). I think we could reasonably turn on this
debug by default if --enable-debug is turned on (the way that
--enable-debug-tcg is turned on by --enable-debug).

I think we should either have this type checking happen when
you use --enable-debug or we should say we flat out don't care
about float32/uint32_t mismatches; the current situation just
means we accumulate errors without noticing.

-- PMM
Peter Maydell - April 16, 2012, 4:25 p.m.
On 20 March 2012 15:36, Peter Maydell <peter.maydell@linaro.org> wrote:
> On 20 March 2012 15:24, Juan Quintela <quintela@redhat.com> wrote:
>> This change makes it compile and return the same value than the #undef one.
>>
>> Signed-off-by: Juan Quintela <quintela@redhat.com>
>
> Oops, that would have been my fault when I put in the fused multiply
> accumulate support.
>
> Reviewed-by: Peter Maydell <peter.maydell@linaro.org>

Ping?

(original patch http://patchwork.ozlabs.org/patch/147810/)

-- PMM
Blue Swirl - April 21, 2012, 1:45 p.m.
On Mon, Apr 16, 2012 at 16:25, Peter Maydell <peter.maydell@linaro.org> wrote:
> On 20 March 2012 15:36, Peter Maydell <peter.maydell@linaro.org> wrote:
>> On 20 March 2012 15:24, Juan Quintela <quintela@redhat.com> wrote:
>>> This change makes it compile and return the same value than the #undef one.
>>>
>>> Signed-off-by: Juan Quintela <quintela@redhat.com>
>>
>> Oops, that would have been my fault when I put in the fused multiply
>> accumulate support.
>>
>> Reviewed-by: Peter Maydell <peter.maydell@linaro.org>
>
> Ping?
>
> (original patch http://patchwork.ozlabs.org/patch/147810/)

Thanks, applied (via patchwork, thanks for the ID).

>
> -- PMM

Patch

diff --git a/fpu/softfloat.c b/fpu/softfloat.c
index 81a7d1a..a1f4527 100644
--- a/fpu/softfloat.c
+++ b/fpu/softfloat.c
@@ -2219,7 +2219,7 @@  float32 float32_muladd(float32 a, float32 b, float32 c, int flags STATUS_PARAM)
             }
         }
         /* Zero plus something non-zero : just return the something */
-        return c ^ (signflip << 31);
+        return make_float32(float32_val(c) ^ (signflip << 31));
     }

     if (aExp == 0) {
@@ -3772,7 +3772,7 @@  float64 float64_muladd(float64 a, float64 b, float64 c, int flags STATUS_PARAM)
             }
         }
         /* Zero plus something non-zero : just return the something */
-        return c ^ ((uint64_t)signflip << 63);
+        return make_float64(float64_val(c) ^ ((uint64_t)signflip << 63));
     }

     if (aExp == 0) {