RE: [RFC PATCH v4 00/29] Hexagon patch series

Taylor Simpson posted 29 patches 3 years, 7 months ago
Only 0 patches received!
There is a newer version of this series
RE: [RFC PATCH v4 00/29] Hexagon patch series
Posted by Taylor Simpson 3 years, 7 months ago

> -----Original Message-----
> From: Philippe Mathieu-Daudé <philippe.mathieu.daude@gmail.com> On
> Behalf Of Philippe Mathieu-Daudé
> Sent: Tuesday, September 29, 2020 6:22 AM
> To: Taylor Simpson <tsimpson@quicinc.com>; qemu-devel@nongnu.org
> Cc: ale@rev.ng; riku.voipio@iki.fi; richard.henderson@linaro.org;
> laurent@vivier.eu; aleksandar.m.mail@gmail.com
> Subject: Re: [RFC PATCH v4 00/29] Hexagon patch series
>
> cc1: all warnings being treated as errors
> make: *** [Makefile.ninja:638:
> libqemu-hexagon-linux-user.fa.p/target_hexagon_decode.c.o] Error 1
>
> $ gcc --version
> gcc (GCC) 10.2.1 20200723 (Red Hat 10.2.1-1)
>

Thanks for all your feedback.  I really appreciate it and will make the changes you mentioned.

I'm using an older GCC that doesn't have these errors.  Is this the version of GCC that is recommended (mandated?) for building qemu?


PS  You were right about Richard recommending const.  It's already on my TODO list from his review 😉

Thanks,
Taylor

Re: [RFC PATCH v4 00/29] Hexagon patch series
Posted by Philippe Mathieu-Daudé 3 years, 7 months ago
On 9/29/20 5:53 PM, Taylor Simpson wrote:
>> -----Original Message-----
>> From: Philippe Mathieu-Daudé <philippe.mathieu.daude@gmail.com> On
>> Behalf Of Philippe Mathieu-Daudé
>> Sent: Tuesday, September 29, 2020 6:22 AM
>> To: Taylor Simpson <tsimpson@quicinc.com>; qemu-devel@nongnu.org
>> Cc: ale@rev.ng; riku.voipio@iki.fi; richard.henderson@linaro.org;
>> laurent@vivier.eu; aleksandar.m.mail@gmail.com
>> Subject: Re: [RFC PATCH v4 00/29] Hexagon patch series
>>
>> cc1: all warnings being treated as errors
>> make: *** [Makefile.ninja:638:
>> libqemu-hexagon-linux-user.fa.p/target_hexagon_decode.c.o] Error 1
>>
>> $ gcc --version
>> gcc (GCC) 10.2.1 20200723 (Red Hat 10.2.1-1)
>>
> 
> Thanks for all your feedback.  I really appreciate it and will make the changes you mentioned.

No problem, I also appreciate the effort you did to address all
of the previous issues :)

> 
> I'm using an older GCC that doesn't have these errors.  Is this the version of GCC that is recommended (mandated?) for building qemu?

QEMU aims to support the 2 latest releases of supported distributions.
From time to time a brave developer look at the different versions
packaged and make some cleanup in the code base. It used to be tedious,
now that repology.org exists it is a bit easier.

The last effort is from Thomas, see commit efc6c070aca:

    The supported distributions use the following version
    of GCC:

          RHEL-7: 4.8.5
          Debian (Stretch): 6.3.0
          Debian (Jessie): 4.8.4
          OpenBSD (ports): 4.9.4
          FreeBSD (ports): 8.2.0
          OpenSUSE Leap 15: 7.3.1
          Ubuntu (Xenial): 5.3.1
          macOS (Homebrew): 8.2.0

    So we can safely assume GCC 4.8 these days.

This is the "mandated" compiler version.


QEMU has some CI jobs, see:
https://wiki.qemu.org/Testing/CI

You can use most of them by opening GitLab and Travis/Cirrus
(for GitHub, which you already use).

GitLab will become our "gating CI" soon, so your series is
expected to pass all the GitLab jobs. IIRC running the tests
is as easy as register and push your branch to your account.

> 
> PS  You were right about Richard recommending const.  It's already on my TODO list from his review 😉
> 

=)

Regards,

Phil.

> Thanks,
> Taylor
> 

RE: [RFC PATCH v4 00/29] Hexagon patch series
Posted by Taylor Simpson 3 years, 7 months ago

> -----Original Message-----
> From: Philippe Mathieu-Daudé <philippe.mathieu.daude@gmail.com> On
> Behalf Of Philippe Mathieu-Daudé
> Sent: Tuesday, September 29, 2020 11:02 AM
> To: Taylor Simpson <tsimpson@quicinc.com>; qemu-devel@nongnu.org
> Cc: ale@rev.ng; riku.voipio@iki.fi; richard.henderson@linaro.org;
> laurent@vivier.eu; aleksandar.m.mail@gmail.com
> Subject: Re: [RFC PATCH v4 00/29] Hexagon patch series
>
> QEMU aims to support the 2 latest releases of supported distributions.
> From time to time a brave developer look at the different versions
> packaged and make some cleanup in the code base. It used to be tedious,
> now that repology.org exists it is a bit easier.
>
> The last effort is from Thomas, see commit efc6c070aca:
>
>     The supported distributions use the following version
>     of GCC:
>
>           RHEL-7: 4.8.5
>           Debian (Stretch): 6.3.0
>           Debian (Jessie): 4.8.4
>           OpenBSD (ports): 4.9.4
>           FreeBSD (ports): 8.2.0
>           OpenSUSE Leap 15: 7.3.1
>           Ubuntu (Xenial): 5.3.1
>           macOS (Homebrew): 8.2.0
>
>     So we can safely assume GCC 4.8 these days.
>
> This is the "mandated" compiler version.

Ouch!  4.8 is old enough that it doesn't support C11 _Generic which I am using.  That needs at least GCC 4.9.

Here are a couple of examples.  As you can see, _Generic is used to dispatch to slightly different TCG generation functions depending on the type of the operands.  I will scratch my head and figure out a different way to do this.

#define MEM_STORE1_FUNC(X) \
    _Generic((X), int : gen_store1i, TCGv_i32 : gen_store1)
#define MEM_STORE1(VA, DATA, SLOT) \
    MEM_STORE1_FUNC(DATA)(cpu_env, VA, DATA, ctx, SLOT)

#define GETBYTE_FUNC(X) \
    _Generic((X), TCGv_i32 : gen_get_byte, TCGv_i64 : gen_get_byte_i64)
#define fGETBYTE(N, SRC) GETBYTE_FUNC(SRC)(BYTE, N, SRC, true)
#define fGETUBYTE(N, SRC) GETBYTE_FUNC(SRC)(BYTE, N, SRC, false)


FWIW, I have been using 5.5.

The errors you saw started around 7.5 and are easy to fix.


Taylor

Re: [RFC PATCH v4 00/29] Hexagon patch series
Posted by Philippe Mathieu-Daudé 3 years, 7 months ago
Cc'ing Eric/Thomas...

On 9/29/20 10:11 PM, Taylor Simpson wrote:
>> -----Original Message-----
>> From: Philippe Mathieu-Daudé <philippe.mathieu.daude@gmail.com> On
>> Behalf Of Philippe Mathieu-Daudé
>> Sent: Tuesday, September 29, 2020 11:02 AM
>> To: Taylor Simpson <tsimpson@quicinc.com>; qemu-devel@nongnu.org
>> Cc: ale@rev.ng; riku.voipio@iki.fi; richard.henderson@linaro.org;
>> laurent@vivier.eu; aleksandar.m.mail@gmail.com
>> Subject: Re: [RFC PATCH v4 00/29] Hexagon patch series
>>
>> QEMU aims to support the 2 latest releases of supported distributions.
>> From time to time a brave developer look at the different versions
>> packaged and make some cleanup in the code base. It used to be tedious,
>> now that repology.org exists it is a bit easier.
>>
>> The last effort is from Thomas, see commit efc6c070aca:
>>
>>     The supported distributions use the following version
>>     of GCC:
>>
>>           RHEL-7: 4.8.5
>>           Debian (Stretch): 6.3.0
>>           Debian (Jessie): 4.8.4
>>           OpenBSD (ports): 4.9.4
>>           FreeBSD (ports): 8.2.0
>>           OpenSUSE Leap 15: 7.3.1
>>           Ubuntu (Xenial): 5.3.1
>>           macOS (Homebrew): 8.2.0
>>
>>     So we can safely assume GCC 4.8 these days.
>>
>> This is the "mandated" compiler version.
> 
> Ouch!  4.8 is old enough that it doesn't support C11 _Generic which I am using.  That needs at least GCC 4.9.
> 
> Here are a couple of examples.  As you can see, _Generic is used to dispatch to slightly different TCG generation functions depending on the type of the operands.  I will scratch my head and figure out a different way to do this.
> 
> #define MEM_STORE1_FUNC(X) \
>     _Generic((X), int : gen_store1i, TCGv_i32 : gen_store1)
> #define MEM_STORE1(VA, DATA, SLOT) \
>     MEM_STORE1_FUNC(DATA)(cpu_env, VA, DATA, ctx, SLOT)
> 
> #define GETBYTE_FUNC(X) \
>     _Generic((X), TCGv_i32 : gen_get_byte, TCGv_i64 : gen_get_byte_i64)
> #define fGETBYTE(N, SRC) GETBYTE_FUNC(SRC)(BYTE, N, SRC, true)
> #define fGETUBYTE(N, SRC) GETBYTE_FUNC(SRC)(BYTE, N, SRC, false)
> 
> 
> FWIW, I have been using 5.5.
> 
> The errors you saw started around 7.5 and are easy to fix.
> 
> 
> Taylor
> 

Re: [RFC PATCH v4 00/29] Hexagon patch series
Posted by Eric Blake 3 years, 7 months ago
On 9/29/20 3:11 PM, Taylor Simpson wrote:

> Ouch!  4.8 is old enough that it doesn't support C11 _Generic which I am using.  That needs at least GCC 4.9.
> 
> Here are a couple of examples.  As you can see, _Generic is used to dispatch to slightly different TCG generation functions depending on the type of the operands.  I will scratch my head and figure out a different way to do this.
> 
> #define MEM_STORE1_FUNC(X) \
>     _Generic((X), int : gen_store1i, TCGv_i32 : gen_store1)
> #define MEM_STORE1(VA, DATA, SLOT) \
>     MEM_STORE1_FUNC(DATA)(cpu_env, VA, DATA, ctx, SLOT)

See if you can use __builtin_choose_expr() instead.  Look at
include/osdep/atomic.h which defines typeof_strip_qual() without
_Generic.  linux-user/qemu.h __put_user_e() is also an example of what
appears to be a poor-man's replacement to _Generic.

-- 
Eric Blake, Principal Software Engineer
Red Hat, Inc.           +1-919-301-3226
Virtualization:  qemu.org | libvirt.org

RE: [RFC PATCH v4 00/29] Hexagon patch series
Posted by Taylor Simpson 3 years, 7 months ago
> -----Original Message-----
> From: Eric Blake <eblake@redhat.com>
> Sent: Tuesday, September 29, 2020 3:29 PM
> To: Taylor Simpson <tsimpson@quicinc.com>; Philippe Mathieu-Daudé
> <f4bug@amsat.org>; qemu-devel@nongnu.org
> Cc: ale@rev.ng; riku.voipio@iki.fi; richard.henderson@linaro.org;
> laurent@vivier.eu; aleksandar.m.mail@gmail.com
> Subject: Re: [RFC PATCH v4 00/29] Hexagon patch series
>
> On 9/29/20 3:11 PM, Taylor Simpson wrote:
>
> > Ouch!  4.8 is old enough that it doesn't support C11 _Generic which I am
> using.  That needs at least GCC 4.9.
> >
> > Here are a couple of examples.  As you can see, _Generic is used to
> dispatch to slightly different TCG generation functions depending on the
> type of the operands.  I will scratch my head and figure out a different way to
> do this.
> >
> > #define MEM_STORE1_FUNC(X) \
> >     _Generic((X), int : gen_store1i, TCGv_i32 : gen_store1)
> > #define MEM_STORE1(VA, DATA, SLOT) \
> >     MEM_STORE1_FUNC(DATA)(cpu_env, VA, DATA, ctx, SLOT)
>
> See if you can use __builtin_choose_expr() instead.  Look at
> include/osdep/atomic.h which defines typeof_strip_qual() without
> _Generic.  linux-user/qemu.h __put_user_e() is also an example of what
> appears to be a poor-man's replacement to _Generic.

Thanks!  It's a pretty straightforward translation for my use cases

#define MEM_STORE1_FUNC(X) \
    __builtin_choose_expr(__builtin_types_compatible_p(typeof(X), int), \
        gen_store1i, \
        gen_store1)
#define MEM_STORE1(VA, DATA, SLOT) \
    MEM_STORE1_FUNC(DATA)(cpu_env, VA, DATA, ctx, SLOT)


Taylor
Re: [RFC PATCH v4 00/29] Hexagon patch series
Posted by Brad Smith 3 years, 7 months ago
On 9/29/2020 1:01 PM, Philippe Mathieu-Daudé wrote:
> On 9/29/20 5:53 PM, Taylor Simpson wrote:
>>> -----Original Message-----
>>> From: Philippe Mathieu-Daudé <philippe.mathieu.daude@gmail.com> On
>>> Behalf Of Philippe Mathieu-Daudé
>>> Sent: Tuesday, September 29, 2020 6:22 AM
>>> To: Taylor Simpson <tsimpson@quicinc.com>; qemu-devel@nongnu.org
>>> Cc: ale@rev.ng; riku.voipio@iki.fi; richard.henderson@linaro.org;
>>> laurent@vivier.eu; aleksandar.m.mail@gmail.com
>>> Subject: Re: [RFC PATCH v4 00/29] Hexagon patch series
>>>
>>> cc1: all warnings being treated as errors
>>> make: *** [Makefile.ninja:638:
>>> libqemu-hexagon-linux-user.fa.p/target_hexagon_decode.c.o] Error 1
>>>
>>> $ gcc --version
>>> gcc (GCC) 10.2.1 20200723 (Red Hat 10.2.1-1)
>>>
>> Thanks for all your feedback.  I really appreciate it and will make the changes you mentioned.
> No problem, I also appreciate the effort you did to address all
> of the previous issues :)
>
>> I'm using an older GCC that doesn't have these errors.  Is this the version of GCC that is recommended (mandated?) for building qemu?
> QEMU aims to support the 2 latest releases of supported distributions.
>  From time to time a brave developer look at the different versions
> packaged and make some cleanup in the code base. It used to be tedious,
> now that repology.org exists it is a bit easier.
>
> The last effort is from Thomas, see commit efc6c070aca:
>
>      The supported distributions use the following version
>      of GCC:
>
>            RHEL-7: 4.8.5
>            Debian (Stretch): 6.3.0
>            Debian (Jessie): 4.8.4
>            OpenBSD (ports): 4.9.4

OpenBSD as of 6.6 uses GCC 8.3.0 for our ports-gcc.

>            FreeBSD (ports): 8.2.0
>            OpenSUSE Leap 15: 7.3.1
>            Ubuntu (Xenial): 5.3.1
>            macOS (Homebrew): 8.2.0
>
>      So we can safely assume GCC 4.8 these days.
FreeBSD and OpenBSD nowadays use Clang for the (system) compiler.
>
> This is the "mandated" compiler version.
>
>
> QEMU has some CI jobs, see:
> https://wiki.qemu.org/Testing/CI
>
> You can use most of them by opening GitLab and Travis/Cirrus
> (for GitHub, which you already use).
>
> GitLab will become our "gating CI" soon, so your series is
> expected to pass all the GitLab jobs. IIRC running the tests
> is as easy as register and push your branch to your account.
>
>> PS  You were right about Richard recommending const.  It's already on my TODO list from his review 😉
>>
> =)
>
> Regards,
>
> Phil.
>
>> Thanks,
>> Taylor
>>