* [Qemu-devel] [PATCH] fpu_helper.c: fix helper_fpscr_clrbit() function
@ 2018-06-17 15:53 John Arbuckle
2018-06-18 0:34 ` David Gibson
2018-06-18 11:30 ` Peter Maydell
0 siblings, 2 replies; 5+ messages in thread
From: John Arbuckle @ 2018-06-17 15:53 UTC (permalink / raw)
To: peter.maydell, qemu-devel, aurelien, david, agraf, qemu-ppc; +Cc: John Arbuckle
Fix the helper_fpscr_clrbit() function so it correctly
sets the FEX and VX bits.
Signed-off-by: John Arbuckle <programmingkidx@gmail.com>
---
target/ppc/fpu_helper.c | 57 +++++++++++++++++++++++++++++++++++++++++++++++++
1 file changed, 57 insertions(+)
diff --git a/target/ppc/fpu_helper.c b/target/ppc/fpu_helper.c
index d31a933cbb..7e697a11d0 100644
--- a/target/ppc/fpu_helper.c
+++ b/target/ppc/fpu_helper.c
@@ -325,6 +325,63 @@ void helper_fpscr_clrbit(CPUPPCState *env, uint32_t bit)
case FPSCR_RN:
fpscr_set_rounding_mode(env);
break;
+ case FPSCR_VXSNAN:
+ case FPSCR_VXISI:
+ case FPSCR_VXIDI:
+ case FPSCR_VXZDZ:
+ case FPSCR_VXIMZ:
+ case FPSCR_VXVC:
+ case FPSCR_VXSOFT:
+ case FPSCR_VXSQRT:
+ case FPSCR_VXCVI:
+ {
+ int vxsnan, vxisi, vxidi, vxzdz, vximz, vxvc, vxsoft, vxsqrt, vxcvi;
+ vxsnan = (env->fpscr >> (31 - FPSCR_VXSNAN)) & 1;
+ vxisi = (env->fpscr >> (31 - FPSCR_VXISI)) & 1;
+ vxidi = (env->fpscr >> (31 - FPSCR_VXIDI)) & 1;
+ vxzdz = (env->fpscr >> (31 - FPSCR_VXZDZ)) & 1;
+ vximz = (env->fpscr >> (31 - FPSCR_VXIMZ)) & 1;
+ vxvc = (env->fpscr >> (31 - FPSCR_VXVC)) & 1;
+ vxsoft = (env->fpscr >> (31 - FPSCR_VXSOFT)) & 1;
+ vxsqrt = (env->fpscr >> (31 - FPSCR_VXSQRT)) & 1;
+ vxcvi = (env->fpscr >> (31 - FPSCR_VXCVI)) & 1;
+ if (~(vxsnan & vxisi & vxidi & vxzdz & vximz & vxvc & vxsoft &
+ vxsqrt & vxcvi)) {
+ /* Set VX bit to zero */
+ env->fpscr = env->fpscr & ~(1 << FPSCR_VX);
+ }
+ }
+ break;
+ case FPSCR_VX:
+ case FPSCR_OX:
+ case FPSCR_UX:
+ case FPSCR_ZX:
+ case FPSCR_XX:
+ case FPSCR_VE:
+ case FPSCR_OE:
+ case FPSCR_UE:
+ case FPSCR_ZE:
+ case FPSCR_XE:
+ {
+ int vx, ox, ux, zx, xx, ve, oe, ue, ze, xe;
+ vx = (env->fpscr >> (31 - FPSCR_VX)) & 1;
+ ox = (env->fpscr >> (31 - FPSCR_OX)) & 1;
+ ux = (env->fpscr >> (31 - FPSCR_UX)) & 1;
+ zx = (env->fpscr >> (31 - FPSCR_ZX)) & 1;
+ xx = (env->fpscr >> (31 - FPSCR_XX)) & 1;
+ ve = (env->fpscr >> (31 - FPSCR_VE)) & 1;
+ oe = (env->fpscr >> (31 - FPSCR_OE)) & 1;
+ ue = (env->fpscr >> (31 - FPSCR_UE)) & 1;
+ ze = (env->fpscr >> (31 - FPSCR_ZE)) & 1;
+ xe = (env->fpscr >> (31 - FPSCR_XE)) & 1;
+ bool fex;
+ fex = (vx & ve) | (ox & oe) | (ux & ue) | (zx & ze) | (xx & xe);
+ unsigned int mask;
+ mask = (1 << FPSCR_FEX);
+ /* Set the FEX bit */
+ env->fpscr = (env->fpscr & ~mask) | (-fex & mask);
+ }
+ break;
default:
break;
}
--
2.14.3 (Apple Git-98)
^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [Qemu-devel] [PATCH] fpu_helper.c: fix helper_fpscr_clrbit() function
2018-06-17 15:53 [Qemu-devel] [PATCH] fpu_helper.c: fix helper_fpscr_clrbit() function John Arbuckle
@ 2018-06-18 0:34 ` David Gibson
2018-06-18 1:20 ` Programmingkid
2018-06-18 11:30 ` Peter Maydell
1 sibling, 1 reply; 5+ messages in thread
From: David Gibson @ 2018-06-18 0:34 UTC (permalink / raw)
To: John Arbuckle; +Cc: peter.maydell, qemu-devel, aurelien, agraf, qemu-ppc
[-- Attachment #1: Type: text/plain, Size: 3736 bytes --]
On Sun, Jun 17, 2018 at 11:53:09AM -0400, John Arbuckle wrote:
> Fix the helper_fpscr_clrbit() function so it correctly
> sets the FEX and VX bits.
This needs a lot more information in the commit message:
* What exactly was wrong with the previous setting of the FEX and VX
bits?
* Where can we find documentation which describes how they should be
set correctly?
This is information we need to properly review the patch.
>
> Signed-off-by: John Arbuckle <programmingkidx@gmail.com>
> ---
> target/ppc/fpu_helper.c | 57 +++++++++++++++++++++++++++++++++++++++++++++++++
> 1 file changed, 57 insertions(+)
>
> diff --git a/target/ppc/fpu_helper.c b/target/ppc/fpu_helper.c
> index d31a933cbb..7e697a11d0 100644
> --- a/target/ppc/fpu_helper.c
> +++ b/target/ppc/fpu_helper.c
> @@ -325,6 +325,63 @@ void helper_fpscr_clrbit(CPUPPCState *env, uint32_t bit)
> case FPSCR_RN:
> fpscr_set_rounding_mode(env);
> break;
> + case FPSCR_VXSNAN:
> + case FPSCR_VXISI:
> + case FPSCR_VXIDI:
> + case FPSCR_VXZDZ:
> + case FPSCR_VXIMZ:
> + case FPSCR_VXVC:
> + case FPSCR_VXSOFT:
> + case FPSCR_VXSQRT:
> + case FPSCR_VXCVI:
> + {
> + int vxsnan, vxisi, vxidi, vxzdz, vximz, vxvc, vxsoft, vxsqrt, vxcvi;
> + vxsnan = (env->fpscr >> (31 - FPSCR_VXSNAN)) & 1;
> + vxisi = (env->fpscr >> (31 - FPSCR_VXISI)) & 1;
> + vxidi = (env->fpscr >> (31 - FPSCR_VXIDI)) & 1;
> + vxzdz = (env->fpscr >> (31 - FPSCR_VXZDZ)) & 1;
> + vximz = (env->fpscr >> (31 - FPSCR_VXIMZ)) & 1;
> + vxvc = (env->fpscr >> (31 - FPSCR_VXVC)) & 1;
> + vxsoft = (env->fpscr >> (31 - FPSCR_VXSOFT)) & 1;
> + vxsqrt = (env->fpscr >> (31 - FPSCR_VXSQRT)) & 1;
> + vxcvi = (env->fpscr >> (31 - FPSCR_VXCVI)) & 1;
> + if (~(vxsnan & vxisi & vxidi & vxzdz & vximz & vxvc & vxsoft &
> + vxsqrt & vxcvi)) {
> + /* Set VX bit to zero */
> + env->fpscr = env->fpscr & ~(1 << FPSCR_VX);
> + }
> + }
> + break;
> + case FPSCR_VX:
> + case FPSCR_OX:
> + case FPSCR_UX:
> + case FPSCR_ZX:
> + case FPSCR_XX:
> + case FPSCR_VE:
> + case FPSCR_OE:
> + case FPSCR_UE:
> + case FPSCR_ZE:
> + case FPSCR_XE:
> + {
> + int vx, ox, ux, zx, xx, ve, oe, ue, ze, xe;
> + vx = (env->fpscr >> (31 - FPSCR_VX)) & 1;
> + ox = (env->fpscr >> (31 - FPSCR_OX)) & 1;
> + ux = (env->fpscr >> (31 - FPSCR_UX)) & 1;
> + zx = (env->fpscr >> (31 - FPSCR_ZX)) & 1;
> + xx = (env->fpscr >> (31 - FPSCR_XX)) & 1;
> + ve = (env->fpscr >> (31 - FPSCR_VE)) & 1;
> + oe = (env->fpscr >> (31 - FPSCR_OE)) & 1;
> + ue = (env->fpscr >> (31 - FPSCR_UE)) & 1;
> + ze = (env->fpscr >> (31 - FPSCR_ZE)) & 1;
> + xe = (env->fpscr >> (31 - FPSCR_XE)) & 1;
> + bool fex;
> + fex = (vx & ve) | (ox & oe) | (ux & ue) | (zx & ze) | (xx & xe);
> + unsigned int mask;
> + mask = (1 << FPSCR_FEX);
> + /* Set the FEX bit */
> + env->fpscr = (env->fpscr & ~mask) | (-fex & mask);
> + }
> + break;
> default:
> break;
> }
--
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
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [Qemu-devel] [PATCH] fpu_helper.c: fix helper_fpscr_clrbit() function
2018-06-18 0:34 ` David Gibson
@ 2018-06-18 1:20 ` Programmingkid
0 siblings, 0 replies; 5+ messages in thread
From: Programmingkid @ 2018-06-18 1:20 UTC (permalink / raw)
To: David Gibson; +Cc: peter.maydell, qemu-devel, aurelien, agraf, qemu-ppc
> On Jun 17, 2018, at 8:34 PM, David Gibson <david@gibson.dropbear.id.au> wrote:
>
> On Sun, Jun 17, 2018 at 11:53:09AM -0400, John Arbuckle wrote:
>> Fix the helper_fpscr_clrbit() function so it correctly
>> sets the FEX and VX bits.
>
> This needs a lot more information in the commit message:
>
> * What exactly was wrong with the previous setting of the FEX and VX
> bits?
Determining the value for the FEX bit is suppose to be done like this:
FEX = (VX & VE) | (OX & OE) | (UX & UE) | (ZX & ZE) | (XX & XE))
It is described as "the logical OR of all the floating-point exception bits
masked by their respective enable bits". It was not implemented correctly. The value of FEX would stay on even when all other bits were set to off.
The VX bit is described as "the logical OR of all of the invalid operation exceptions". This bit was also not implemented correctly.
> * Where can we find documentation which describes how they should be
> set correctly?
My main source of information is an IBM document called:
PowerPC Microprocessor Family:
The Programming Environments for 32-Bit Microprocessors
Page 62 is where the FPSCR information is located.
This is an older copy than the one I use but it is still very useful:
https://www.pdfdrive.net/powerpc-microprocessor-family-the-programming-environments-for-32-e3087633.html
I have an G3 and G5 iMac that I used to compare values with QEMU. This patch fixed all the problems I was having with these bits.
Please let me know if there is anything else I could provide to help.
>
> This is information we need to properly review the patch.
>
>>
>> Signed-off-by: John Arbuckle <programmingkidx@gmail.com>
>> ---
>> target/ppc/fpu_helper.c | 57 +++++++++++++++++++++++++++++++++++++++++++++++++
>> 1 file changed, 57 insertions(+)
>>
>> diff --git a/target/ppc/fpu_helper.c b/target/ppc/fpu_helper.c
>> index d31a933cbb..7e697a11d0 100644
>> --- a/target/ppc/fpu_helper.c
>> +++ b/target/ppc/fpu_helper.c
>> @@ -325,6 +325,63 @@ void helper_fpscr_clrbit(CPUPPCState *env, uint32_t bit)
>> case FPSCR_RN:
>> fpscr_set_rounding_mode(env);
>> break;
>> + case FPSCR_VXSNAN:
>> + case FPSCR_VXISI:
>> + case FPSCR_VXIDI:
>> + case FPSCR_VXZDZ:
>> + case FPSCR_VXIMZ:
>> + case FPSCR_VXVC:
>> + case FPSCR_VXSOFT:
>> + case FPSCR_VXSQRT:
>> + case FPSCR_VXCVI:
>> + {
>> + int vxsnan, vxisi, vxidi, vxzdz, vximz, vxvc, vxsoft, vxsqrt, vxcvi;
>> + vxsnan = (env->fpscr >> (31 - FPSCR_VXSNAN)) & 1;
>> + vxisi = (env->fpscr >> (31 - FPSCR_VXISI)) & 1;
>> + vxidi = (env->fpscr >> (31 - FPSCR_VXIDI)) & 1;
>> + vxzdz = (env->fpscr >> (31 - FPSCR_VXZDZ)) & 1;
>> + vximz = (env->fpscr >> (31 - FPSCR_VXIMZ)) & 1;
>> + vxvc = (env->fpscr >> (31 - FPSCR_VXVC)) & 1;
>> + vxsoft = (env->fpscr >> (31 - FPSCR_VXSOFT)) & 1;
>> + vxsqrt = (env->fpscr >> (31 - FPSCR_VXSQRT)) & 1;
>> + vxcvi = (env->fpscr >> (31 - FPSCR_VXCVI)) & 1;
>> + if (~(vxsnan & vxisi & vxidi & vxzdz & vximz & vxvc & vxsoft &
>> + vxsqrt & vxcvi)) {
>> + /* Set VX bit to zero */
>> + env->fpscr = env->fpscr & ~(1 << FPSCR_VX);
>> + }
>> + }
>> + break;
>> + case FPSCR_VX:
>> + case FPSCR_OX:
>> + case FPSCR_UX:
>> + case FPSCR_ZX:
>> + case FPSCR_XX:
>> + case FPSCR_VE:
>> + case FPSCR_OE:
>> + case FPSCR_UE:
>> + case FPSCR_ZE:
>> + case FPSCR_XE:
>> + {
>> + int vx, ox, ux, zx, xx, ve, oe, ue, ze, xe;
>> + vx = (env->fpscr >> (31 - FPSCR_VX)) & 1;
>> + ox = (env->fpscr >> (31 - FPSCR_OX)) & 1;
>> + ux = (env->fpscr >> (31 - FPSCR_UX)) & 1;
>> + zx = (env->fpscr >> (31 - FPSCR_ZX)) & 1;
>> + xx = (env->fpscr >> (31 - FPSCR_XX)) & 1;
>> + ve = (env->fpscr >> (31 - FPSCR_VE)) & 1;
>> + oe = (env->fpscr >> (31 - FPSCR_OE)) & 1;
>> + ue = (env->fpscr >> (31 - FPSCR_UE)) & 1;
>> + ze = (env->fpscr >> (31 - FPSCR_ZE)) & 1;
>> + xe = (env->fpscr >> (31 - FPSCR_XE)) & 1;
>> + bool fex;
>> + fex = (vx & ve) | (ox & oe) | (ux & ue) | (zx & ze) | (xx & xe);
>> + unsigned int mask;
>> + mask = (1 << FPSCR_FEX);
>> + /* Set the FEX bit */
>> + env->fpscr = (env->fpscr & ~mask) | (-fex & mask);
>> + }
>> + break;
>> default:
>> break;
>> }
>
> --
> 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
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [Qemu-devel] [PATCH] fpu_helper.c: fix helper_fpscr_clrbit() function
2018-06-17 15:53 [Qemu-devel] [PATCH] fpu_helper.c: fix helper_fpscr_clrbit() function John Arbuckle
2018-06-18 0:34 ` David Gibson
@ 2018-06-18 11:30 ` Peter Maydell
2018-06-18 15:15 ` Programmingkid
1 sibling, 1 reply; 5+ messages in thread
From: Peter Maydell @ 2018-06-18 11:30 UTC (permalink / raw)
To: John Arbuckle
Cc: QEMU Developers, Aurelien Jarno, David Gibson, Alexander Graf, qemu-ppc
On 17 June 2018 at 16:53, John Arbuckle <programmingkidx@gmail.com> wrote:
> Fix the helper_fpscr_clrbit() function so it correctly
> sets the FEX and VX bits.
>
> Signed-off-by: John Arbuckle <programmingkidx@gmail.com>
> ---
> target/ppc/fpu_helper.c | 57 +++++++++++++++++++++++++++++++++++++++++++++++++
> 1 file changed, 57 insertions(+)
>
> diff --git a/target/ppc/fpu_helper.c b/target/ppc/fpu_helper.c
> index d31a933cbb..7e697a11d0 100644
> --- a/target/ppc/fpu_helper.c
> +++ b/target/ppc/fpu_helper.c
> @@ -325,6 +325,63 @@ void helper_fpscr_clrbit(CPUPPCState *env, uint32_t bit)
> case FPSCR_RN:
> fpscr_set_rounding_mode(env);
> break;
> + case FPSCR_VXSNAN:
> + case FPSCR_VXISI:
> + case FPSCR_VXIDI:
> + case FPSCR_VXZDZ:
> + case FPSCR_VXIMZ:
> + case FPSCR_VXVC:
> + case FPSCR_VXSOFT:
> + case FPSCR_VXSQRT:
> + case FPSCR_VXCVI:
> + {
> + int vxsnan, vxisi, vxidi, vxzdz, vximz, vxvc, vxsoft, vxsqrt, vxcvi;
> + vxsnan = (env->fpscr >> (31 - FPSCR_VXSNAN)) & 1;
> + vxisi = (env->fpscr >> (31 - FPSCR_VXISI)) & 1;
> + vxidi = (env->fpscr >> (31 - FPSCR_VXIDI)) & 1;
> + vxzdz = (env->fpscr >> (31 - FPSCR_VXZDZ)) & 1;
> + vximz = (env->fpscr >> (31 - FPSCR_VXIMZ)) & 1;
> + vxvc = (env->fpscr >> (31 - FPSCR_VXVC)) & 1;
> + vxsoft = (env->fpscr >> (31 - FPSCR_VXSOFT)) & 1;
> + vxsqrt = (env->fpscr >> (31 - FPSCR_VXSQRT)) & 1;
> + vxcvi = (env->fpscr >> (31 - FPSCR_VXCVI)) & 1;
> + if (~(vxsnan & vxisi & vxidi & vxzdz & vximz & vxvc & vxsoft &
> + vxsqrt & vxcvi)) {
If all you're doing is checking "are none of these bits set"
then you don't need to laboriously extract every bit and then
AND them together -- you can just use
if (!(env->fpscr & some_mask))
In fact the headers already define a macro which does that mask
on env->fpscr, so this can just be
if (!fpscr_ix) {
env->fpscr &= ~(1 << FPSCR_VX);
}
> + /* Set VX bit to zero */
> + env->fpscr = env->fpscr & ~(1 << FPSCR_VX);
> + }
> + }
> + break;
> + case FPSCR_VX:
> + case FPSCR_OX:
> + case FPSCR_UX:
> + case FPSCR_ZX:
> + case FPSCR_XX:
> + case FPSCR_VE:
> + case FPSCR_OE:
> + case FPSCR_UE:
> + case FPSCR_ZE:
> + case FPSCR_XE:
> + {
> + int vx, ox, ux, zx, xx, ve, oe, ue, ze, xe;
> + vx = (env->fpscr >> (31 - FPSCR_VX)) & 1;
> + ox = (env->fpscr >> (31 - FPSCR_OX)) & 1;
> + ux = (env->fpscr >> (31 - FPSCR_UX)) & 1;
> + zx = (env->fpscr >> (31 - FPSCR_ZX)) & 1;
> + xx = (env->fpscr >> (31 - FPSCR_XX)) & 1;
> + ve = (env->fpscr >> (31 - FPSCR_VE)) & 1;
> + oe = (env->fpscr >> (31 - FPSCR_OE)) & 1;
> + ue = (env->fpscr >> (31 - FPSCR_UE)) & 1;
> + ze = (env->fpscr >> (31 - FPSCR_ZE)) & 1;
> + xe = (env->fpscr >> (31 - FPSCR_XE)) & 1;
> + bool fex;
> + fex = (vx & ve) | (ox & oe) | (ux & ue) | (zx & ze) | (xx & xe);
> + unsigned int mask;
> + mask = (1 << FPSCR_FEX);
> + /* Set the FEX bit */
> + env->fpscr = (env->fpscr & ~mask) | (-fex & mask);
Similarly I think this is
if (!fpscr_eex) {
env->fpscr &= ~(1 << FPSCR_FEX);
}
> + }
> + break;
> default:
> break;
> }
> --
> 2.14.3 (Apple Git-98)
>
thanks
-- PMM
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [Qemu-devel] [PATCH] fpu_helper.c: fix helper_fpscr_clrbit() function
2018-06-18 11:30 ` Peter Maydell
@ 2018-06-18 15:15 ` Programmingkid
0 siblings, 0 replies; 5+ messages in thread
From: Programmingkid @ 2018-06-18 15:15 UTC (permalink / raw)
To: Peter Maydell
Cc: QEMU Developers, Aurelien Jarno, David Gibson, Alexander Graf, qemu-ppc
> On Jun 18, 2018, at 7:30 AM, Peter Maydell <peter.maydell@linaro.org> wrote:
>
> On 17 June 2018 at 16:53, John Arbuckle <programmingkidx@gmail.com> wrote:
>> Fix the helper_fpscr_clrbit() function so it correctly
>> sets the FEX and VX bits.
>>
>> Signed-off-by: John Arbuckle <programmingkidx@gmail.com>
>> ---
>> target/ppc/fpu_helper.c | 57 +++++++++++++++++++++++++++++++++++++++++++++++++
>> 1 file changed, 57 insertions(+)
>>
>> diff --git a/target/ppc/fpu_helper.c b/target/ppc/fpu_helper.c
>> index d31a933cbb..7e697a11d0 100644
>> --- a/target/ppc/fpu_helper.c
>> +++ b/target/ppc/fpu_helper.c
>> @@ -325,6 +325,63 @@ void helper_fpscr_clrbit(CPUPPCState *env, uint32_t bit)
>> case FPSCR_RN:
>> fpscr_set_rounding_mode(env);
>> break;
>> + case FPSCR_VXSNAN:
>> + case FPSCR_VXISI:
>> + case FPSCR_VXIDI:
>> + case FPSCR_VXZDZ:
>> + case FPSCR_VXIMZ:
>> + case FPSCR_VXVC:
>> + case FPSCR_VXSOFT:
>> + case FPSCR_VXSQRT:
>> + case FPSCR_VXCVI:
>> + {
>> + int vxsnan, vxisi, vxidi, vxzdz, vximz, vxvc, vxsoft, vxsqrt, vxcvi;
>> + vxsnan = (env->fpscr >> (31 - FPSCR_VXSNAN)) & 1;
>> + vxisi = (env->fpscr >> (31 - FPSCR_VXISI)) & 1;
>> + vxidi = (env->fpscr >> (31 - FPSCR_VXIDI)) & 1;
>> + vxzdz = (env->fpscr >> (31 - FPSCR_VXZDZ)) & 1;
>> + vximz = (env->fpscr >> (31 - FPSCR_VXIMZ)) & 1;
>> + vxvc = (env->fpscr >> (31 - FPSCR_VXVC)) & 1;
>> + vxsoft = (env->fpscr >> (31 - FPSCR_VXSOFT)) & 1;
>> + vxsqrt = (env->fpscr >> (31 - FPSCR_VXSQRT)) & 1;
>> + vxcvi = (env->fpscr >> (31 - FPSCR_VXCVI)) & 1;
>> + if (~(vxsnan & vxisi & vxidi & vxzdz & vximz & vxvc & vxsoft &
>> + vxsqrt & vxcvi)) {
>
> If all you're doing is checking "are none of these bits set"
> then you don't need to laboriously extract every bit and then
> AND them together -- you can just use
> if (!(env->fpscr & some_mask))
> In fact the headers already define a macro which does that mask
> on env->fpscr, so this can just be
>
> if (!fpscr_ix) {
> env->fpscr &= ~(1 << FPSCR_VX);
> }
>
>> + /* Set VX bit to zero */
>> + env->fpscr = env->fpscr & ~(1 << FPSCR_VX);
>> + }
>> + }
>> + break;
>> + case FPSCR_VX:
>> + case FPSCR_OX:
>> + case FPSCR_UX:
>> + case FPSCR_ZX:
>> + case FPSCR_XX:
>> + case FPSCR_VE:
>> + case FPSCR_OE:
>> + case FPSCR_UE:
>> + case FPSCR_ZE:
>> + case FPSCR_XE:
>> + {
>> + int vx, ox, ux, zx, xx, ve, oe, ue, ze, xe;
>> + vx = (env->fpscr >> (31 - FPSCR_VX)) & 1;
>> + ox = (env->fpscr >> (31 - FPSCR_OX)) & 1;
>> + ux = (env->fpscr >> (31 - FPSCR_UX)) & 1;
>> + zx = (env->fpscr >> (31 - FPSCR_ZX)) & 1;
>> + xx = (env->fpscr >> (31 - FPSCR_XX)) & 1;
>> + ve = (env->fpscr >> (31 - FPSCR_VE)) & 1;
>> + oe = (env->fpscr >> (31 - FPSCR_OE)) & 1;
>> + ue = (env->fpscr >> (31 - FPSCR_UE)) & 1;
>> + ze = (env->fpscr >> (31 - FPSCR_ZE)) & 1;
>> + xe = (env->fpscr >> (31 - FPSCR_XE)) & 1;
>> + bool fex;
>> + fex = (vx & ve) | (ox & oe) | (ux & ue) | (zx & ze) | (xx & xe);
>> + unsigned int mask;
>> + mask = (1 << FPSCR_FEX);
>> + /* Set the FEX bit */
>> + env->fpscr = (env->fpscr & ~mask) | (-fex & mask);
>
> Similarly I think this is
> if (!fpscr_eex) {
> env->fpscr &= ~(1 << FPSCR_FEX);
> }
>
>> + }
>> + break;
>> default:
>> break;
>> }
>> --
>> 2.14.3 (Apple Git-98)
>>
>
> thanks
> -- PMM
Your suggestions look great. Thank you. I will be sure to implement your suggestions and make a better commit message for version 2 of this patch.
^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2018-06-18 15:16 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-06-17 15:53 [Qemu-devel] [PATCH] fpu_helper.c: fix helper_fpscr_clrbit() function John Arbuckle
2018-06-18 0:34 ` David Gibson
2018-06-18 1:20 ` Programmingkid
2018-06-18 11:30 ` Peter Maydell
2018-06-18 15:15 ` Programmingkid
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.