mbox

[PULL,00/21] Hexagon update: bug fixes, performance, idef-parser

Message ID 20221216204845.19290-1-tsimpson@quicinc.com (mailing list archive)
State New, archived
Headers show

Pull-request

https://github.com/quic/qemu tags/pull-hex-20221216-1

Message

Taylor Simpson Dec. 16, 2022, 8:48 p.m. UTC
The following changes since commit 4208e6ae114ac8266dcacc9696a443ce5c37b04e:

  Merge tag 'pull-request-2022-12-15' of https://gitlab.com/thuth/qemu into staging (2022-12-15 21:39:56 +0000)

are available in the Git repository at:

  https://github.com/quic/qemu tags/pull-hex-20221216-1

for you to fetch changes up to 585a86b1041a45c3b4074440c7f1b54944570867:

  target/hexagon: import additional tests (2022-12-16 12:30:28 -0800)

----------------------------------------------------------------

******************************************************************
*****                      WARNING                           *****
***** This PR requires updated CI containers with flex/bison *****
******************************************************************

1)
Performance improvement
Add pkt and insn to DisasContext
Many functions need information from all 3 structures, so merge
them together.

2)
Bug fix
Fix predicated assignment to .tmp and .cur

3)
Performance improvement
Add overrides for S2_asr_r_r_sat/S2_asl_r_r_sat
These functions will not be handled by idef-parser

4-11)
The final 8 patches improve change-of-flow handling.

Currently, we set the PC to a new address before exiting a TB.  The
ultimate goal is to use direct block chaining.  However, several steps
are needed along the way.

4)
When a packet has more than one change-of-flow (COF) instruction, only
the first one taken is considered.  The runtime bookkeeping is only
needed when there is more than one COF instruction in a packet.

5, 6)
Remove PC and next_PC from the runtime state and always use a
translation-time constant.  Note that next_PC is used by call instructions
to set LR and by conditional COF instructions to set the fall-through
address.

7, 8, 9)
Add helper overrides for COF instructions.  In particular, we must
distinguish those that use a PC-relative address for the destination.
These are candidates for direct block chaining later.

10)
Use direct block chaining for packets that have a single PC-relative
COF instruction.  Instead of generating the code while processing the
instruction, we record the effect in DisasContext and generate the code
during gen_end_tb.

11)
Use direct block chaining for tight loops.  We look for TBs that end
with an endloop0 that will branch back to the TB start address.

12-21)
Instruction definition parser (idef-parser) from rev.ng
Parses the instruction semantics and generates TCG

----------------------------------------------------------------
Alessandro Di Federico (4):
      target/hexagon: update MAINTAINERS for idef-parser
      target/hexagon: import README for idef-parser
      target/hexagon: prepare input for the idef-parser
      target/hexagon: call idef-parser functions

Anton Johansson (1):
      target/hexagon: import parser for idef-parser

Niccolò Izzo (2):
      target/hexagon: introduce new helper functions
      target/hexagon: import additional tests

Paolo Montesel (3):
      target/hexagon: make slot number an unsigned
      target/hexagon: make helper functions non-static
      target/hexagon: import lexer for idef-parser

Taylor Simpson (11):
      Hexagon (target/hexagon) Add pkt and insn to DisasContext
      Hexagon (target/hexagon) Fix predicated assignment to .tmp and .cur
      Hexagon (target/hexagon) Add overrides for S2_asr_r_r_sat/S2_asl_r_r_sat
      Hexagon (target/hexagon) Only use branch_taken when packet has multi cof
      Hexagon (target/hexagon) Remove PC from the runtime state
      Hexagon (target/hexagon) Remove next_PC from runtime state
      Hexagon (target/hexagon) Add overrides for direct call instructions
      Hexagon (target/hexagon) Add overrides for compound compare and jump
      Hexagon (target/hexagon) Add overrides for various forms of jump
      Hexagon (target/hexagon) Use direct block chaining for direct jump/branch
      Hexagon (target/hexagon) Use direct block chaining for tight loops

 target/hexagon/idef-parser/README.rst       |  722 ++++++++
 target/hexagon/cpu.h                        |   14 +-
 target/hexagon/gen_tcg.h                    |  412 ++++-
 target/hexagon/gen_tcg_hvx.h                |    6 +-
 target/hexagon/genptr.h                     |   36 +
 target/hexagon/idef-parser/idef-parser.h    |  253 +++
 target/hexagon/idef-parser/parser-helpers.h |  376 +++++
 target/hexagon/insn.h                       |    9 +-
 target/hexagon/macros.h                     |   27 +-
 target/hexagon/mmvec/macros.h               |    4 +-
 target/hexagon/op_helper.h                  |   37 +
 target/hexagon/translate.h                  |   20 +-
 target/hexagon/decode.c                     |   15 +-
 target/hexagon/genptr.c                     |  585 ++++++-
 target/hexagon/idef-parser/parser-helpers.c | 2360 +++++++++++++++++++++++++++
 target/hexagon/op_helper.c                  |   55 +-
 target/hexagon/translate.c                  |  229 ++-
 tests/tcg/hexagon/hvx_misc.c                |   72 +
 tests/tcg/hexagon/usr.c                     |   34 +-
 MAINTAINERS                                 |    9 +
 meson_options.txt                           |    3 +
 target/hexagon/README                       |    5 +
 target/hexagon/gen_helper_funcs.py          |   30 +-
 target/hexagon/gen_helper_protos.py         |   31 +-
 target/hexagon/gen_idef_parser_funcs.py     |  130 ++
 target/hexagon/gen_tcg_funcs.py             |   79 +-
 target/hexagon/hex_common.py                |   40 +-
 target/hexagon/idef-parser/idef-parser.lex  |  471 ++++++
 target/hexagon/idef-parser/idef-parser.y    |  965 +++++++++++
 target/hexagon/idef-parser/macros.inc       |  140 ++
 target/hexagon/idef-parser/prepare          |   24 +
 target/hexagon/meson.build                  |  158 +-
 tests/tcg/hexagon/Makefile.target           |   28 +-
 tests/tcg/hexagon/crt.S                     |   14 +
 tests/tcg/hexagon/test_abs.S                |   17 +
 tests/tcg/hexagon/test_bitcnt.S             |   40 +
 tests/tcg/hexagon/test_bitsplit.S           |   22 +
 tests/tcg/hexagon/test_call.S               |   64 +
 tests/tcg/hexagon/test_clobber.S            |   29 +
 tests/tcg/hexagon/test_cmp.S                |   31 +
 tests/tcg/hexagon/test_dotnew.S             |   38 +
 tests/tcg/hexagon/test_ext.S                |   13 +
 tests/tcg/hexagon/test_fibonacci.S          |   30 +
 tests/tcg/hexagon/test_hl.S                 |   16 +
 tests/tcg/hexagon/test_hwloops.S            |   19 +
 tests/tcg/hexagon/test_jmp.S                |   22 +
 tests/tcg/hexagon/test_lsr.S                |   36 +
 tests/tcg/hexagon/test_mpyi.S               |   17 +
 tests/tcg/hexagon/test_packet.S             |   29 +
 tests/tcg/hexagon/test_reorder.S            |   33 +
 tests/tcg/hexagon/test_round.S              |   29 +
 tests/tcg/hexagon/test_vavgw.S              |   31 +
 tests/tcg/hexagon/test_vcmpb.S              |   30 +
 tests/tcg/hexagon/test_vcmpw.S              |   30 +
 tests/tcg/hexagon/test_vlsrw.S              |   20 +
 tests/tcg/hexagon/test_vmaxh.S              |   35 +
 tests/tcg/hexagon/test_vminh.S              |   35 +
 tests/tcg/hexagon/test_vpmpyh.S             |   28 +
 tests/tcg/hexagon/test_vspliceb.S           |   31 +
 59 files changed, 7908 insertions(+), 210 deletions(-)
 create mode 100644 target/hexagon/idef-parser/README.rst
 create mode 100644 target/hexagon/idef-parser/idef-parser.h
 create mode 100644 target/hexagon/idef-parser/parser-helpers.h
 create mode 100644 target/hexagon/op_helper.h
 create mode 100644 target/hexagon/idef-parser/parser-helpers.c
 create mode 100644 target/hexagon/gen_idef_parser_funcs.py
 create mode 100644 target/hexagon/idef-parser/idef-parser.lex
 create mode 100644 target/hexagon/idef-parser/idef-parser.y
 create mode 100644 target/hexagon/idef-parser/macros.inc
 create mode 100755 target/hexagon/idef-parser/prepare
 create mode 100644 tests/tcg/hexagon/crt.S
 create mode 100644 tests/tcg/hexagon/test_abs.S
 create mode 100644 tests/tcg/hexagon/test_bitcnt.S
 create mode 100644 tests/tcg/hexagon/test_bitsplit.S
 create mode 100644 tests/tcg/hexagon/test_call.S
 create mode 100644 tests/tcg/hexagon/test_clobber.S
 create mode 100644 tests/tcg/hexagon/test_cmp.S
 create mode 100644 tests/tcg/hexagon/test_dotnew.S
 create mode 100644 tests/tcg/hexagon/test_ext.S
 create mode 100644 tests/tcg/hexagon/test_fibonacci.S
 create mode 100644 tests/tcg/hexagon/test_hl.S
 create mode 100644 tests/tcg/hexagon/test_hwloops.S
 create mode 100644 tests/tcg/hexagon/test_jmp.S
 create mode 100644 tests/tcg/hexagon/test_lsr.S
 create mode 100644 tests/tcg/hexagon/test_mpyi.S
 create mode 100644 tests/tcg/hexagon/test_packet.S
 create mode 100644 tests/tcg/hexagon/test_reorder.S
 create mode 100644 tests/tcg/hexagon/test_round.S
 create mode 100644 tests/tcg/hexagon/test_vavgw.S
 create mode 100644 tests/tcg/hexagon/test_vcmpb.S
 create mode 100644 tests/tcg/hexagon/test_vcmpw.S
 create mode 100644 tests/tcg/hexagon/test_vlsrw.S
 create mode 100644 tests/tcg/hexagon/test_vmaxh.S
 create mode 100644 tests/tcg/hexagon/test_vminh.S
 create mode 100644 tests/tcg/hexagon/test_vpmpyh.S
 create mode 100644 tests/tcg/hexagon/test_vspliceb.S

Comments

Peter Maydell Dec. 17, 2022, 9:21 p.m. UTC | #1
On Fri, 16 Dec 2022 at 20:49, Taylor Simpson <tsimpson@quicinc.com> wrote:
>
> The following changes since commit 4208e6ae114ac8266dcacc9696a443ce5c37b04e:
>
>   Merge tag 'pull-request-2022-12-15' of https://gitlab.com/thuth/qemu into staging (2022-12-15 21:39:56 +0000)
>
> are available in the Git repository at:
>
>   https://github.com/quic/qemu tags/pull-hex-20221216-1
>
> for you to fetch changes up to 585a86b1041a45c3b4074440c7f1b54944570867:
>
>   target/hexagon: import additional tests (2022-12-16 12:30:28 -0800)
>
> ----------------------------------------------------------------
>
> ******************************************************************
> *****                      WARNING                           *****
> ***** This PR requires updated CI containers with flex/bison *****
> ******************************************************************

Does that mean somebody needs to do something before I can merge this?

thanks
-- PMM
Gao,Shiyuan" via Dec. 18, 2022, 9:33 a.m. UTC | #2
On 12/17/22 22:21, Peter Maydell wrote:

> Does that mean somebody needs to do something before I can merge this?
>
> thanks
> -- PMM

Hi Peter,

no one needs to do anything about the CI, it has already been taken care of.

All containers that need flex/bison have it, and the 
debian-hexagon-cross container
has already been rebuilt manually and pushed by Alex.

cheers,
Peter Maydell Dec. 18, 2022, 1:52 p.m. UTC | #3
On Fri, 16 Dec 2022 at 20:49, Taylor Simpson <tsimpson@quicinc.com> wrote:
>
> The following changes since commit 4208e6ae114ac8266dcacc9696a443ce5c37b04e:
>
>   Merge tag 'pull-request-2022-12-15' of https://gitlab.com/thuth/qemu into staging (2022-12-15 21:39:56 +0000)
>
> are available in the Git repository at:
>
>   https://github.com/quic/qemu tags/pull-hex-20221216-1
>
> for you to fetch changes up to 585a86b1041a45c3b4074440c7f1b54944570867:
>
>   target/hexagon: import additional tests (2022-12-16 12:30:28 -0800)


Failed to build, s390x:

https://gitlab.com/qemu-project/qemu/-/jobs/3492490152

Program scripts/decodetree.py found: YES (/usr/bin/python3
/home/gitlab-runner/builds/-LCfcJ2T/0/qemu-project/qemu/scripts/decodetree.py)
Program flex found: NO
../target/hexagon/meson.build:179:4: ERROR: Program 'flex' not found
or not executable

Can you get the CI requirements sorted out first, and then
let me know when I can try re-merging this, please?

thanks
-- PMM
Gao,Shiyuan" via Dec. 18, 2022, 3:34 p.m. UTC | #4
On 12/18/22 14:52, Peter Maydell wrote:

> Failed to build, s390x:
>
> https://gitlab.com/qemu-project/qemu/-/jobs/3492490152
>
> Program scripts/decodetree.py found: YES (/usr/bin/python3
> /home/gitlab-runner/builds/-LCfcJ2T/0/qemu-project/qemu/scripts/decodetree.py)
> Program flex found: NO
> ../target/hexagon/meson.build:179:4: ERROR: Program 'flex' not found
> or not executable
>
> Can you get the CI requirements sorted out first, and then
> let me know when I can try re-merging this, please?
>
> thanks
> -- PMM
Oh, this is my bad. I didn't know we built Hexagon on s390x, I naively 
assumed the CI jobs ran locally matched those upstream.

I'll send a patch adding flex/bison to s390x then.

Thanks,
Richard Henderson Dec. 18, 2022, 4:53 p.m. UTC | #5
On 12/18/22 05:52, Peter Maydell wrote:
> On Fri, 16 Dec 2022 at 20:49, Taylor Simpson <tsimpson@quicinc.com> wrote:
>>
>> The following changes since commit 4208e6ae114ac8266dcacc9696a443ce5c37b04e:
>>
>>    Merge tag 'pull-request-2022-12-15' of https://gitlab.com/thuth/qemu into staging (2022-12-15 21:39:56 +0000)
>>
>> are available in the Git repository at:
>>
>>    https://github.com/quic/qemu tags/pull-hex-20221216-1
>>
>> for you to fetch changes up to 585a86b1041a45c3b4074440c7f1b54944570867:
>>
>>    target/hexagon: import additional tests (2022-12-16 12:30:28 -0800)
> 
> 
> Failed to build, s390x:
> 
> https://gitlab.com/qemu-project/qemu/-/jobs/3492490152
> 
> Program scripts/decodetree.py found: YES (/usr/bin/python3
> /home/gitlab-runner/builds/-LCfcJ2T/0/qemu-project/qemu/scripts/decodetree.py)
> Program flex found: NO
> ../target/hexagon/meson.build:179:4: ERROR: Program 'flex' not found
> or not executable
> 
> Can you get the CI requirements sorted out first, and then
> let me know when I can try re-merging this, please?

Our s390x host won't be affected by any of the scripts,
it simply needs to be installed.

Alex, can you please install flex + bison on s390x.ci.qemu.org?


r~
Peter Maydell Dec. 18, 2022, 5:01 p.m. UTC | #6
On Sun, 18 Dec 2022 at 16:53, Richard Henderson
<richard.henderson@linaro.org> wrote:
>
> On 12/18/22 05:52, Peter Maydell wrote:
> > On Fri, 16 Dec 2022 at 20:49, Taylor Simpson <tsimpson@quicinc.com> wrote:
> >>
> >> The following changes since commit 4208e6ae114ac8266dcacc9696a443ce5c37b04e:
> >>
> >>    Merge tag 'pull-request-2022-12-15' of https://gitlab.com/thuth/qemu into staging (2022-12-15 21:39:56 +0000)
> >>
> >> are available in the Git repository at:
> >>
> >>    https://github.com/quic/qemu tags/pull-hex-20221216-1
> >>
> >> for you to fetch changes up to 585a86b1041a45c3b4074440c7f1b54944570867:
> >>
> >>    target/hexagon: import additional tests (2022-12-16 12:30:28 -0800)
> >
> >
> > Failed to build, s390x:
> >
> > https://gitlab.com/qemu-project/qemu/-/jobs/3492490152
> >
> > Program scripts/decodetree.py found: YES (/usr/bin/python3
> > /home/gitlab-runner/builds/-LCfcJ2T/0/qemu-project/qemu/scripts/decodetree.py)
> > Program flex found: NO
> > ../target/hexagon/meson.build:179:4: ERROR: Program 'flex' not found
> > or not executable
> >
> > Can you get the CI requirements sorted out first, and then
> > let me know when I can try re-merging this, please?
>
> Our s390x host won't be affected by any of the scripts,
> it simply needs to be installed.
>
> Alex, can you please install flex + bison on s390x.ci.qemu.org?

Ah, if that's all we need to do, I have access for that. I'll
install the packages and retry.

thanks
-- PMM
Peter Maydell Dec. 19, 2022, 10:28 a.m. UTC | #7
On Fri, 16 Dec 2022 at 20:49, Taylor Simpson <tsimpson@quicinc.com> wrote:
>
> The following changes since commit 4208e6ae114ac8266dcacc9696a443ce5c37b04e:
>
>   Merge tag 'pull-request-2022-12-15' of https://gitlab.com/thuth/qemu into staging (2022-12-15 21:39:56 +0000)
>
> are available in the Git repository at:
>
>   https://github.com/quic/qemu tags/pull-hex-20221216-1
>
> for you to fetch changes up to 585a86b1041a45c3b4074440c7f1b54944570867:
>
>   target/hexagon: import additional tests (2022-12-16 12:30:28 -0800)


Applied, thanks.

Please update the changelog at https://wiki.qemu.org/ChangeLog/8.0
for any user-visible changes. (In particular there is a section
of the changelog for 'Build Dependencies' -- please add something
there indicating under what circumstances flex/bison are now required.)

-- PMM
Philippe Mathieu-Daudé Dec. 19, 2022, 11:18 a.m. UTC | #8
On 18/12/22 18:01, Peter Maydell wrote:
> On Sun, 18 Dec 2022 at 16:53, Richard Henderson
> <richard.henderson@linaro.org> wrote:
>>
>> On 12/18/22 05:52, Peter Maydell wrote:
>>> On Fri, 16 Dec 2022 at 20:49, Taylor Simpson <tsimpson@quicinc.com> wrote:
>>>>
>>>> The following changes since commit 4208e6ae114ac8266dcacc9696a443ce5c37b04e:
>>>>
>>>>     Merge tag 'pull-request-2022-12-15' of https://gitlab.com/thuth/qemu into staging (2022-12-15 21:39:56 +0000)
>>>>
>>>> are available in the Git repository at:
>>>>
>>>>     https://github.com/quic/qemu tags/pull-hex-20221216-1
>>>>
>>>> for you to fetch changes up to 585a86b1041a45c3b4074440c7f1b54944570867:
>>>>
>>>>     target/hexagon: import additional tests (2022-12-16 12:30:28 -0800)
>>>
>>>
>>> Failed to build, s390x:
>>>
>>> https://gitlab.com/qemu-project/qemu/-/jobs/3492490152
>>>
>>> Program scripts/decodetree.py found: YES (/usr/bin/python3
>>> /home/gitlab-runner/builds/-LCfcJ2T/0/qemu-project/qemu/scripts/decodetree.py)
>>> Program flex found: NO
>>> ../target/hexagon/meson.build:179:4: ERROR: Program 'flex' not found
>>> or not executable
>>>
>>> Can you get the CI requirements sorted out first, and then
>>> let me know when I can try re-merging this, please?
>>
>> Our s390x host won't be affected by any of the scripts,
>> it simply needs to be installed.

scripts/ci/setup/build-environment.yml need to be updated although.

>> Alex, can you please install flex + bison on s390x.ci.qemu.org?
> 
> Ah, if that's all we need to do, I have access for that. I'll
> install the packages and retry.
> 
> thanks
> -- PMM
Taylor Simpson Dec. 19, 2022, 6:54 p.m. UTC | #9
> -----Original Message-----
> From: Peter Maydell <peter.maydell@linaro.org>
> Sent: Monday, December 19, 2022 4:28 AM
> To: Taylor Simpson <tsimpson@quicinc.com>
> Cc: qemu-devel@nongnu.org; richard.henderson@linaro.org;
> philmd@linaro.org; Brian Cain <bcain@quicinc.com>; Matheus Bernardino
> (QUIC) <quic_mathbern@quicinc.com>; stefanha@redhat.com
> Subject: Re: [PULL 00/21] Hexagon update: bug fixes, performance, idef-
> parser
> 
> On Fri, 16 Dec 2022 at 20:49, Taylor Simpson <tsimpson@quicinc.com> wrote:
> >
> > The following changes since commit
> 4208e6ae114ac8266dcacc9696a443ce5c37b04e:
> >
> >   Merge tag 'pull-request-2022-12-15' of https://gitlab.com/thuth/qemu
> > into staging (2022-12-15 21:39:56 +0000)
> >
> > are available in the Git repository at:
> >
> >   https://github.com/quic/qemu tags/pull-hex-20221216-1
> >
> > for you to fetch changes up to
> 585a86b1041a45c3b4074440c7f1b54944570867:
> >
> >   target/hexagon: import additional tests (2022-12-16 12:30:28 -0800)
> 
> 
> Applied, thanks.

Thanks!!

> 
> Please update the changelog at https://wiki.qemu.org/ChangeLog/8.0
> for any user-visible changes. (In particular there is a section of the changelog
> for 'Build Dependencies' -- please add something there indicating under what
> circumstances flex/bison are now required.)

I've forgotten my password.  How do I reset it?

Taylor
Philippe Mathieu-Daudé Dec. 19, 2022, 11:19 p.m. UTC | #10
Hi,

On 16/12/22 21:48, Taylor Simpson wrote:

> ----------------------------------------------------------------

> 12-21)
> Instruction definition parser (idef-parser) from rev.ng
> Parses the instruction semantics and generates TCG

Building QEMU with Clang I'm now getting:

target/hexagon/idef-parser.p/idef-parser.tab.c:2197:9: error: variable 
'yynerrs' set but not used [-Werror,-Wunused-but-set-variable]
     int yynerrs = 0;
         ^

Regards,

Phil.
Philippe Mathieu-Daudé Dec. 20, 2022, 7:30 a.m. UTC | #11
On 20/12/22 00:19, Philippe Mathieu-Daudé wrote:
> Hi,
> 
> On 16/12/22 21:48, Taylor Simpson wrote:
> 
>> ----------------------------------------------------------------
> 
>> 12-21)
>> Instruction definition parser (idef-parser) from rev.ng
>> Parses the instruction semantics and generates TCG
> 
> Building QEMU with Clang I'm now getting:
> 
> target/hexagon/idef-parser.p/idef-parser.tab.c:2197:9: error: variable 
> 'yynerrs' set but not used [-Werror,-Wunused-but-set-variable]
>      int yynerrs = 0;
>          ^

idef-parser.tab.c is built using:

193     idef_parser = executable(
194         'idef-parser',
195         [flex.process(idef_parser_dir / 'idef-parser.lex'),
196          bison.process(idef_parser_dir / 'idef-parser.y'),
197          idef_parser_dir / 'parser-helpers.c'],
198         include_directories: ['idef-parser', '../../include/'],
199         dependencies: [glib_dep],
200         c_args: ['-Wextra'],
                       ^^^^^^^^
201         native: true
202     )

(see commit c0a41ee631 "target/hexagon: import parser for idef-parser")

Do we really need this level? IIUC the problem with -Wextra is using a
newer compiler toolchain it can include warnings we haven't fixed. Maybe
worthwhile but it can break from times to times.

Using '-Wextra -Wno-unused-but-set-variable' seems a hack. I guess I'd
simply remove -Wextra for simplicity, since no much value is added here.

Regards,

Phil.
Alessandro Di Federico Dec. 20, 2022, 12:49 p.m. UTC | #12
On Mon, 19 Dec 2022 18:54:22 +0000
Taylor Simpson <tsimpson@quicinc.com> wrote:

> > Applied, thanks.  
> 
> Thanks!!

Thanks from our side too! :)

We started this project back in 2015, I'm really happy we finally got
it in!
Alessandro Di Federico Dec. 20, 2022, 12:51 p.m. UTC | #13
On Tue, 20 Dec 2022 08:30:02 +0100
Philippe Mathieu-Daudé <philmd@linaro.org> wrote:

> Do we really need this level? IIUC the problem with -Wextra is using a
> newer compiler toolchain it can include warnings we haven't fixed.
> Maybe worthwhile but it can break from times to times.

I think we just wanted to be overly zealous.
Flags typically used by QEMU are fine.

Shall I send a patch to drop -Wextra?
Philippe Mathieu-Daudé Dec. 20, 2022, 2:19 p.m. UTC | #14
On 20/12/22 13:51, Alessandro Di Federico wrote:
> On Tue, 20 Dec 2022 08:30:02 +0100
> Philippe Mathieu-Daudé <philmd@linaro.org> wrote:
> 
>> Do we really need this level? IIUC the problem with -Wextra is using a
>> newer compiler toolchain it can include warnings we haven't fixed.
>> Maybe worthwhile but it can break from times to times.
> 
> I think we just wanted to be overly zealous.
> Flags typically used by QEMU are fine.
> 
> Shall I send a patch to drop -Wextra?

Well I think so, but meanwhile I'm surprised nobody else reported that,
not even the CI.
Taylor Simpson Dec. 20, 2022, 6:13 p.m. UTC | #15
> -----Original Message-----
> From: Philippe Mathieu-Daudé <philmd@linaro.org>
> Sent: Tuesday, December 20, 2022 8:20 AM
> To: Alessandro Di Federico <ale@rev.ng>
> Cc: Taylor Simpson <tsimpson@quicinc.com>; qemu-devel@nongnu.org;
> Paolo Montesel <babush@rev.ng>; Anton Johansson <anjo@rev.ng>;
> richard.henderson@linaro.org; peter.maydell@linaro.org; Brian Cain
> <bcain@quicinc.com>; Matheus Bernardino (QUIC)
> <quic_mathbern@quicinc.com>; stefanha@redhat.com
> Subject: Re: [PULL 00/21] Hexagon update: bug fixes, performance, idef-
> parser
> 
> On 20/12/22 13:51, Alessandro Di Federico wrote:
> > On Tue, 20 Dec 2022 08:30:02 +0100
> > Philippe Mathieu-Daudé <philmd@linaro.org> wrote:
> >
> >> Do we really need this level? IIUC the problem with -Wextra is using
> >> a newer compiler toolchain it can include warnings we haven't fixed.
> >> Maybe worthwhile but it can break from times to times.
> >
> > I think we just wanted to be overly zealous.
> > Flags typically used by QEMU are fine.
> >
> > Shall I send a patch to drop -Wextra?
> 
> Well I think so, but meanwhile I'm surprised nobody else reported that, not
> even the CI.

What version of clang are you using?  I tried 15.0.6, but just dropping the -W extra didn't fix the problem.  This worked for me

diff --git a/target/hexagon/meson.build b/target/hexagon/meson.build
index e8f250fcac..4dbceb7765 100644
--- a/target/hexagon/meson.build
+++ b/target/hexagon/meson.build
@@ -197,7 +197,7 @@ if idef_parser_enabled and 'hexagon-linux-user' in target_dirs
          idef_parser_dir / 'parser-helpers.c'],
         include_directories: ['idef-parser', '../../include/'],
         dependencies: [glib_dep],
-        c_args: ['-Wextra'],
+        c_args: ['-Wno-unused-but-set-variable'],
         native: true
     )

Taylor