Re: [Qemu-devel] [Qemu-ppc] [PATCH] ppc: Three floating point fixes
On Tue, Aug 20, 2019 at 11:35:29AM +0100, Peter Maydell wrote: > On Tue, 20 Aug 2019 at 08:36, David Gibson > wrote: > > On Mon, Aug 19, 2019 at 12:13:34PM -0500, Paul Clarke wrote: > > > These issues were found while running Glibc's test suite for "math", > > > and there are still a *LOT* of QEMU-only FAILs, so I may be back > > > again with suggested fixes or questions. :-) > > > > That doesn't greatly surprise me, TCG's ppc target stuff is only so-so > > tested, TBH. > > You might also consider using/extending the risu test cases for > ppc64 -- individual checks of each insn against a known-good > implementation can be easier to track down bugs than trying > to figure out why a higher-level test suite like the glibc one > has reported a failure, IME. (There are already risu patterns > for XSCVDPSP and XSCVDPSPN, so I think that bug at least ought > to be found by risu if you run it against the right h/w as > known-good reference...) Oh, I'd love to. I tried running risu once, it failed cryptically and I haven't had time to investigate deeper. -- David Gibson| I'll have my music baroque, and my code david AT gibson.dropbear.id.au | minimalist, thank you. NOT _the_ _other_ | _way_ _around_! http://www.ozlabs.org/~dgibson signature.asc Description: PGP signature
Re: [Qemu-devel] [Qemu-ppc] [PATCH] ppc: Three floating point fixes
On Tue, 20 Aug 2019 at 08:36, David Gibson wrote: > On Mon, Aug 19, 2019 at 12:13:34PM -0500, Paul Clarke wrote: > > These issues were found while running Glibc's test suite for "math", > > and there are still a *LOT* of QEMU-only FAILs, so I may be back > > again with suggested fixes or questions. :-) > > That doesn't greatly surprise me, TCG's ppc target stuff is only so-so > tested, TBH. You might also consider using/extending the risu test cases for ppc64 -- individual checks of each insn against a known-good implementation can be easier to track down bugs than trying to figure out why a higher-level test suite like the glibc one has reported a failure, IME. (There are already risu patterns for XSCVDPSP and XSCVDPSPN, so I think that bug at least ought to be found by risu if you run it against the right h/w as known-good reference...) thanks -- PMM
Re: [Qemu-devel] [Qemu-ppc] [PATCH] ppc: Three floating point fixes
On Mon, Aug 19, 2019 at 12:13:34PM -0500, Paul Clarke wrote: > On 8/19/19 1:44 AM, Aleksandar Markovic wrote: > > 19.08.2019. 08.30, "David Gibson" је > > написао/ла: > >> > >> On Sun, Aug 18, 2019 at 10:59:01PM +0200, Aleksandar Markovic wrote: > >>> 18.08.2019. 10.10, "Richard Henderson" је > >>> написао/ла: > > On 8/16/19 11:59 PM, Aleksandar Markovic wrote: > >> From: "Paul A. Clarke" > ... > >> ISA 3.0B has xscvdpspn leaving its result in word 1 of the target > > register, > >> and mffprwz expecting its input to come from word 0 of the source > > register. > >> This sequence fails with QEMU, as a shift is required between > > those > >>> two > >> instructions. However, the hardware splats the result to both > > word 0 > > and > >> word 1 of its output register, so the shift is not necessary. > >> Expect a future revision of the ISA to specify this behavior. > >> > > > > Hmmm... Isn't this a gcc bug (using undocumented hardware feature), > >>> given > > everything you said here? > > The key here is "expect a future revision of the ISA to specify this > >>> behavior". > > It's clearly within IBM's purview to adjust the specification to > > document > >>> a > previously undocumented hardware feature. > > >>> > >>> By no means, yes, the key is in ISA documentation. But, the impression > > that > >>> full original commit message conveys is that the main reason for change > > is > >>> gcc behavior. If we accepted in general that gcc behavior determines > > QEMU > >>> behavior, I am afraid we would be on a very slippery slope - therefore I > >>> think the commit message (and possible code comment) should, in my > > opinion, > >>> mention ISA docs as the central reason for change. Paul, is there any > >>> tentative release date of the new ISA specification? > >> > >> It's not really a question of gcc behaviour, it's a question of actual > >> cpu behaviour versus ISA document. Which one qemu should follow is > >> somewhat debatable, but it sounds here like the ISA will be updated to > >> match the cpu, which weights it heavily in favour of mimicing the > >> actual cpu. > >> > > > > This sounds right to me. > > Thanks for the reviews and great discussion. > > While not yet part of a published version of the ISA, I did find the behavior > documented in the User's Manuals for the POWER8 and POWER9 processors: > > https://www-355.ibm.com/systems/power/openpower/ > "Public Documents" > - "POWER9 Processor User's Manual" > - "POWER8 Processor User's Manual for the SCM" > > POWER9 Processor User's Manual > 4. Power Architecture Compliance > 4.3 Floating-Point Processor (FP, VMX, and VSX) > 4.3.7 Floating-Point Invalid Forms and Undefined Conditions > > POWER8 Processor User's Manual for the Single-Chip Module > 3. Power Architecture Compliance > 3.2 Floating-Point Processor (FP, VMX, and VSX) > 3.2.9 Floating-Point Invalid Forms and Undefined Conditions > > In a bullet: > - VSX scalar convert from double-precision to single-precision (xscvdpsp, > xscvdpspn). > VSR[32:63] is set to VSR[0:31]. > > I have not confirmed when the new revision of the ISA will be published, but > it's on somebody's "to do" list. > > I will respin the patch as 3 independent patches, and include a reference to > the above documents for the change under discussion here. (The other two > changes may take a bit more time, because like David, I find the FPU > emulation code cryptic. :-/ ) > > These issues were found while running Glibc's test suite for "math", > and there are still a *LOT* of QEMU-only FAILs, so I may be back > again with suggested fixes or questions. :-) That doesn't greatly surprise me, TCG's ppc target stuff is only so-so tested, TBH. -- David Gibson| I'll have my music baroque, and my code david AT gibson.dropbear.id.au | minimalist, thank you. NOT _the_ _other_ | _way_ _around_! http://www.ozlabs.org/~dgibson signature.asc Description: PGP signature
Re: [Qemu-devel] [Qemu-ppc] [PATCH] ppc: Three floating point fixes
On 8/19/19 1:44 AM, Aleksandar Markovic wrote: > 19.08.2019. 08.30, "David Gibson" је > написао/ла: >> >> On Sun, Aug 18, 2019 at 10:59:01PM +0200, Aleksandar Markovic wrote: >>> 18.08.2019. 10.10, "Richard Henderson" је >>> написао/ла: On 8/16/19 11:59 PM, Aleksandar Markovic wrote: >> From: "Paul A. Clarke" ... >> ISA 3.0B has xscvdpspn leaving its result in word 1 of the target > register, >> and mffprwz expecting its input to come from word 0 of the source > register. >> This sequence fails with QEMU, as a shift is required between > those >>> two >> instructions. However, the hardware splats the result to both > word 0 > and >> word 1 of its output register, so the shift is not necessary. >> Expect a future revision of the ISA to specify this behavior. >> > > Hmmm... Isn't this a gcc bug (using undocumented hardware feature), >>> given > everything you said here? The key here is "expect a future revision of the ISA to specify this >>> behavior". It's clearly within IBM's purview to adjust the specification to > document >>> a previously undocumented hardware feature. >>> >>> By no means, yes, the key is in ISA documentation. But, the impression > that >>> full original commit message conveys is that the main reason for change > is >>> gcc behavior. If we accepted in general that gcc behavior determines > QEMU >>> behavior, I am afraid we would be on a very slippery slope - therefore I >>> think the commit message (and possible code comment) should, in my > opinion, >>> mention ISA docs as the central reason for change. Paul, is there any >>> tentative release date of the new ISA specification? >> >> It's not really a question of gcc behaviour, it's a question of actual >> cpu behaviour versus ISA document. Which one qemu should follow is >> somewhat debatable, but it sounds here like the ISA will be updated to >> match the cpu, which weights it heavily in favour of mimicing the >> actual cpu. >> > > This sounds right to me. Thanks for the reviews and great discussion. While not yet part of a published version of the ISA, I did find the behavior documented in the User's Manuals for the POWER8 and POWER9 processors: https://www-355.ibm.com/systems/power/openpower/ "Public Documents" - "POWER9 Processor User's Manual" - "POWER8 Processor User's Manual for the SCM" POWER9 Processor User's Manual 4. Power Architecture Compliance 4.3 Floating-Point Processor (FP, VMX, and VSX) 4.3.7 Floating-Point Invalid Forms and Undefined Conditions POWER8 Processor User's Manual for the Single-Chip Module 3. Power Architecture Compliance 3.2 Floating-Point Processor (FP, VMX, and VSX) 3.2.9 Floating-Point Invalid Forms and Undefined Conditions In a bullet: - VSX scalar convert from double-precision to single-precision (xscvdpsp, xscvdpspn). VSR[32:63] is set to VSR[0:31]. I have not confirmed when the new revision of the ISA will be published, but it's on somebody's "to do" list. I will respin the patch as 3 independent patches, and include a reference to the above documents for the change under discussion here. (The other two changes may take a bit more time, because like David, I find the FPU emulation code cryptic. :-/ ) These issues were found while running Glibc's test suite for "math", and there are still a *LOT* of QEMU-only FAILs, so I may be back again with suggested fixes or questions. :-) PC
