| Message ID | 20190629130017.2973-1-richard.henderson@linaro.org |
|---|---|
| Headers | show |
| Series | tcg/ppc: Add vector opcodes | expand |
Patchew URL: https://patchew.org/QEMU/20190629130017.2973-1-richard.henderson@linaro.org/
Hi,
This series seems to have some coding style problems. See output below for
more information:
Subject: [Qemu-devel] [PATCH v6 00/16] tcg/ppc: Add vector opcodes
Message-id: 20190629130017.2973-1-richard.henderson@linaro.org
Type: series
=== 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 ===
Switched to a new branch 'test'
735e428 tcg/ppc: Update vector support to v3.00
d5df8ce tcg/ppc: Update vector support to v2.07
70bae8c tcg/ppc: Update vector support to v2.06
cdcb6fd tcg/ppc: Enable Altivec detection
5eca04a tcg/ppc: Support vector dup2
9a92a5b tcg/ppc: Support vector multiply
9dcbbb5 tcg/ppc: Support vector shift by immediate
5707cff tcg/ppc: Prepare case for vector multiply
4e8c856 tcg/ppc: Add support for vector saturated add/subtract
8542349 tcg/ppc: Add support for vector add/subtract
09dcca3 tcg/ppc: Add support for vector maximum/minimum
940d802 tcg/ppc: Add support for load/store/logic/comparison
1354b48 tcg/ppc: Enable tcg backend vector compilation
ce65dc7 tcg/ppc: Introduce macros VRT(), VRA(), VRB(), VRC()
c15e076 tcg/ppc: Introduce macro VX4()
a351796 tcg/ppc: Introduce Altivec registers
=== OUTPUT BEGIN ===
1/16 Checking commit a35179674cf2 (tcg/ppc: Introduce Altivec registers)
2/16 Checking commit c15e076c7d0f (tcg/ppc: Introduce macro VX4())
ERROR: spaces required around that '|' (ctx:VxV)
#21: FILE: tcg/ppc/tcg-target.inc.c:322:
+#define VX4(opc) (OPCD(4)|(opc))
^
total: 1 errors, 0 warnings, 7 lines checked
Patch 2/16 has style problems, please review. If any of these errors
are false positives report them to the maintainer, see
CHECKPATCH in MAINTAINERS.
3/16 Checking commit ce65dc76f743 (tcg/ppc: Introduce macros VRT(), VRA(), VRB(), VRC())
4/16 Checking commit 1354b48a4dce (tcg/ppc: Enable tcg backend vector compilation)
WARNING: Block comments use a leading /* on a separate line
#155: FILE: tcg/ppc/tcg-target.inc.c:2842:
+ if (hwcap & /* PPC_FEATURE_HAS_ALTIVEC -- NOT YET */ 0) {
WARNING: added, moved or deleted file(s), does MAINTAINERS need updating?
#173:
new file mode 100644
total: 0 errors, 2 warnings, 138 lines checked
Patch 4/16 has style problems, please review. If any of these errors
are false positives report them to the maintainer, see
CHECKPATCH in MAINTAINERS.
5/16 Checking commit 940d8027994d (tcg/ppc: Add support for load/store/logic/comparison)
6/16 Checking commit 09dcca3c9f87 (tcg/ppc: Add support for vector maximum/minimum)
7/16 Checking commit 8542349a45f4 (tcg/ppc: Add support for vector add/subtract)
8/16 Checking commit 4e8c8565186d (tcg/ppc: Add support for vector saturated add/subtract)
9/16 Checking commit 5707cff60faf (tcg/ppc: Prepare case for vector multiply)
10/16 Checking commit 9dcbbb561046 (tcg/ppc: Support vector shift by immediate)
11/16 Checking commit 9a92a5bffebd (tcg/ppc: Support vector multiply)
ERROR: code indent should never use tabs
#133: FILE: tcg/ppc/tcg-target.inc.c:3220:
+^Ibreak;$
total: 1 errors, 0 warnings, 185 lines checked
Patch 11/16 has style problems, please review. If any of these errors
are false positives report them to the maintainer, see
CHECKPATCH in MAINTAINERS.
12/16 Checking commit 5eca04a86aea (tcg/ppc: Support vector dup2)
13/16 Checking commit cdcb6fdbe190 (tcg/ppc: Enable Altivec detection)
14/16 Checking commit 70bae8c6b3df (tcg/ppc: Update vector support to v2.06)
15/16 Checking commit d5df8cec3718 (tcg/ppc: Update vector support to v2.07)
16/16 Checking commit 735e428f5f2a (tcg/ppc: Update vector support to v3.00)
=== OUTPUT END ===
Test command exited with code: 1
The full log is available at
http://patchew.org/logs/20190629130017.2973-1-richard.henderson@linaro.org/testing.checkpatch/?type=message.
---
Email generated automatically by Patchew [https://patchew.org/].
Please send your feedback to patchew-devel@redhat.com
On 29/06/2019 14:00, Richard Henderson wrote: > Changes since v5: > * Disable runtime altivec detection until all of the required > opcodes are implemented. > Because dup2 was last, that really means all of the pure altivec > bits, so the initial patches are not bisectable in any meaningful > sense. I thought about reshuffling dup2 earlier, but that created > too many conflicts and I was too lazy. > * Rearranged the patches a little bit to make sure that each > one actually builds, which was not the case before. > * Folded in the fix to tcg_out_mem_long, as discussed in the > followup within the v4 thread. > > Changes since v4: > * Patch 1, "tcg/ppc: Introduce Altivec registers", is divided into > ten smaller patches. > * The net result (code-wise) is not changed between former patch 1 > and ten new patches. > * Remaining (2-7) patches from v4 are applied verbatim. > * This means that code-wise v5 and v4 do not differ. > * v5 is devised to help debugging, and to better organize the code. > > Changes since v3: > * Add support for bitsel, with the vsx xxsel insn. > * Rely on the new relocation overflow handling, so > we don't require 3 insns for a vector load. > > Changes since v2: > * Several generic tcg patches to improve dup vs dupi vs dupm. > In particular, if a global temp (like guest r10) is not in > a host register, we should duplicate from memory instead of > loading to an integer register, spilling to stack, loading > to a vector register, and then duplicating. > * I have more confidence that 32-bit ppc host should work > this time around. No testing on that front yet, but I've > unified some code sequences with 64-bit ppc host. > * Base altivec now supports V128 only. Moved V64 support to > Power7 (v2.06), which has 64-bit load/store. > * Dropped support for 64-bit vector multiply using Power8. > The expansion was too large compared to using integer regs. > > Richard Henderson (16): > tcg/ppc: Introduce Altivec registers > tcg/ppc: Introduce macro VX4() > tcg/ppc: Introduce macros VRT(), VRA(), VRB(), VRC() > tcg/ppc: Enable tcg backend vector compilation > tcg/ppc: Add support for load/store/logic/comparison > tcg/ppc: Add support for vector maximum/minimum > tcg/ppc: Add support for vector add/subtract > tcg/ppc: Add support for vector saturated add/subtract > tcg/ppc: Prepare case for vector multiply > tcg/ppc: Support vector shift by immediate > tcg/ppc: Support vector multiply > tcg/ppc: Support vector dup2 > tcg/ppc: Enable Altivec detection > tcg/ppc: Update vector support to v2.06 > tcg/ppc: Update vector support to v2.07 > tcg/ppc: Update vector support to v3.00 > > tcg/ppc/tcg-target.h | 39 +- > tcg/ppc/tcg-target.opc.h | 13 + > tcg/ppc/tcg-target.inc.c | 1091 +++++++++++++++++++++++++++++++++++--- > 3 files changed, 1076 insertions(+), 67 deletions(-) > create mode 100644 tcg/ppc/tcg-target.opc.h I don't have space for a full set of images on the G4, however I've tried boot tests on installer CDs for MacOS 9, OS X 10.2, Linux and HelenOS and it looks good here. Tested-by: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk> [PPC32] ATB, Mark.
On 6/30/19 7:58 PM, Mark Cave-Ayland wrote: > I don't have space for a full set of images on the G4, however I've tried boot tests > on installer CDs for MacOS 9, OS X 10.2, Linux and HelenOS and it looks good here. > > Tested-by: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk> [PPC32] Thanks! r!
On Mon, Jul 1, 2019 at 12:30 PM Richard Henderson < richard.henderson@linaro.org> wrote: > On 6/30/19 7:58 PM, Mark Cave-Ayland wrote: > > I don't have space for a full set of images on the G4, however I've > tried boot tests > > on installer CDs for MacOS 9, OS X 10.2, Linux and HelenOS and it looks > good here. > > > > Tested-by: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk> [PPC32] > > Thanks! > > Hi I just compiled the v6 set applied to current master on my G5, Ubuntu 16. command line: ./qemu-system-ppc -L pc-bios -boot c m 512 -M mac99,via=pmu \ -netdev user,id=net1 -device sungem,netdev=net1 \ -drive file=10.3.img,format=raw,media=disk \ With no specific cpu set, Mac OS 9.2 hard disk image and 9.2 iso do not get to the desktop, they just hang while still in the openbios window. They need -cpu G4 on the command line to get to the desktop. OSX 10.3 installed image boots to desktop. OSX 10.3 iso boots to installer OSX 10.4 installed image boots to desktop. OSX 10.4 iso boot to installer OSX 10.5 installed image boots to desktop. OSX 10.5 iso boots to installer So there seems to be a difference between hosts: If ran on a G4 host there is no need to add -cpu G4 to run Mac OS 9.x, while there is when ran on a G5 host. Best, Howard
On 01/07/2019 19:34, Howard Spoelstra wrote: > On Mon, Jul 1, 2019 at 12:30 PM Richard Henderson < > richard.henderson@linaro.org> wrote: > >> On 6/30/19 7:58 PM, Mark Cave-Ayland wrote: >>> I don't have space for a full set of images on the G4, however I've >> tried boot tests >>> on installer CDs for MacOS 9, OS X 10.2, Linux and HelenOS and it looks >> good here. >>> >>> Tested-by: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk> [PPC32] >> >> Thanks! >> >> Hi > > I just compiled the v6 set applied to current master on my G5, Ubuntu 16. > command line: > ./qemu-system-ppc -L pc-bios -boot c m 512 -M mac99,via=pmu \ > -netdev user,id=net1 -device sungem,netdev=net1 \ > -drive file=10.3.img,format=raw,media=disk \ > > With no specific cpu set, Mac OS 9.2 hard disk image and 9.2 iso do not get > to the desktop, they just hang while still in the openbios window. They > need -cpu G4 on the command line to get to the desktop. > > OSX 10.3 installed image boots to desktop. > OSX 10.3 iso boots to installer > OSX 10.4 installed image boots to desktop. > OSX 10.4 iso boot to installer > OSX 10.5 installed image boots to desktop. > OSX 10.5 iso boots to installer > > So there seems to be a difference between hosts: If ran on a G4 host there > is no need to add -cpu G4 to run Mac OS 9.x, while there is when ran on a > G5 host. Are there any outstanding issues with this patchset now, or is it ready to be merged? I'm really looking forward to seeing the improved performance when testing QEMU on my Mac Mini :) ATB, Mark.
On Tue, Sep 3, 2019 at 7:05 PM Mark Cave-Ayland < mark.cave-ayland@ilande.co.uk> wrote: > On 01/07/2019 19:34, Howard Spoelstra wrote: > > > On Mon, Jul 1, 2019 at 12:30 PM Richard Henderson < > > richard.henderson@linaro.org> wrote: > > > >> On 6/30/19 7:58 PM, Mark Cave-Ayland wrote: > >>> I don't have space for a full set of images on the G4, however I've > >> tried boot tests > >>> on installer CDs for MacOS 9, OS X 10.2, Linux and HelenOS and it looks > >> good here. > >>> > >>> Tested-by: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk> [PPC32] > >> > >> Thanks! > >> > >> Hi > > > > I just compiled the v6 set applied to current master on my G5, Ubuntu 16. > > command line: > > ./qemu-system-ppc -L pc-bios -boot c m 512 -M mac99,via=pmu \ > > -netdev user,id=net1 -device sungem,netdev=net1 \ > > -drive file=10.3.img,format=raw,media=disk \ > > > > With no specific cpu set, Mac OS 9.2 hard disk image and 9.2 iso do not > get > > to the desktop, they just hang while still in the openbios window. They > > need -cpu G4 on the command line to get to the desktop. > > > > OSX 10.3 installed image boots to desktop. > > OSX 10.3 iso boots to installer > > OSX 10.4 installed image boots to desktop. > > OSX 10.4 iso boot to installer > > OSX 10.5 installed image boots to desktop. > > OSX 10.5 iso boots to installer > > > > So there seems to be a difference between hosts: If ran on a G4 host > there > > is no need to add -cpu G4 to run Mac OS 9.x, while there is when ran on a > > G5 host. > > Are there any outstanding issues with this patchset now, or is it ready to > be merged? > I'm really looking forward to seeing the improved performance when testing > QEMU on my > Mac Mini :) > > Howard pointed to some illogical quirks of command line: > If ran on a G4 host there is no need to add -cpu G4 to run Mac OS 9.x, > while there is when ran on a G5 host. I am not sure if Howard says that this is a consequence of this series though. Overall, I think this is a very good series - however, I had a number of minor objections to multiple patches, that don't affect (or affect in a minimal way) provided functionality - those objections are not addressed, nor properly discussed - but I do think they should be addressed in order to get the series in a better shape before upstreaming. Thanks, Aleksandar > ATB, > > Mark. > >
On 03/09/2019 18:37, Aleksandar Markovic wrote: > On Tue, Sep 3, 2019 at 7:05 PM Mark Cave-Ayland < > mark.cave-ayland@ilande.co.uk> wrote: > >> On 01/07/2019 19:34, Howard Spoelstra wrote: >> >>> On Mon, Jul 1, 2019 at 12:30 PM Richard Henderson < >>> richard.henderson@linaro.org> wrote: >>> >>>> On 6/30/19 7:58 PM, Mark Cave-Ayland wrote: >>>>> I don't have space for a full set of images on the G4, however I've >>>> tried boot tests >>>>> on installer CDs for MacOS 9, OS X 10.2, Linux and HelenOS and it looks >>>> good here. >>>>> >>>>> Tested-by: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk> [PPC32] >>>> >>>> Thanks! >>>> >>>> Hi >>> >>> I just compiled the v6 set applied to current master on my G5, Ubuntu 16. >>> command line: >>> ./qemu-system-ppc -L pc-bios -boot c m 512 -M mac99,via=pmu \ >>> -netdev user,id=net1 -device sungem,netdev=net1 \ >>> -drive file=10.3.img,format=raw,media=disk \ >>> >>> With no specific cpu set, Mac OS 9.2 hard disk image and 9.2 iso do not >> get >>> to the desktop, they just hang while still in the openbios window. They >>> need -cpu G4 on the command line to get to the desktop. >>> >>> OSX 10.3 installed image boots to desktop. >>> OSX 10.3 iso boots to installer >>> OSX 10.4 installed image boots to desktop. >>> OSX 10.4 iso boot to installer >>> OSX 10.5 installed image boots to desktop. >>> OSX 10.5 iso boots to installer >>> >>> So there seems to be a difference between hosts: If ran on a G4 host >> there >>> is no need to add -cpu G4 to run Mac OS 9.x, while there is when ran on a >>> G5 host. >> >> Are there any outstanding issues with this patchset now, or is it ready to >> be merged? >> I'm really looking forward to seeing the improved performance when testing >> QEMU on my >> Mac Mini :) >> >> > Howard pointed to some illogical quirks of command line: > >> If ran on a G4 host there is no need to add -cpu G4 to run Mac OS 9.x, >> while there is when ran on a G5 host. > > I am not sure if Howard says that this is a consequence of this series > though. No, that has been an existing issue for a long time :) > Overall, I think this is a very good series - however, I had a number of > minor > objections to multiple patches, that don't affect (or affect in a minimal > way) > provided functionality - those objections are not addressed, nor properly > discussed - but I do think they should be addressed in order to get the > series > in a better shape before upstreaming. I've had a quick look at some of your review comments, and certainly I can see how the earlier revisions have benefited from your feedback. There has been a lot of positive discussion, and Richard has taken the time to respond and update the patchset over several weeks to its latest revision. AFAICT the only remaining issue is that related to the ISA flags, but to me this isn't something that should prevent the patchset being merged. I can certainly see how the current flags implementation may not be considered technically correct, but then from your comments I don't see that it would be something that would be particularly difficult to change at a later date either. The things that are important to me are i) is the patchset functionally correct and ii) is it understandable and maintainable. I would say that the first point is clearly true (both myself and Howard have spent a lot of time testing it), and given that I had to delve into these patches to fix the R2 register issue on 32-bit PPC then I can confirm that the contents of the patches were a reasonably accurate representation of the changes described within. And that's from someone like me who is mostly still a TCG beginner :) From a slightly more selfish position as the PPC Mac machine maintainer, these patches make a significant difference to me in that they reduce the MacOS boot times during everyday testing. Now for someone like myself who works on QEMU as a hobby outside of family life and a full time job, those few minutes are really important to me and soon add up really quickly during testing. I would really like these patches to be merged soon, since the worst thing that can happen is that the patchset ends up bit-rotting and then all the time and effort put into writing, testing and reviewing the patches by Richard, Howard, David, myself and indeed your review time will all end up going to waste. ATB, Mark.
ping for Richard 03.09.2019. 20.34, "Mark Cave-Ayland" <mark.cave-ayland@ilande.co.uk> је написао/ла: > > On 03/09/2019 18:37, Aleksandar Markovic wrote: > > > On Tue, Sep 3, 2019 at 7:05 PM Mark Cave-Ayland < > > mark.cave-ayland@ilande.co.uk> wrote: > > > >> On 01/07/2019 19:34, Howard Spoelstra wrote: > >> > >>> On Mon, Jul 1, 2019 at 12:30 PM Richard Henderson < > >>> richard.henderson@linaro.org> wrote: > >>> > >>>> On 6/30/19 7:58 PM, Mark Cave-Ayland wrote: > >>>>> I don't have space for a full set of images on the G4, however I've > >>>> tried boot tests > >>>>> on installer CDs for MacOS 9, OS X 10.2, Linux and HelenOS and it looks > >>>> good here. > >>>>> > >>>>> Tested-by: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk> [PPC32] > >>>> > >>>> Thanks! > >>>> > >>>> Hi > >>> > >>> I just compiled the v6 set applied to current master on my G5, Ubuntu 16. > >>> command line: > >>> ./qemu-system-ppc -L pc-bios -boot c m 512 -M mac99,via=pmu \ > >>> -netdev user,id=net1 -device sungem,netdev=net1 \ > >>> -drive file=10.3.img,format=raw,media=disk \ > >>> > >>> With no specific cpu set, Mac OS 9.2 hard disk image and 9.2 iso do not > >> get > >>> to the desktop, they just hang while still in the openbios window. They > >>> need -cpu G4 on the command line to get to the desktop. > >>> > >>> OSX 10.3 installed image boots to desktop. > >>> OSX 10.3 iso boots to installer > >>> OSX 10.4 installed image boots to desktop. > >>> OSX 10.4 iso boot to installer > >>> OSX 10.5 installed image boots to desktop. > >>> OSX 10.5 iso boots to installer > >>> > >>> So there seems to be a difference between hosts: If ran on a G4 host > >> there > >>> is no need to add -cpu G4 to run Mac OS 9.x, while there is when ran on a > >>> G5 host. > >> > >> Are there any outstanding issues with this patchset now, or is it ready to > >> be merged? > >> I'm really looking forward to seeing the improved performance when testing > >> QEMU on my > >> Mac Mini :) > >> > >> > > Howard pointed to some illogical quirks of command line: > > > >> If ran on a G4 host there is no need to add -cpu G4 to run Mac OS 9.x, > >> while there is when ran on a G5 host. > > > > I am not sure if Howard says that this is a consequence of this series > > though. > > No, that has been an existing issue for a long time :) > > > Overall, I think this is a very good series - however, I had a number of > > minor > > objections to multiple patches, that don't affect (or affect in a minimal > > way) > > provided functionality - those objections are not addressed, nor properly > > discussed - but I do think they should be addressed in order to get the > > series > > in a better shape before upstreaming. > > I've had a quick look at some of your review comments, and certainly I can see how > the earlier revisions have benefited from your feedback. There has been a lot of > positive discussion, and Richard has taken the time to respond and update the > patchset over several weeks to its latest revision. > > AFAICT the only remaining issue is that related to the ISA flags, but to me this > isn't something that should prevent the patchset being merged. I can certainly see > how the current flags implementation may not be considered technically correct, but > then from your comments I don't see that it would be something that would be > particularly difficult to change at a later date either. > > The things that are important to me are i) is the patchset functionally correct and > ii) is it understandable and maintainable. I would say that the first point is > clearly true (both myself and Howard have spent a lot of time testing it), and given > that I had to delve into these patches to fix the R2 register issue on 32-bit PPC > then I can confirm that the contents of the patches were a reasonably accurate > representation of the changes described within. And that's from someone like me who > is mostly still a TCG beginner :) > > From a slightly more selfish position as the PPC Mac machine maintainer, these > patches make a significant difference to me in that they reduce the MacOS boot times > during everyday testing. Now for someone like myself who works on QEMU as a hobby > outside of family life and a full time job, those few minutes are really important to > me and soon add up really quickly during testing. > > I would really like these patches to be merged soon, since the worst thing that can > happen is that the patchset ends up bit-rotting and then all the time and effort put > into writing, testing and reviewing the patches by Richard, Howard, David, myself and > indeed your review time will all end up going to waste. > > > ATB, > > Mark. >
ping 05.09.2019. 13.43, "Aleksandar Markovic" <aleksandar.m.mail@gmail.com> је написао/ла: > > > ping for Richard > > 03.09.2019. 20.34, "Mark Cave-Ayland" <mark.cave-ayland@ilande.co.uk> је написао/ла: > > > > On 03/09/2019 18:37, Aleksandar Markovic wrote: > > > > > On Tue, Sep 3, 2019 at 7:05 PM Mark Cave-Ayland < > > > mark.cave-ayland@ilande.co.uk> wrote: > > > > > >> On 01/07/2019 19:34, Howard Spoelstra wrote: > > >> > > >>> On Mon, Jul 1, 2019 at 12:30 PM Richard Henderson < > > >>> richard.henderson@linaro.org> wrote: > > >>> > > >>>> On 6/30/19 7:58 PM, Mark Cave-Ayland wrote: > > >>>>> I don't have space for a full set of images on the G4, however I've > > >>>> tried boot tests > > >>>>> on installer CDs for MacOS 9, OS X 10.2, Linux and HelenOS and it looks > > >>>> good here. > > >>>>> > > >>>>> Tested-by: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk> [PPC32] > > >>>> > > >>>> Thanks! > > >>>> > > >>>> Hi > > >>> > > >>> I just compiled the v6 set applied to current master on my G5, Ubuntu 16. > > >>> command line: > > >>> ./qemu-system-ppc -L pc-bios -boot c m 512 -M mac99,via=pmu \ > > >>> -netdev user,id=net1 -device sungem,netdev=net1 \ > > >>> -drive file=10.3.img,format=raw,media=disk \ > > >>> > > >>> With no specific cpu set, Mac OS 9.2 hard disk image and 9.2 iso do not > > >> get > > >>> to the desktop, they just hang while still in the openbios window. They > > >>> need -cpu G4 on the command line to get to the desktop. > > >>> > > >>> OSX 10.3 installed image boots to desktop. > > >>> OSX 10.3 iso boots to installer > > >>> OSX 10.4 installed image boots to desktop. > > >>> OSX 10.4 iso boot to installer > > >>> OSX 10.5 installed image boots to desktop. > > >>> OSX 10.5 iso boots to installer > > >>> > > >>> So there seems to be a difference between hosts: If ran on a G4 host > > >> there > > >>> is no need to add -cpu G4 to run Mac OS 9.x, while there is when ran on a > > >>> G5 host. > > >> > > >> Are there any outstanding issues with this patchset now, or is it ready to > > >> be merged? > > >> I'm really looking forward to seeing the improved performance when testing > > >> QEMU on my > > >> Mac Mini :) > > >> > > >> > > > Howard pointed to some illogical quirks of command line: > > > > > >> If ran on a G4 host there is no need to add -cpu G4 to run Mac OS 9.x, > > >> while there is when ran on a G5 host. > > > > > > I am not sure if Howard says that this is a consequence of this series > > > though. > > > > No, that has been an existing issue for a long time :) > > > > > Overall, I think this is a very good series - however, I had a number of > > > minor > > > objections to multiple patches, that don't affect (or affect in a minimal > > > way) > > > provided functionality - those objections are not addressed, nor properly > > > discussed - but I do think they should be addressed in order to get the > > > series > > > in a better shape before upstreaming. > > > > I've had a quick look at some of your review comments, and certainly I can see how > > the earlier revisions have benefited from your feedback. There has been a lot of > > positive discussion, and Richard has taken the time to respond and update the > > patchset over several weeks to its latest revision. > > > > AFAICT the only remaining issue is that related to the ISA flags, but to me this > > isn't something that should prevent the patchset being merged. I can certainly see > > how the current flags implementation may not be considered technically correct, but > > then from your comments I don't see that it would be something that would be > > particularly difficult to change at a later date either. > > > > The things that are important to me are i) is the patchset functionally correct and > > ii) is it understandable and maintainable. I would say that the first point is > > clearly true (both myself and Howard have spent a lot of time testing it), and given > > that I had to delve into these patches to fix the R2 register issue on 32-bit PPC > > then I can confirm that the contents of the patches were a reasonably accurate > > representation of the changes described within. And that's from someone like me who > > is mostly still a TCG beginner :) > > > > From a slightly more selfish position as the PPC Mac machine maintainer, these > > patches make a significant difference to me in that they reduce the MacOS boot times > > during everyday testing. Now for someone like myself who works on QEMU as a hobby > > outside of family life and a full time job, those few minutes are really important to > > me and soon add up really quickly during testing. > > > > I would really like these patches to be merged soon, since the worst thing that can > > happen is that the patchset ends up bit-rotting and then all the time and effort put > > into writing, testing and reviewing the patches by Richard, Howard, David, myself and > > indeed your review time will all end up going to waste. > > > > > > ATB, > > > > Mark. > > <p dir="ltr">ping</p> <p dir="ltr">05.09.2019. 13.43, "Aleksandar Markovic" <<a href="mailto:aleksandar.m.mail@gmail.com">aleksandar.m.mail@gmail.com</a>> је написао/ла:<br> ><br> ><br> > ping for Richard<br> ><br> > 03.09.2019. 20.34, "Mark Cave-Ayland" <<a href="mailto:mark.cave-ayland@ilande.co.uk">mark.cave-ayland@ilande.co.uk</a>> је написао/ла:<br> > ><br> > > On 03/09/2019 18:37, Aleksandar Markovic wrote:<br> > ><br> > > > On Tue, Sep 3, 2019 at 7:05 PM Mark Cave-Ayland <<br> > > > <a href="mailto:mark.cave-ayland@ilande.co.uk">mark.cave-ayland@ilande.co.uk</a>> wrote:<br> > > > <br> > > >> On 01/07/2019 19:34, Howard Spoelstra wrote:<br> > > >><br> > > >>> On Mon, Jul 1, 2019 at 12:30 PM Richard Henderson <<br> > > >>> <a href="mailto:richard.henderson@linaro.org">richard.henderson@linaro.org</a>> wrote:<br> > > >>><br> > > >>>> On 6/30/19 7:58 PM, Mark Cave-Ayland wrote:<br> > > >>>>> I don't have space for a full set of images on the G4, however I've<br> > > >>>> tried boot tests<br> > > >>>>> on installer CDs for MacOS 9, OS X 10.2, Linux and HelenOS and it looks<br> > > >>>> good here.<br> > > >>>>><br> > > >>>>> Tested-by: Mark Cave-Ayland <<a href="mailto:mark.cave-ayland@ilande.co.uk">mark.cave-ayland@ilande.co.uk</a>> [PPC32]<br> > > >>>><br> > > >>>> Thanks!<br> > > >>>><br> > > >>>> Hi<br> > > >>><br> > > >>> I just compiled the v6 set applied to current master on my G5, Ubuntu 16.<br> > > >>> command line:<br> > > >>> ./qemu-system-ppc -L pc-bios -boot c m 512 -M mac99,via=pmu \<br> > > >>> -netdev user,id=net1 -device sungem,netdev=net1 \<br> > > >>> -drive file=10.3.img,format=raw,media=disk \<br> > > >>><br> > > >>> With no specific cpu set, Mac OS 9.2 hard disk image and 9.2 iso do not<br> > > >> get<br> > > >>> to the desktop, they just hang while still in the openbios window. They<br> > > >>> need -cpu G4 on the command line to get to the desktop.<br> > > >>><br> > > >>> OSX 10.3 installed image boots to desktop.<br> > > >>> OSX 10.3 iso boots to installer<br> > > >>> OSX 10.4 installed image boots to desktop.<br> > > >>> OSX 10.4 iso boot to installer<br> > > >>> OSX 10.5 installed image boots to desktop.<br> > > >>> OSX 10.5 iso boots to installer<br> > > >>><br> > > >>> So there seems to be a difference between hosts: If ran on a G4 host<br> > > >> there<br> > > >>> is no need to add -cpu G4 to run Mac OS 9.x, while there is when ran on a<br> > > >>> G5 host.<br> > > >><br> > > >> Are there any outstanding issues with this patchset now, or is it ready to<br> > > >> be merged?<br> > > >> I'm really looking forward to seeing the improved performance when testing<br> > > >> QEMU on my<br> > > >> Mac Mini :)<br> > > >><br> > > >><br> > > > Howard pointed to some illogical quirks of command line:<br> > > > <br> > > >> If ran on a G4 host there is no need to add -cpu G4 to run Mac OS 9.x,<br> > > >> while there is when ran on a G5 host.<br> > > > <br> > > > I am not sure if Howard says that this is a consequence of this series<br> > > > though.<br> > ><br> > > No, that has been an existing issue for a long time :)<br> > ><br> > > > Overall, I think this is a very good series - however, I had a number of<br> > > > minor<br> > > > objections to multiple patches, that don't affect (or affect in a minimal<br> > > > way)<br> > > > provided functionality - those objections are not addressed, nor properly<br> > > > discussed - but I do think they should be addressed in order to get the<br> > > > series<br> > > > in a better shape before upstreaming.<br> > ><br> > > I've had a quick look at some of your review comments, and certainly I can see how<br> > > the earlier revisions have benefited from your feedback. There has been a lot of<br> > > positive discussion, and Richard has taken the time to respond and update the<br> > > patchset over several weeks to its latest revision.<br> > ><br> > > AFAICT the only remaining issue is that related to the ISA flags, but to me this<br> > > isn't something that should prevent the patchset being merged. I can certainly see<br> > > how the current flags implementation may not be considered technically correct, but<br> > > then from your comments I don't see that it would be something that would be<br> > > particularly difficult to change at a later date either.<br> > ><br> > > The things that are important to me are i) is the patchset functionally correct and<br> > > ii) is it understandable and maintainable. I would say that the first point is<br> > > clearly true (both myself and Howard have spent a lot of time testing it), and given<br> > > that I had to delve into these patches to fix the R2 register issue on 32-bit PPC<br> > > then I can confirm that the contents of the patches were a reasonably accurate<br> > > representation of the changes described within. And that's from someone like me who<br> > > is mostly still a TCG beginner :)<br> > ><br> > > From a slightly more selfish position as the PPC Mac machine maintainer, these<br> > > patches make a significant difference to me in that they reduce the MacOS boot times<br> > > during everyday testing. Now for someone like myself who works on QEMU as a hobby<br> > > outside of family life and a full time job, those few minutes are really important to<br> > > me and soon add up really quickly during testing.<br> > ><br> > > I would really like these patches to be merged soon, since the worst thing that can<br> > > happen is that the patchset ends up bit-rotting and then all the time and effort put<br> > > into writing, testing and reviewing the patches by Richard, Howard, David, myself and<br> > > indeed your review time will all end up going to waste.<br> > ><br> > ><br> > > ATB,<br> > ><br> > > Mark.<br> > ><br> </p>
Changes since v5: * Disable runtime altivec detection until all of the required opcodes are implemented. Because dup2 was last, that really means all of the pure altivec bits, so the initial patches are not bisectable in any meaningful sense. I thought about reshuffling dup2 earlier, but that created too many conflicts and I was too lazy. * Rearranged the patches a little bit to make sure that each one actually builds, which was not the case before. * Folded in the fix to tcg_out_mem_long, as discussed in the followup within the v4 thread. Changes since v4: * Patch 1, "tcg/ppc: Introduce Altivec registers", is divided into ten smaller patches. * The net result (code-wise) is not changed between former patch 1 and ten new patches. * Remaining (2-7) patches from v4 are applied verbatim. * This means that code-wise v5 and v4 do not differ. * v5 is devised to help debugging, and to better organize the code. Changes since v3: * Add support for bitsel, with the vsx xxsel insn. * Rely on the new relocation overflow handling, so we don't require 3 insns for a vector load. Changes since v2: * Several generic tcg patches to improve dup vs dupi vs dupm. In particular, if a global temp (like guest r10) is not in a host register, we should duplicate from memory instead of loading to an integer register, spilling to stack, loading to a vector register, and then duplicating. * I have more confidence that 32-bit ppc host should work this time around. No testing on that front yet, but I've unified some code sequences with 64-bit ppc host. * Base altivec now supports V128 only. Moved V64 support to Power7 (v2.06), which has 64-bit load/store. * Dropped support for 64-bit vector multiply using Power8. The expansion was too large compared to using integer regs. Richard Henderson (16): tcg/ppc: Introduce Altivec registers tcg/ppc: Introduce macro VX4() tcg/ppc: Introduce macros VRT(), VRA(), VRB(), VRC() tcg/ppc: Enable tcg backend vector compilation tcg/ppc: Add support for load/store/logic/comparison tcg/ppc: Add support for vector maximum/minimum tcg/ppc: Add support for vector add/subtract tcg/ppc: Add support for vector saturated add/subtract tcg/ppc: Prepare case for vector multiply tcg/ppc: Support vector shift by immediate tcg/ppc: Support vector multiply tcg/ppc: Support vector dup2 tcg/ppc: Enable Altivec detection tcg/ppc: Update vector support to v2.06 tcg/ppc: Update vector support to v2.07 tcg/ppc: Update vector support to v3.00 tcg/ppc/tcg-target.h | 39 +- tcg/ppc/tcg-target.opc.h | 13 + tcg/ppc/tcg-target.inc.c | 1091 +++++++++++++++++++++++++++++++++++--- 3 files changed, 1076 insertions(+), 67 deletions(-) create mode 100644 tcg/ppc/tcg-target.opc.h -- 2.17.1