[PATCH] target/hppa: correct size bit parity for fmpyadd

Gabriel Brookman posted 1 patch 1 month ago
Patches applied successfully (tree, apply log)
git fetch https://github.com/patchew-project/qemu tags/patchew/20251009-hppa-correct-fmpyadd-size-bit-decoding-v1-1-f63bb6c3290c@gmail.com
Maintainers: Richard Henderson <richard.henderson@linaro.org>, Helge Deller <deller@gmx.de>
target/hppa/insns.decode | 8 ++++----
1 file changed, 4 insertions(+), 4 deletions(-)
[PATCH] target/hppa: correct size bit parity for fmpyadd
Posted by Gabriel Brookman 1 month ago
For the fmpyadd instruction on the hppa architecture, there is a bit
used to specify whether the instruction is operating on a 32 bit or 64
bit floating point register. For most instructions, such a bit is 0 when
operating on the smaller register and 1 when operating on the larger
register. However, according to page 6-57 of the PA-RISC 1.1 Architecture
and Instruction Set Reference Manual, this convention is reversed for the
fmpyadd instruction specifically, meaning the bit is 1 for operations on
32 bit registers and 0 for 64 bit registers. Previously, QEMU decoded
this operation as operating on the other size of register, leading to
bugs when translating the fmpyadd instruction. This patch fixes that
issue.

Reported-by: Andreas Hüttel <andreas.huettel@ur.de>
Signed-off-by: Gabriel Brookman <brookmangabriel@gmail.com>
Resolves: https://gitlab.com/qemu-project/qemu/-/issues/3096
---
Hi all,

This patch fixes the decoding of the fmpyadd instruction on the hppa
target, which uses an inverted bit convention to select between
32-bit and 64-bit floating-point registers. The issue was reported by
Andreas Hüttel after observing incorrect behavior when running real
binaries under that target. He kindly submitted a minimal reproducer
which I was able to use to debug the issue. I used this reproducer
to verify correct operation after my fix.

Thanks,
Gabriel

Reported-by: Andreas Hüttel
---
 target/hppa/insns.decode | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/target/hppa/insns.decode b/target/hppa/insns.decode
index 4eaac750ea..13c6a55bf2 100644
--- a/target/hppa/insns.decode
+++ b/target/hppa/insns.decode
@@ -365,10 +365,10 @@ fstd            011100 ..... ..... .. ............1.    @ldstim11
 &mpyadd         rm1 rm2 ta ra tm
 @mpyadd         ...... rm1:5 rm2:5 ta:5 ra:5 . tm:5     &mpyadd
 
-fmpyadd_f       000110 ..... ..... ..... ..... 0 .....  @mpyadd
-fmpyadd_d       000110 ..... ..... ..... ..... 1 .....  @mpyadd
-fmpysub_f       100110 ..... ..... ..... ..... 0 .....  @mpyadd
-fmpysub_d       100110 ..... ..... ..... ..... 1 .....  @mpyadd
+fmpyadd_f       000110 ..... ..... ..... ..... 1 .....  @mpyadd
+fmpyadd_d       000110 ..... ..... ..... ..... 0 .....  @mpyadd
+fmpysub_f       100110 ..... ..... ..... ..... 1 .....  @mpyadd
+fmpysub_d       100110 ..... ..... ..... ..... 0 .....  @mpyadd
 
 ####
 # Conditional Branches

---
base-commit: 94474a7733a57365d5a27efc28c05462e90e8944
change-id: 20251009-hppa-correct-fmpyadd-size-bit-decoding-059501a0ae49

Best regards,
-- 
Gabriel Brookman <brookmangabriel@gmail.com>


Re: [PATCH] target/hppa: correct size bit parity for fmpyadd
Posted by Michael Tokarev 4 weeks ago
On 10/9/25 23:51, Gabriel Brookman wrote:
> For the fmpyadd instruction on the hppa architecture, there is a bit
> used to specify whether the instruction is operating on a 32 bit or 64
> bit floating point register. For most instructions, such a bit is 0 when
> operating on the smaller register and 1 when operating on the larger
> register. However, according to page 6-57 of the PA-RISC 1.1 Architecture
> and Instruction Set Reference Manual, this convention is reversed for the
> fmpyadd instruction specifically, meaning the bit is 1 for operations on
> 32 bit registers and 0 for 64 bit registers. Previously, QEMU decoded
> this operation as operating on the other size of register, leading to
> bugs when translating the fmpyadd instruction. This patch fixes that
> issue.
> 
> Reported-by: Andreas Hüttel <andreas.huettel@ur.de>
> Signed-off-by: Gabriel Brookman <brookmangabriel@gmail.com>
> Resolves: https://gitlab.com/qemu-project/qemu/-/issues/3096
> ---
> Hi all,
> 
> This patch fixes the decoding of the fmpyadd instruction on the hppa
> target, which uses an inverted bit convention to select between
> 32-bit and 64-bit floating-point registers. The issue was reported by
> Andreas Hüttel after observing incorrect behavior when running real
> binaries under that target. He kindly submitted a minimal reproducer
> which I was able to use to debug the issue. I used this reproducer
> to verify correct operation after my fix.
> 
> Thanks,
> Gabriel
> 
> Reported-by: Andreas Hüttel
> ---
>   target/hppa/insns.decode | 8 ++++----
>   1 file changed, 4 insertions(+), 4 deletions(-)

I'm picking this up for qemu-stable (10.0 & 10.1).
Please let me know if I shouldn't.

Thanks,

/mjt

> diff --git a/target/hppa/insns.decode b/target/hppa/insns.decode
> index 4eaac750ea..13c6a55bf2 100644
> --- a/target/hppa/insns.decode
> +++ b/target/hppa/insns.decode
> @@ -365,10 +365,10 @@ fstd            011100 ..... ..... .. ............1.    @ldstim11
>   &mpyadd         rm1 rm2 ta ra tm
>   @mpyadd         ...... rm1:5 rm2:5 ta:5 ra:5 . tm:5     &mpyadd
>   
> -fmpyadd_f       000110 ..... ..... ..... ..... 0 .....  @mpyadd
> -fmpyadd_d       000110 ..... ..... ..... ..... 1 .....  @mpyadd
> -fmpysub_f       100110 ..... ..... ..... ..... 0 .....  @mpyadd
> -fmpysub_d       100110 ..... ..... ..... ..... 1 .....  @mpyadd
> +fmpyadd_f       000110 ..... ..... ..... ..... 1 .....  @mpyadd
> +fmpyadd_d       000110 ..... ..... ..... ..... 0 .....  @mpyadd
> +fmpysub_f       100110 ..... ..... ..... ..... 1 .....  @mpyadd
> +fmpysub_d       100110 ..... ..... ..... ..... 0 .....  @mpyadd
>   
>   ####
>   # Conditional Branches
> 
> ---
> base-commit: 94474a7733a57365d5a27efc28c05462e90e8944
> change-id: 20251009-hppa-correct-fmpyadd-size-bit-decoding-059501a0ae49
> 
> Best regards,


Re: [PATCH] target/hppa: correct size bit parity for fmpyadd
Posted by Helge Deller 4 weeks ago
On 10/17/25 07:55, Michael Tokarev wrote:
> On 10/9/25 23:51, Gabriel Brookman wrote:
>> For the fmpyadd instruction on the hppa architecture, there is a bit
>> used to specify whether the instruction is operating on a 32 bit or 64
>> bit floating point register. For most instructions, such a bit is 0 when
>> operating on the smaller register and 1 when operating on the larger
>> register. However, according to page 6-57 of the PA-RISC 1.1 Architecture
>> and Instruction Set Reference Manual, this convention is reversed for the
>> fmpyadd instruction specifically, meaning the bit is 1 for operations on
>> 32 bit registers and 0 for 64 bit registers. Previously, QEMU decoded
>> this operation as operating on the other size of register, leading to
>> bugs when translating the fmpyadd instruction. This patch fixes that
>> issue.
>>
>> Reported-by: Andreas Hüttel <andreas.huettel@ur.de>
>> Signed-off-by: Gabriel Brookman <brookmangabriel@gmail.com>
>> Resolves: https://gitlab.com/qemu-project/qemu/-/issues/3096
>> ---
>> Hi all,
>>
>> This patch fixes the decoding of the fmpyadd instruction on the hppa
>> target, which uses an inverted bit convention to select between
>> 32-bit and 64-bit floating-point registers. The issue was reported by
>> Andreas Hüttel after observing incorrect behavior when running real
>> binaries under that target. He kindly submitted a minimal reproducer
>> which I was able to use to debug the issue. I used this reproducer
>> to verify correct operation after my fix.
>>
>> Thanks,
>> Gabriel
>>
>> Reported-by: Andreas Hüttel
>> ---
>>   target/hppa/insns.decode | 8 ++++----
>>   1 file changed, 4 insertions(+), 4 deletions(-)
> 
> I'm picking this up for qemu-stable (10.0 & 10.1).
Yes, please.

Helge
Re: [PATCH] target/hppa: correct size bit parity for fmpyadd
Posted by Philippe Mathieu-Daudé 4 weeks, 1 day ago
On 9/10/25 22:51, Gabriel Brookman wrote:
> For the fmpyadd instruction on the hppa architecture, there is a bit
> used to specify whether the instruction is operating on a 32 bit or 64
> bit floating point register. For most instructions, such a bit is 0 when
> operating on the smaller register and 1 when operating on the larger
> register. However, according to page 6-57 of the PA-RISC 1.1 Architecture
> and Instruction Set Reference Manual, this convention is reversed for the
> fmpyadd instruction specifically, meaning the bit is 1 for operations on
> 32 bit registers and 0 for 64 bit registers. Previously, QEMU decoded
> this operation as operating on the other size of register, leading to
> bugs when translating the fmpyadd instruction. This patch fixes that
> issue.
> 
> Reported-by: Andreas Hüttel <andreas.huettel@ur.de>
> Signed-off-by: Gabriel Brookman <brookmangabriel@gmail.com>
> Resolves: https://gitlab.com/qemu-project/qemu/-/issues/3096
> ---
> Hi all,
> 
> This patch fixes the decoding of the fmpyadd instruction on the hppa
> target, which uses an inverted bit convention to select between
> 32-bit and 64-bit floating-point registers. The issue was reported by
> Andreas Hüttel after observing incorrect behavior when running real
> binaries under that target. He kindly submitted a minimal reproducer
> which I was able to use to debug the issue. I used this reproducer
> to verify correct operation after my fix.
> 
> Thanks,
> Gabriel
> 
> Reported-by: Andreas Hüttel
> ---
>   target/hppa/insns.decode | 8 ++++----
>   1 file changed, 4 insertions(+), 4 deletions(-)

Patch queued (including doc refs Andreas added).

Re: [EXT] Re: [PATCH] target/hppa: correct size bit parity for fmpyadd
Posted by Andreas K. Huettel 4 weeks, 1 day ago
Am Donnerstag, 16. Oktober 2025, 10:49:56 Mitteleuropäische Sommerzeit schrieb Philippe Mathieu-Daudé:
> On 9/10/25 22:51, Gabriel Brookman wrote:
> > For the fmpyadd instruction on the hppa architecture, there is a bit
> > used to specify whether the instruction is operating on a 32 bit or 64
> > bit floating point register. For most instructions, such a bit is 0 when
> > operating on the smaller register and 1 when operating on the larger
> > register. However, according to page 6-57 of the PA-RISC 1.1 Architecture
> > and Instruction Set Reference Manual, this convention is reversed for the
> > fmpyadd instruction specifically, meaning the bit is 1 for operations on
> > 32 bit registers and 0 for 64 bit registers. Previously, QEMU decoded
> > this operation as operating on the other size of register, leading to
> > bugs when translating the fmpyadd instruction. This patch fixes that
> > issue.
> > 
> > Reported-by: Andreas Hüttel <andreas.huettel@ur.de>
> > Signed-off-by: Gabriel Brookman <brookmangabriel@gmail.com>
> > Resolves: https://gitlab.com/qemu-project/qemu/-/issues/3096
> > ---
> > Hi all,
> > 
> > This patch fixes the decoding of the fmpyadd instruction on the hppa
> > target, which uses an inverted bit convention to select between
> > 32-bit and 64-bit floating-point registers. The issue was reported by
> > Andreas Hüttel after observing incorrect behavior when running real
> > binaries under that target. He kindly submitted a minimal reproducer
> > which I was able to use to debug the issue. I used this reproducer
> > to verify correct operation after my fix.
> > 
> > Thanks,
> > Gabriel
> > 
> > Reported-by: Andreas Hüttel
> > ---
> >   target/hppa/insns.decode | 8 ++++----
> >   1 file changed, 4 insertions(+), 4 deletions(-)
> 
> Patch queued (including doc refs Andreas added).
> 

Thank you all! 

Will eventually help a lot here
https://www.gentoo.org/downloads/#hppa
(which is right now still blocked by an unrelated python bug)


-- 
Andreas K. Hüttel
dilfridge@gentoo.org
Gentoo Linux developer
(council, toolchain, base-system, perl, libreoffice)

-- 
PD Dr. Andreas K. Huettel
Institute for Experimental and Applied Physics
University of Regensburg
93040 Regensburg
Germany

tel. +49 151 241 67748 (mobile)
tel. +49 941 943 1618 (office)
e-mail andreas.huettel@ur.de
https://www.akhuettel.de/
https://www.akhuettel.de/group/
Re: [PATCH] target/hppa: correct size bit parity for fmpyadd
Posted by Richard Henderson 1 month ago
On 10/9/25 13:51, Gabriel Brookman wrote:
> For the fmpyadd instruction on the hppa architecture, there is a bit
> used to specify whether the instruction is operating on a 32 bit or 64
> bit floating point register. For most instructions, such a bit is 0 when
> operating on the smaller register and 1 when operating on the larger
> register. However, according to page 6-57 of the PA-RISC 1.1 Architecture
> and Instruction Set Reference Manual, 

Annoyingly, page 6-57 doesn't actually mention the interpretation of the f bit at all. 
However, I can see how the H specifier is processed in gas, and the confirmation of glibc 
fixes is nice.

Reviewed-by: Richard Henderson <richard.henderson@linaro.org>


r~
Re: [PATCH] target/hppa: correct size bit parity for fmpyadd
Posted by Helge Deller 1 month ago
On 10/9/25 23:23, Richard Henderson wrote:
> On 10/9/25 13:51, Gabriel Brookman wrote:
>> For the fmpyadd instruction on the hppa architecture, there is a bit
>> used to specify whether the instruction is operating on a 32 bit or 64
>> bit floating point register. For most instructions, such a bit is 0 when
>> operating on the smaller register and 1 when operating on the larger
>> register. However, according to page 6-57 of the PA-RISC 1.1 Architecture
>> and Instruction Set Reference Manual, 
> 
> Annoyingly, page 6-57 doesn't actually mention the interpretation of the f bit at all. However, I can see how the H specifier is processed in gas, and the confirmation of glibc fixes is nice.
> 
> Reviewed-by: Richard Henderson <richard.henderson@linaro.org>

Acked-by: Helge Deller <deller@gmx.de>

Nice catch!
Thanks!
Helge
Re: [EXT] Re: [PATCH] target/hppa: correct size bit parity for fmpyadd
Posted by Andreas K. Huettel 1 month ago
Am Donnerstag, 9. Oktober 2025, 23:23:33 Mitteleuropäische Sommerzeit schrieb Richard Henderson:
> On 10/9/25 13:51, Gabriel Brookman wrote:
> > For the fmpyadd instruction on the hppa architecture, there is a bit
> > used to specify whether the instruction is operating on a 32 bit or 64
> > bit floating point register. For most instructions, such a bit is 0 when
> > operating on the smaller register and 1 when operating on the larger
> > register. However, according to page 6-57 of the PA-RISC 1.1 Architecture
> > and Instruction Set Reference Manual, 
> 
> Annoyingly, page 6-57 doesn't actually mention the interpretation of the f bit at all. 
> However, I can see how the H specifier is processed in gas, and the confirmation of glibc 
> fixes is nice.

1) I can confirm the patch makes the test case run correctly.

2) The doc is probably page 6-18 (fig. 6-8) and 6-19 (table 6-16), where the 
f field for FMPYADD and FMPYSUB is documented. 


> 
> Reviewed-by: Richard Henderson <richard.henderson@linaro.org>
> 
> 
> r~
> 


-- 
PD Dr. Andreas K. Huettel
Institute for Experimental and Applied Physics
University of Regensburg
93040 Regensburg
Germany

tel. +49 151 241 67748 (mobile)
tel. +49 941 943 1618 (office)
fax +49 941 943 3196
e-mail andreas.huettel@ur.de
http://www.akhuettel.de/


-- 
Andreas K. Hüttel
dilfridge@gentoo.org
Gentoo Linux developer
(council, toolchain, base-system, perl, libreoffice)
Re: [EXT] Re: [PATCH] target/hppa: correct size bit parity for fmpyadd
Posted by Richard Henderson 1 month ago
On 10/9/25 14:30, Andreas K. Huettel wrote:
> 2) The doc is probably page 6-18 (fig. 6-8) and 6-19 (table 6-16), where the
> f field for FMPYADD and FMPYSUB is documented.

Yep, that's it.


r~