[PATCH] configure: Check whether we can compile the s390-ccw bios with -msoft-float

[PATCH] configure: Check whether we can compile the s390-ccw bios with -msoft-float

[Thomas Huth]

The -msoft-float switch is not available in older versions of Clang.
Since we rely on the compiler to not generate floating point instructions
unexpectedly, we block those old compilers now via a test in the configure
script. Note that for some weird reasons, the Clang compiler only complains
about the missing soft-float support if no other flags are passed via
"-Wl,..." to the linker. So we have to use "compile_object" instead of
"compile_prog" for this check.

Signed-off-by: Thomas Huth <thuth@redhat.com>
---
 configure | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/configure b/configure
index 676239c697..673419ff31 100755
--- a/configure
+++ b/configure
@@ -5462,7 +5462,7 @@ if test "$cpu" = "s390x" ; then
   write_c_skeleton
   compile_prog "-march=z900" ""
   has_z900=$?
-  if [ $has_z900 = 0 ] || compile_prog "-march=z10" ""; then
+  if [ $has_z900 = 0 ] || compile_object "-march=z10 -msoft-float -Werror"; then
     if [ $has_z900 != 0 ]; then
       echo "WARNING: Your compiler does not support the z900!"
       echo "         The s390-ccw bios will only work with guest CPUs >= z10."
-- 
2.27.0

Re: [PATCH] configure: Check whether we can compile the s390-ccw bios with -msoft-float

[Cornelia Huck]

On Tue, 25 May 2021 16:20:32 +0200
Thomas Huth <thuth@redhat.com> wrote:

> The -msoft-float switch is not available in older versions of Clang.
> Since we rely on the compiler to not generate floating point instructions
> unexpectedly, we block those old compilers now via a test in the configure
> script. Note that for some weird reasons, the Clang compiler only complains
> about the missing soft-float support if no other flags are passed via
> "-Wl,..." to the linker. So we have to use "compile_object" instead of
> "compile_prog" for this check.

Ugh.

> 
> Signed-off-by: Thomas Huth <thuth@redhat.com>
> ---
>  configure | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/configure b/configure
> index 676239c697..673419ff31 100755
> --- a/configure
> +++ b/configure
> @@ -5462,7 +5462,7 @@ if test "$cpu" = "s390x" ; then
>    write_c_skeleton
>    compile_prog "-march=z900" ""
>    has_z900=$?
> -  if [ $has_z900 = 0 ] || compile_prog "-march=z10" ""; then
> +  if [ $has_z900 = 0 ] || compile_object "-march=z10 -msoft-float -Werror"; then

Do you believe that we should have caught the various clang
specialties now?

>      if [ $has_z900 != 0 ]; then
>        echo "WARNING: Your compiler does not support the z900!"
>        echo "         The s390-ccw bios will only work with guest CPUs >= z10."

Re: [PATCH] configure: Check whether we can compile the s390-ccw bios with -msoft-float

[Thomas Huth]

On 25/05/2021 16.31, Cornelia Huck wrote:
> On Tue, 25 May 2021 16:20:32 +0200
> Thomas Huth <thuth@redhat.com> wrote:
> 
>> The -msoft-float switch is not available in older versions of Clang.
>> Since we rely on the compiler to not generate floating point instructions
>> unexpectedly, we block those old compilers now via a test in the configure
>> script. Note that for some weird reasons, the Clang compiler only complains
>> about the missing soft-float support if no other flags are passed via
>> "-Wl,..." to the linker. So we have to use "compile_object" instead of
>> "compile_prog" for this check.
> 
> Ugh.

It's maybe better to use compile_object for testing -msoft-float anyway 
since it could influence the way of linking against libraries (if I get 
https://reviews.llvm.org/D72189 right).

>>
>> Signed-off-by: Thomas Huth <thuth@redhat.com>
>> ---
>>   configure | 2 +-
>>   1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/configure b/configure
>> index 676239c697..673419ff31 100755
>> --- a/configure
>> +++ b/configure
>> @@ -5462,7 +5462,7 @@ if test "$cpu" = "s390x" ; then
>>     write_c_skeleton
>>     compile_prog "-march=z900" ""
>>     has_z900=$?
>> -  if [ $has_z900 = 0 ] || compile_prog "-march=z10" ""; then
>> +  if [ $has_z900 = 0 ] || compile_object "-march=z10 -msoft-float -Werror"; then
> 
> Do you believe that we should have caught the various clang
> specialties now?

Apart from one compiler warning that just popped up today, yes. I'm seeing 
this compiler warning with Clang 12.0 now:

roms/SLOF/lib/libnet/ipv6.c:447:18: warning: variable length array folded to 
constant array as an extension [-Wgnu-folding-constant]
                 unsigned short raw[ip6size];
                                ^

... not sure what to do with that one yet.

  Thomas

Re: [PATCH] configure: Check whether we can compile the s390-ccw bios with -msoft-float

[Philippe Mathieu-Daudé]

On 5/25/21 4:40 PM, Thomas Huth wrote:
> On 25/05/2021 16.31, Cornelia Huck wrote:
>> On Tue, 25 May 2021 16:20:32 +0200
>> Thomas Huth <thuth@redhat.com> wrote:
>>
>>> The -msoft-float switch is not available in older versions of Clang.
>>> Since we rely on the compiler to not generate floating point
>>> instructions
>>> unexpectedly, we block those old compilers now via a test in the
>>> configure
>>> script. Note that for some weird reasons, the Clang compiler only
>>> complains
>>> about the missing soft-float support if no other flags are passed via
>>> "-Wl,..." to the linker. So we have to use "compile_object" instead of
>>> "compile_prog" for this check.
>>
>> Ugh.
> 
> It's maybe better to use compile_object for testing -msoft-float anyway
> since it could influence the way of linking against libraries (if I get
> https://reviews.llvm.org/D72189 right).
> 
>>>
>>> Signed-off-by: Thomas Huth <thuth@redhat.com>
>>> ---
>>>   configure | 2 +-
>>>   1 file changed, 1 insertion(+), 1 deletion(-)
>>>
>>> diff --git a/configure b/configure
>>> index 676239c697..673419ff31 100755
>>> --- a/configure
>>> +++ b/configure
>>> @@ -5462,7 +5462,7 @@ if test "$cpu" = "s390x" ; then
>>>     write_c_skeleton
>>>     compile_prog "-march=z900" ""
>>>     has_z900=$?
>>> -  if [ $has_z900 = 0 ] || compile_prog "-march=z10" ""; then
>>> +  if [ $has_z900 = 0 ] || compile_object "-march=z10 -msoft-float
>>> -Werror"; then
>>
>> Do you believe that we should have caught the various clang
>> specialties now?
> 
> Apart from one compiler warning that just popped up today, yes. I'm
> seeing this compiler warning with Clang 12.0 now:
> 
> roms/SLOF/lib/libnet/ipv6.c:447:18: warning: variable length array
> folded to constant array as an extension [-Wgnu-folding-constant]
>                 unsigned short raw[ip6size];
>                                ^
> 
> ... not sure what to do with that one yet.

Fix it? =)

Same class as:
https://gitlab.freedesktop.org/slirp/libslirp/-/commit/6690d55626cc

-- >8 --
diff --git a/lib/libnet/ipv6.c b/lib/libnet/ipv6.c
index 6420004..b7daeda 100644
--- a/lib/libnet/ipv6.c
+++ b/lib/libnet/ipv6.c
@@ -441,10 +441,9 @@ static unsigned short ip6_checksum(struct ip6hdr
*ip6h, unsigned char *packet,
 {
        int i;
        unsigned long checksum;
-       const int ip6size = sizeof(struct ip6hdr)/sizeof(unsigned short);
        union {
                struct ip6hdr ip6h;
-               unsigned short raw[ip6size];
+               unsigned short raw[sizeof(struct ip6hdr) /
sizeof(unsigned short)];
        } pseudo;

        memcpy (&pseudo.ip6h, ip6h, sizeof(struct ip6hdr));
@@ -455,7 +454,7 @@ static unsigned short ip6_checksum(struct ip6hdr
*ip6h, unsigned char *packet,
        for (checksum = 0, i = 0; i < bytes; i += 2)
                checksum += (packet[i] << 8) | packet[i + 1];

-       for (i = 0; i < ip6size; i++)
+       for (i = 0; i < sizeof(pseudo.raw); i++)
                checksum += pseudo.raw[i];

        checksum = (checksum >> 16) + (checksum & 0xffff);
---

Do you want me to send the fix since I have it ready?

Re: [PATCH] configure: Check whether we can compile the s390-ccw bios with -msoft-float

[Philippe Mathieu-Daudé]

On 5/25/21 5:13 PM, Philippe Mathieu-Daudé wrote:
> On 5/25/21 4:40 PM, Thomas Huth wrote:
>> On 25/05/2021 16.31, Cornelia Huck wrote:
>>> On Tue, 25 May 2021 16:20:32 +0200
>>> Thomas Huth <thuth@redhat.com> wrote:
>>>
>>>> The -msoft-float switch is not available in older versions of Clang.
>>>> Since we rely on the compiler to not generate floating point
>>>> instructions
>>>> unexpectedly, we block those old compilers now via a test in the
>>>> configure
>>>> script. Note that for some weird reasons, the Clang compiler only
>>>> complains
>>>> about the missing soft-float support if no other flags are passed via
>>>> "-Wl,..." to the linker. So we have to use "compile_object" instead of
>>>> "compile_prog" for this check.
>>>
>>> Ugh.
>>
>> It's maybe better to use compile_object for testing -msoft-float anyway
>> since it could influence the way of linking against libraries (if I get
>> https://reviews.llvm.org/D72189 right).
>>
>>>>
>>>> Signed-off-by: Thomas Huth <thuth@redhat.com>
>>>> ---
>>>>   configure | 2 +-
>>>>   1 file changed, 1 insertion(+), 1 deletion(-)
>>>>
>>>> diff --git a/configure b/configure
>>>> index 676239c697..673419ff31 100755
>>>> --- a/configure
>>>> +++ b/configure
>>>> @@ -5462,7 +5462,7 @@ if test "$cpu" = "s390x" ; then
>>>>     write_c_skeleton
>>>>     compile_prog "-march=z900" ""
>>>>     has_z900=$?
>>>> -  if [ $has_z900 = 0 ] || compile_prog "-march=z10" ""; then
>>>> +  if [ $has_z900 = 0 ] || compile_object "-march=z10 -msoft-float
>>>> -Werror"; then
>>>
>>> Do you believe that we should have caught the various clang
>>> specialties now?
>>
>> Apart from one compiler warning that just popped up today, yes. I'm
>> seeing this compiler warning with Clang 12.0 now:
>>
>> roms/SLOF/lib/libnet/ipv6.c:447:18: warning: variable length array
>> folded to constant array as an extension [-Wgnu-folding-constant]
>>                 unsigned short raw[ip6size];
>>                                ^
>>
>> ... not sure what to do with that one yet.

See Eric's explanation:
https://www.mail-archive.com/qemu-block@nongnu.org/msg84461.html

> 
> Fix it? =)
> 
> Same class as:
> https://gitlab.freedesktop.org/slirp/libslirp/-/commit/6690d55626cc
> 
> -- >8 --
> diff --git a/lib/libnet/ipv6.c b/lib/libnet/ipv6.c
> index 6420004..b7daeda 100644
> --- a/lib/libnet/ipv6.c
> +++ b/lib/libnet/ipv6.c
> @@ -441,10 +441,9 @@ static unsigned short ip6_checksum(struct ip6hdr
> *ip6h, unsigned char *packet,
>  {
>         int i;
>         unsigned long checksum;
> -       const int ip6size = sizeof(struct ip6hdr)/sizeof(unsigned short);
>         union {
>                 struct ip6hdr ip6h;
> -               unsigned short raw[ip6size];
> +               unsigned short raw[sizeof(struct ip6hdr) /
> sizeof(unsigned short)];
>         } pseudo;
> 
>         memcpy (&pseudo.ip6h, ip6h, sizeof(struct ip6hdr));
> @@ -455,7 +454,7 @@ static unsigned short ip6_checksum(struct ip6hdr
> *ip6h, unsigned char *packet,
>         for (checksum = 0, i = 0; i < bytes; i += 2)
>                 checksum += (packet[i] << 8) | packet[i + 1];
> 
> -       for (i = 0; i < ip6size; i++)
> +       for (i = 0; i < sizeof(pseudo.raw); i++)
>                 checksum += pseudo.raw[i];
> 
>         checksum = (checksum >> 16) + (checksum & 0xffff);
> ---
> 
> Do you want me to send the fix since I have it ready?
> 

Re: [PATCH] configure: Check whether we can compile the s390-ccw bios with -msoft-float

[Thomas Huth]

On 25/05/2021 17.13, Philippe Mathieu-Daudé wrote:
> On 5/25/21 4:40 PM, Thomas Huth wrote:
>> On 25/05/2021 16.31, Cornelia Huck wrote:
>>> On Tue, 25 May 2021 16:20:32 +0200
>>> Thomas Huth <thuth@redhat.com> wrote:
>>>
>>>> The -msoft-float switch is not available in older versions of Clang.
>>>> Since we rely on the compiler to not generate floating point
>>>> instructions
>>>> unexpectedly, we block those old compilers now via a test in the
>>>> configure
>>>> script. Note that for some weird reasons, the Clang compiler only
>>>> complains
>>>> about the missing soft-float support if no other flags are passed via
>>>> "-Wl,..." to the linker. So we have to use "compile_object" instead of
>>>> "compile_prog" for this check.
>>>
>>> Ugh.
>>
>> It's maybe better to use compile_object for testing -msoft-float anyway
>> since it could influence the way of linking against libraries (if I get
>> https://reviews.llvm.org/D72189 right).
>>
>>>>
>>>> Signed-off-by: Thomas Huth <thuth@redhat.com>
>>>> ---
>>>>    configure | 2 +-
>>>>    1 file changed, 1 insertion(+), 1 deletion(-)
>>>>
>>>> diff --git a/configure b/configure
>>>> index 676239c697..673419ff31 100755
>>>> --- a/configure
>>>> +++ b/configure
>>>> @@ -5462,7 +5462,7 @@ if test "$cpu" = "s390x" ; then
>>>>      write_c_skeleton
>>>>      compile_prog "-march=z900" ""
>>>>      has_z900=$?
>>>> -  if [ $has_z900 = 0 ] || compile_prog "-march=z10" ""; then
>>>> +  if [ $has_z900 = 0 ] || compile_object "-march=z10 -msoft-float
>>>> -Werror"; then
>>>
>>> Do you believe that we should have caught the various clang
>>> specialties now?
>>
>> Apart from one compiler warning that just popped up today, yes. I'm
>> seeing this compiler warning with Clang 12.0 now:
>>
>> roms/SLOF/lib/libnet/ipv6.c:447:18: warning: variable length array
>> folded to constant array as an extension [-Wgnu-folding-constant]
>>                  unsigned short raw[ip6size];
>>                                 ^
>>
>> ... not sure what to do with that one yet.
> 
> Fix it? =)
> 
> Same class as:
> https://gitlab.freedesktop.org/slirp/libslirp/-/commit/6690d55626cc
> 
> -- >8 --
> diff --git a/lib/libnet/ipv6.c b/lib/libnet/ipv6.c
> index 6420004..b7daeda 100644
> --- a/lib/libnet/ipv6.c
> +++ b/lib/libnet/ipv6.c
> @@ -441,10 +441,9 @@ static unsigned short ip6_checksum(struct ip6hdr
> *ip6h, unsigned char *packet,
>   {
>          int i;
>          unsigned long checksum;
> -       const int ip6size = sizeof(struct ip6hdr)/sizeof(unsigned short);
>          union {
>                  struct ip6hdr ip6h;
> -               unsigned short raw[ip6size];
> +               unsigned short raw[sizeof(struct ip6hdr) /
> sizeof(unsigned short)];
>          } pseudo;
> 
>          memcpy (&pseudo.ip6h, ip6h, sizeof(struct ip6hdr));
> @@ -455,7 +454,7 @@ static unsigned short ip6_checksum(struct ip6hdr
> *ip6h, unsigned char *packet,
>          for (checksum = 0, i = 0; i < bytes; i += 2)
>                  checksum += (packet[i] << 8) | packet[i + 1];
> 
> -       for (i = 0; i < ip6size; i++)
> +       for (i = 0; i < sizeof(pseudo.raw); i++)
>                  checksum += pseudo.raw[i];
> 
>          checksum = (checksum >> 16) + (checksum & 0xffff);
> ---
> 
> Do you want me to send the fix since I have it ready?

Yes, please!

  Thanks,
   Thomas

Re: [PATCH] configure: Check whether we can compile the s390-ccw bios with -msoft-float

[Cornelia Huck]

On Tue, May 25 2021, Thomas Huth <thuth@redhat.com> wrote:

> The -msoft-float switch is not available in older versions of Clang.
> Since we rely on the compiler to not generate floating point instructions
> unexpectedly, we block those old compilers now via a test in the configure
> script. Note that for some weird reasons, the Clang compiler only complains
> about the missing soft-float support if no other flags are passed via
> "-Wl,..." to the linker. So we have to use "compile_object" instead of
> "compile_prog" for this check.
>
> Signed-off-by: Thomas Huth <thuth@redhat.com>
> ---
>  configure | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)

Thanks, applied.