[PATCH 1/3] softfloat: Fix declaration of partsN_compare

Richard Henderson posted 3 patches 3 years, 10 months ago
Maintainers: Aurelien Jarno <aurelien@aurel32.net>, Peter Maydell <peter.maydell@linaro.org>, "Alex Bennée" <alex.bennee@linaro.org>
[PATCH 1/3] softfloat: Fix declaration of partsN_compare
Posted by Richard Henderson 3 years, 10 months ago
The declaration used 'int', while the definition used 'FloatRelation'.
This should have resulted in a compiler error, but mysteriously didn't.

Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
---
 fpu/softfloat.c | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/fpu/softfloat.c b/fpu/softfloat.c
index 7f524d4377..7e62fcf414 100644
--- a/fpu/softfloat.c
+++ b/fpu/softfloat.c
@@ -874,10 +874,10 @@ static FloatParts128 *parts128_minmax(FloatParts128 *a, FloatParts128 *b,
 #define parts_minmax(A, B, S, F) \
     PARTS_GENERIC_64_128(minmax, A)(A, B, S, F)
 
-static int parts64_compare(FloatParts64 *a, FloatParts64 *b,
-                           float_status *s, bool q);
-static int parts128_compare(FloatParts128 *a, FloatParts128 *b,
-                            float_status *s, bool q);
+static FloatRelation parts64_compare(FloatParts64 *a, FloatParts64 *b,
+                                     float_status *s, bool q);
+static FloatRelation parts128_compare(FloatParts128 *a, FloatParts128 *b,
+                                      float_status *s, bool q);
 
 #define parts_compare(A, B, S, Q) \
     PARTS_GENERIC_64_128(compare, A)(A, B, S, Q)
-- 
2.25.1
Re: [PATCH 1/3] softfloat: Fix declaration of partsN_compare
Posted by Richard Henderson 3 years, 10 months ago
On 4/1/22 07:22, Richard Henderson wrote:
> The declaration used 'int', while the definition used 'FloatRelation'.
> This should have resulted in a compiler error, but mysteriously didn't.

Bah, of course there's no error -- this is C not C++.

The enumeration has values -1 ... 2, which means that the enumeration is compatible with 
an implementation defined signed integer type, which for our set of hosts will be 'int'. 
So, no conflict.

I'll edit the commit message.


r~
Re: [PATCH 1/3] softfloat: Fix declaration of partsN_compare
Posted by Peter Maydell 3 years, 10 months ago
On Fri, 1 Apr 2022 at 19:08, Richard Henderson
<richard.henderson@linaro.org> wrote:
>
> On 4/1/22 07:22, Richard Henderson wrote:
> > The declaration used 'int', while the definition used 'FloatRelation'.
> > This should have resulted in a compiler error, but mysteriously didn't.
>
> Bah, of course there's no error -- this is C not C++.
>
> The enumeration has values -1 ... 2, which means that the enumeration is compatible with
> an implementation defined signed integer type, which for our set of hosts will be 'int'.
> So, no conflict.

The types are compatible, but it's weird that the compiler doesn't
warn that the prototype and definition are different types: it
seems like the kind of "technically valid but usually a bug" that
a warning would be nice for.

-- PMM
Re: [PATCH 1/3] softfloat: Fix declaration of partsN_compare
Posted by Richard Henderson 3 years, 10 months ago
On 4/1/22 12:12, Peter Maydell wrote:
> The types are compatible, but it's weird that the compiler doesn't
> warn that the prototype and definition are different types: it
> seems like the kind of "technically valid but usually a bug" that
> a warning would be nice for.

Good point. Submitted

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=105131
https://github.com/llvm/llvm-project/issues/54706


r~
Re: [PATCH 1/3] softfloat: Fix declaration of partsN_compare
Posted by Peter Maydell 3 years, 10 months ago
On Fri, 1 Apr 2022 at 14:22, Richard Henderson
<richard.henderson@linaro.org> wrote:
>
> The declaration used 'int', while the definition used 'FloatRelation'.
> This should have resulted in a compiler error, but mysteriously didn't.
>
> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
> ---
>  fpu/softfloat.c | 8 ++++----
>  1 file changed, 4 insertions(+), 4 deletions(-)
Reviewed-by: Peter Maydell <peter.maydell@linaro.org>

thanks
-- PMM