1. Patchew
  2. QEMU
  3. View series

[Qemu-devel] [PATCH v2 00/15] target/ppc: remove getVSR()/putVSR() and further tidy-up

Mark Cave-Ayland posted 15 patches 4 years, 10 months ago
Test s390x passed
Test checkpatch failed
Test asan passed
Test docker-mingw@fedora passed
Test docker-clang@ubuntu passed
Test FreeBSD passed
Patches applied successfully (tree, apply log)
git fetch https://github.com/patchew-project/qemu tags/patchew/20190602110903.3431-1-mark.cave-ayland@ilande.co.uk
Maintainers: David Gibson <david@gibson.dropbear.id.au>
There is a newer version of this series
target/ppc/fpu_helper.c             | 841 ++++++++++++++++--------------------
target/ppc/helper.h                 | 320 +++++++-------
target/ppc/int_helper.c             |  26 +-
target/ppc/internal.h               |  12 -
target/ppc/mem_helper.c             |  27 +-
target/ppc/translate/vsx-impl.inc.c | 567 ++++++++++++++++--------
target/ppc/translate/vsx-ops.inc.c  |  70 +--
7 files changed, 954 insertions(+), 909 deletions(-)
[Qemu-devel] [PATCH v2 00/15] target/ppc: remove getVSR()/putVSR() and further tidy-up
Posted by Mark Cave-Ayland 4 years, 10 months ago 
With the conversion of PPC VSX registers to host endian during the 4.0 development
cycle, the VSX helpers getVSR() and putVSR() which were used to convert between big
endian and host endian (and are currently just a no-op) can now be removed. This
eliminates an extra copy for each VSX source register at runtime.

Patches 1-3 do the elimination work on a per-file basis and switch VSX register
accesses to be via pointers rather than on copies managed using getVSR()/putVSR().

After this patches 4-14 change the VSX registers to be passed to helpers via pointers
rather than register number so that the decode of the vector register pointers occurs
at translation time instead of at runtime. This matches how VMX instructions are
currently decoded.

Finally patch 15 performs some related tidy-up around VSX_FMADD which decodes the
a or m form at translation time, allowing a single helper function to be used for
both implementations.

Greg: I've added you as CC since you managed to find a bug in my last series. This
one is much more mechanical, but if you are able to confirm this doesn't introduce
any regressions in your test images then that would be great.

Signed-off-by: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk>

v2:
- Rebase onto master
- Use working copy of VSX destination registers in patches 1-3 to keep current
  semantics where src == dest and exception handling
- Add patches 4 and 6 to split out helper functions still requiring an opcode
  parameter
- Remove opcode parameter from GEN_VSX_HELPER_X3 and GEN_VSX_HELPER_X2 as it
  isn't required for the common case
- Drop VSX_TEST_DC improvement patch since it is no longer applicable with the
  removal of opcode from the above macros
- Rework VSX_MADD improvement patch to use a single helper for both a and m
  forms as suggested by Richard


Mark Cave-Ayland (15):
  target/ppc: remove getVSR()/putVSR() from fpu_helper.c
  target/ppc: remove getVSR()/putVSR() from mem_helper.c
  target/ppc: remove getVSR()/putVSR() from int_helper.c
  target/ppc: introduce separate VSX_CMP macro for xvcmp* instructions
  target/ppc: introduce GEN_VSX_HELPER_X3 macro to fpu_helper.c
  target/ppc: introduce separate generator and helper for xscvqpdp
  target/ppc: introduce GEN_VSX_HELPER_X2 macro to fpu_helper.c
  target/ppc: introduce GEN_VSX_HELPER_X2_AB macro to fpu_helper.c
  target/ppc: introduce GEN_VSX_HELPER_X1 macro to fpu_helper.c
  target/ppc: introduce GEN_VSX_HELPER_R3 macro to fpu_helper.c
  target/ppc: introduce GEN_VSX_HELPER_R2 macro to fpu_helper.c
  target/ppc: introduce GEN_VSX_HELPER_R2_AB macro to fpu_helper.c
  target/ppc: decode target register in VSX_VECTOR_LOAD_STORE_LENGTH at
    translation time
  target/ppc: decode target register in VSX_EXTRACT_INSERT at
    translation time
  target/ppc: improve VSX_FMADD with new GEN_VSX_HELPER_VSX_MADD macro

 target/ppc/fpu_helper.c             | 841 ++++++++++++++++--------------------
 target/ppc/helper.h                 | 320 +++++++-------
 target/ppc/int_helper.c             |  26 +-
 target/ppc/internal.h               |  12 -
 target/ppc/mem_helper.c             |  27 +-
 target/ppc/translate/vsx-impl.inc.c | 567 ++++++++++++++++--------
 target/ppc/translate/vsx-ops.inc.c  |  70 +--
 7 files changed, 954 insertions(+), 909 deletions(-)

-- 
2.11.0
Re: [Qemu-devel] [PATCH v2 00/15] target/ppc: remove getVSR()/putVSR() and further tidy-up
Posted by no-reply@patchew.org 4 years, 10 months ago 
Patchew URL: https://patchew.org/QEMU/20190602110903.3431-1-mark.cave-ayland@ilande.co.uk/



Hi,

This series seems to have some coding style problems. See output below for
more information:

Subject: [Qemu-devel] [PATCH v2 00/15] target/ppc: remove getVSR()/putVSR() and further tidy-up
Type: series
Message-id: 20190602110903.3431-1-mark.cave-ayland@ilande.co.uk

=== TEST SCRIPT BEGIN ===
#!/bin/bash
git rev-parse base > /dev/null || exit 0
git config --local diff.renamelimit 0
git config --local diff.renames True
git config --local diff.algorithm histogram
./scripts/checkpatch.pl --mailback base..
=== TEST SCRIPT END ===

From https://github.com/patchew-project/qemu
   60905286cb..ad88e4252f  master     -> master
From https://github.com/patchew-project/qemu
 * [new tag]               patchew/20190602110903.3431-1-mark.cave-ayland@ilande.co.uk -> patchew/20190602110903.3431-1-mark.cave-ayland@ilande.co.uk
Switched to a new branch 'test'
868f640501 target/ppc: improve VSX_FMADD with new GEN_VSX_HELPER_VSX_MADD macro
11658435f9 target/ppc: decode target register in VSX_EXTRACT_INSERT at translation time
e9e52e3f0a target/ppc: decode target register in VSX_VECTOR_LOAD_STORE_LENGTH at translation time
239e07a0cf target/ppc: introduce GEN_VSX_HELPER_R2_AB macro to fpu_helper.c
6aeb8a2e36 target/ppc: introduce GEN_VSX_HELPER_R2 macro to fpu_helper.c
b43836d4e8 target/ppc: introduce GEN_VSX_HELPER_R3 macro to fpu_helper.c
241920056a target/ppc: introduce GEN_VSX_HELPER_X1 macro to fpu_helper.c
959261a074 target/ppc: introduce GEN_VSX_HELPER_X2_AB macro to fpu_helper.c
ecd519b86e target/ppc: introduce GEN_VSX_HELPER_X2 macro to fpu_helper.c
62b466770b target/ppc: introduce separate generator and helper for xscvqpdp
d8a5beec33 target/ppc: introduce GEN_VSX_HELPER_X3 macro to fpu_helper.c
b2de1761e5 target/ppc: introduce separate VSX_CMP macro for xvcmp* instructions
895b953f81 target/ppc: remove getVSR()/putVSR() from int_helper.c
ca941ffe9e target/ppc: remove getVSR()/putVSR() from mem_helper.c
eae59c79c5 target/ppc: remove getVSR()/putVSR() from fpu_helper.c

=== OUTPUT BEGIN ===
1/15 Checking commit eae59c79c5ea (target/ppc: remove getVSR()/putVSR() from fpu_helper.c)
2/15 Checking commit ca941ffe9eaf (target/ppc: remove getVSR()/putVSR() from mem_helper.c)
3/15 Checking commit 895b953f8175 (target/ppc: remove getVSR()/putVSR() from int_helper.c)
4/15 Checking commit b2de1761e581 (target/ppc: introduce separate VSX_CMP macro for xvcmp* instructions)
5/15 Checking commit d8a5beec33bd (target/ppc: introduce GEN_VSX_HELPER_X3 macro to fpu_helper.c)
6/15 Checking commit 62b466770baa (target/ppc: introduce separate generator and helper for xscvqpdp)
7/15 Checking commit ecd519b86eec (target/ppc: introduce GEN_VSX_HELPER_X2 macro to fpu_helper.c)
8/15 Checking commit 959261a07480 (target/ppc: introduce GEN_VSX_HELPER_X2_AB macro to fpu_helper.c)
9/15 Checking commit 241920056ab0 (target/ppc: introduce GEN_VSX_HELPER_X1 macro to fpu_helper.c)
10/15 Checking commit b43836d4e8bb (target/ppc: introduce GEN_VSX_HELPER_R3 macro to fpu_helper.c)
11/15 Checking commit 6aeb8a2e36c7 (target/ppc: introduce GEN_VSX_HELPER_R2 macro to fpu_helper.c)
12/15 Checking commit 239e07a0cffa (target/ppc: introduce GEN_VSX_HELPER_R2_AB macro to fpu_helper.c)
13/15 Checking commit e9e52e3f0a39 (target/ppc: decode target register in VSX_VECTOR_LOAD_STORE_LENGTH at translation time)
14/15 Checking commit 11658435f9ff (target/ppc: decode target register in VSX_EXTRACT_INSERT at translation time)
15/15 Checking commit 868f6405015c (target/ppc: improve VSX_FMADD with new GEN_VSX_HELPER_VSX_MADD macro)
WARNING: Block comments use a leading /* on a separate line
#278: FILE: target/ppc/translate/vsx-impl.inc.c:1311:
+        /*                                                                    \

WARNING: Block comments use a leading /* on a separate line
#284: FILE: target/ppc/translate/vsx-impl.inc.c:1317:
+        /*                                                                    \

ERROR: Macros with complex values should be enclosed in parenthesis
#325: FILE: target/ppc/translate/vsx-ops.inc.c:66:
+#define GEN_XX3FORM_NAME(name, opcname, opc2, opc3, fl2)               \
+GEN_HANDLER2_E(name, opcname, 0x3C, opc2 | 0, opc3, 0, PPC_NONE, fl2), \
+GEN_HANDLER2_E(name, opcname, 0x3C, opc2 | 1, opc3, 0, PPC_NONE, fl2), \
+GEN_HANDLER2_E(name, opcname, 0x3C, opc2 | 2, opc3, 0, PPC_NONE, fl2), \
+GEN_HANDLER2_E(name, opcname, 0x3C, opc2 | 3, opc3, 0, PPC_NONE, fl2)

total: 1 errors, 2 warnings, 377 lines checked

Patch 15/15 has style problems, please review.  If any of these errors
are false positives report them to the maintainer, see
CHECKPATCH in MAINTAINERS.

=== OUTPUT END ===

Test command exited with code: 1


The full log is available at
http://patchew.org/logs/20190602110903.3431-1-mark.cave-ayland@ilande.co.uk/testing.checkpatch/?type=message.
---
Email generated automatically by Patchew [https://patchew.org/].
Please send your feedback to patchew-devel@redhat.com
Re: [Qemu-devel] [PATCH v2 00/15] target/ppc: remove getVSR()/putVSR() and further tidy-up
Posted by Mark Cave-Ayland 4 years, 10 months ago 
On 03/06/2019 11:48, no-reply@patchew.org wrote:

> Patchew URL: https://patchew.org/QEMU/20190602110903.3431-1-mark.cave-ayland@ilande.co.uk/
> 
> 
> 
> Hi,
> 
> This series seems to have some coding style problems. See output below for
> more information:
> 
> Subject: [Qemu-devel] [PATCH v2 00/15] target/ppc: remove getVSR()/putVSR() and further tidy-up
> Type: series
> Message-id: 20190602110903.3431-1-mark.cave-ayland@ilande.co.uk
> 
> === TEST SCRIPT BEGIN ===
> #!/bin/bash
> git rev-parse base > /dev/null || exit 0
> git config --local diff.renamelimit 0
> git config --local diff.renames True
> git config --local diff.algorithm histogram
> ./scripts/checkpatch.pl --mailback base..
> === TEST SCRIPT END ===
> 
> From https://github.com/patchew-project/qemu
>    60905286cb..ad88e4252f  master     -> master
> From https://github.com/patchew-project/qemu
>  * [new tag]               patchew/20190602110903.3431-1-mark.cave-ayland@ilande.co.uk -> patchew/20190602110903.3431-1-mark.cave-ayland@ilande.co.uk
> Switched to a new branch 'test'
> 868f640501 target/ppc: improve VSX_FMADD with new GEN_VSX_HELPER_VSX_MADD macro
> 11658435f9 target/ppc: decode target register in VSX_EXTRACT_INSERT at translation time
> e9e52e3f0a target/ppc: decode target register in VSX_VECTOR_LOAD_STORE_LENGTH at translation time
> 239e07a0cf target/ppc: introduce GEN_VSX_HELPER_R2_AB macro to fpu_helper.c
> 6aeb8a2e36 target/ppc: introduce GEN_VSX_HELPER_R2 macro to fpu_helper.c
> b43836d4e8 target/ppc: introduce GEN_VSX_HELPER_R3 macro to fpu_helper.c
> 241920056a target/ppc: introduce GEN_VSX_HELPER_X1 macro to fpu_helper.c
> 959261a074 target/ppc: introduce GEN_VSX_HELPER_X2_AB macro to fpu_helper.c
> ecd519b86e target/ppc: introduce GEN_VSX_HELPER_X2 macro to fpu_helper.c
> 62b466770b target/ppc: introduce separate generator and helper for xscvqpdp
> d8a5beec33 target/ppc: introduce GEN_VSX_HELPER_X3 macro to fpu_helper.c
> b2de1761e5 target/ppc: introduce separate VSX_CMP macro for xvcmp* instructions
> 895b953f81 target/ppc: remove getVSR()/putVSR() from int_helper.c
> ca941ffe9e target/ppc: remove getVSR()/putVSR() from mem_helper.c
> eae59c79c5 target/ppc: remove getVSR()/putVSR() from fpu_helper.c
> 
> === OUTPUT BEGIN ===
> 1/15 Checking commit eae59c79c5ea (target/ppc: remove getVSR()/putVSR() from fpu_helper.c)
> 2/15 Checking commit ca941ffe9eaf (target/ppc: remove getVSR()/putVSR() from mem_helper.c)
> 3/15 Checking commit 895b953f8175 (target/ppc: remove getVSR()/putVSR() from int_helper.c)
> 4/15 Checking commit b2de1761e581 (target/ppc: introduce separate VSX_CMP macro for xvcmp* instructions)
> 5/15 Checking commit d8a5beec33bd (target/ppc: introduce GEN_VSX_HELPER_X3 macro to fpu_helper.c)
> 6/15 Checking commit 62b466770baa (target/ppc: introduce separate generator and helper for xscvqpdp)
> 7/15 Checking commit ecd519b86eec (target/ppc: introduce GEN_VSX_HELPER_X2 macro to fpu_helper.c)
> 8/15 Checking commit 959261a07480 (target/ppc: introduce GEN_VSX_HELPER_X2_AB macro to fpu_helper.c)
> 9/15 Checking commit 241920056ab0 (target/ppc: introduce GEN_VSX_HELPER_X1 macro to fpu_helper.c)
> 10/15 Checking commit b43836d4e8bb (target/ppc: introduce GEN_VSX_HELPER_R3 macro to fpu_helper.c)
> 11/15 Checking commit 6aeb8a2e36c7 (target/ppc: introduce GEN_VSX_HELPER_R2 macro to fpu_helper.c)
> 12/15 Checking commit 239e07a0cffa (target/ppc: introduce GEN_VSX_HELPER_R2_AB macro to fpu_helper.c)
> 13/15 Checking commit e9e52e3f0a39 (target/ppc: decode target register in VSX_VECTOR_LOAD_STORE_LENGTH at translation time)
> 14/15 Checking commit 11658435f9ff (target/ppc: decode target register in VSX_EXTRACT_INSERT at translation time)
> 15/15 Checking commit 868f6405015c (target/ppc: improve VSX_FMADD with new GEN_VSX_HELPER_VSX_MADD macro)
> WARNING: Block comments use a leading /* on a separate line
> #278: FILE: target/ppc/translate/vsx-impl.inc.c:1311:
> +        /*                                                                    \

It looks like checkpatch is getting confused here because the comment is part of a
macro definition with the '\' continuation marker at the end of the line.

> WARNING: Block comments use a leading /* on a separate line
> #284: FILE: target/ppc/translate/vsx-impl.inc.c:1317:
> +        /*                                                                    \
> 
> ERROR: Macros with complex values should be enclosed in parenthesis
> #325: FILE: target/ppc/translate/vsx-ops.inc.c:66:
> +#define GEN_XX3FORM_NAME(name, opcname, opc2, opc3, fl2)               \
> +GEN_HANDLER2_E(name, opcname, 0x3C, opc2 | 0, opc3, 0, PPC_NONE, fl2), \
> +GEN_HANDLER2_E(name, opcname, 0x3C, opc2 | 1, opc3, 0, PPC_NONE, fl2), \
> +GEN_HANDLER2_E(name, opcname, 0x3C, opc2 | 2, opc3, 0, PPC_NONE, fl2), \
> +GEN_HANDLER2_E(name, opcname, 0x3C, opc2 | 3, opc3, 0, PPC_NONE, fl2)
> 
> total: 1 errors, 2 warnings, 377 lines checked

And this is just another "macro of macros" in the same style as all the others within
the file so I'm not convinced that this is relevant here either.

> Patch 15/15 has style problems, please review.  If any of these errors
> are false positives report them to the maintainer, see
> CHECKPATCH in MAINTAINERS.
> 
> === OUTPUT END ===
> 
> Test command exited with code: 1
> 
> 
> The full log is available at
> http://patchew.org/logs/20190602110903.3431-1-mark.cave-ayland@ilande.co.uk/testing.checkpatch/?type=message.
> ---
> Email generated automatically by Patchew [https://patchew.org/].
> Please send your feedback to patchew-devel@redhat.com


ATB,

Mark.

Patchew 2.1-devel-0870f84

© 2016 - 2024 Red Hat, Inc.