* [PATCH] i386: DPPS rounding fix
@ 2022-08-25 16:48 Paolo Bonzini
2022-08-25 17:08 ` Richard Henderson
0 siblings, 1 reply; 3+ messages in thread
From: Paolo Bonzini @ 2022-08-25 16:48 UTC (permalink / raw)
To: qemu-devel
The DPPS (Dot Product) instruction is defined to first sum pairs of
intermediate results, then sum those values to get the final result.
i.e. (A+B)+(C+D)
We incrementally sum the results, i.e. ((A+B)+C)+D, which can result
in incorrect rouding.
For consistency, also change the variable names to the ones used
in the Intel SDM and implement DPPD following the manual.
Based on a patch by Paul Brook <paul@nowt.org>.
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
target/i386/ops_sse.h | 67 ++++++++++++++++++++++---------------------
1 file changed, 35 insertions(+), 32 deletions(-)
diff --git a/target/i386/ops_sse.h b/target/i386/ops_sse.h
index 2524db4c25..b12b271fcd 100644
--- a/target/i386/ops_sse.h
+++ b/target/i386/ops_sse.h
@@ -1943,56 +1943,59 @@ SSE_HELPER_I(helper_pblendw, W, 8, FBLENDP)
void glue(helper_dpps, SUFFIX)(CPUX86State *env, Reg *d, Reg *s, uint32_t mask)
{
- float32 iresult = float32_zero;
+ float32 prod1, prod2, temp2, temp3, temp4;
+ /*
+ * We must evaluate (A+B)+(C+D), not ((A+B)+C)+D
+ * to correctly round the intermediate results
+ */
if (mask & (1 << 4)) {
- iresult = float32_add(iresult,
- float32_mul(d->ZMM_S(0), s->ZMM_S(0),
- &env->sse_status),
- &env->sse_status);
+ prod1 = float32_mul(d->ZMM_S(0), s->ZMM_S(0), &env->sse_status);
+ } else {
+ prod1 = float32_zero;
}
if (mask & (1 << 5)) {
- iresult = float32_add(iresult,
- float32_mul(d->ZMM_S(1), s->ZMM_S(1),
- &env->sse_status),
- &env->sse_status);
+ prod2 = float32_mul(d->ZMM_S(1), s->ZMM_S(1), &env->sse_status);
+ } else {
+ prod2 = float32_zero;
}
+ temp2 = float32_add(prod1, prod2, &env->sse_status);
if (mask & (1 << 6)) {
- iresult = float32_add(iresult,
- float32_mul(d->ZMM_S(2), s->ZMM_S(2),
- &env->sse_status),
- &env->sse_status);
+ prod1 = float32_mul(d->ZMM_S(2), s->ZMM_S(2), &env->sse_status);
+ } else {
+ prod1 = float32_zero;
}
if (mask & (1 << 7)) {
- iresult = float32_add(iresult,
- float32_mul(d->ZMM_S(3), s->ZMM_S(3),
- &env->sse_status),
- &env->sse_status);
+ prod2 = float32_mul(d->ZMM_S(3), s->ZMM_S(3), &env->sse_status);
+ } else {
+ prod2 = float32_zero;
}
- d->ZMM_S(0) = (mask & (1 << 0)) ? iresult : float32_zero;
- d->ZMM_S(1) = (mask & (1 << 1)) ? iresult : float32_zero;
- d->ZMM_S(2) = (mask & (1 << 2)) ? iresult : float32_zero;
- d->ZMM_S(3) = (mask & (1 << 3)) ? iresult : float32_zero;
+ temp3 = float32_add(prod1, prod2, &env->sse_status);
+ temp4 = float32_add(temp2, temp3, &env->sse_status);
+
+ d->ZMM_S(0) = (mask & (1 << 0)) ? temp4 : float32_zero;
+ d->ZMM_S(1) = (mask & (1 << 1)) ? temp4 : float32_zero;
+ d->ZMM_S(2) = (mask & (1 << 2)) ? temp4 : float32_zero;
+ d->ZMM_S(3) = (mask & (1 << 3)) ? temp4 : float32_zero;
}
void glue(helper_dppd, SUFFIX)(CPUX86State *env, Reg *d, Reg *s, uint32_t mask)
{
- float64 iresult = float64_zero;
+ float64 prod1, prod2, temp2;
if (mask & (1 << 4)) {
- iresult = float64_add(iresult,
- float64_mul(d->ZMM_D(0), s->ZMM_D(0),
- &env->sse_status),
- &env->sse_status);
+ prod1 = float64_mul(d->ZMM_D(0), s->ZMM_D(0), &env->sse_status);
+ } else {
+ prod1 = float64_zero;
}
if (mask & (1 << 5)) {
- iresult = float64_add(iresult,
- float64_mul(d->ZMM_D(1), s->ZMM_D(1),
- &env->sse_status),
- &env->sse_status);
+ prod2 = float64_mul(d->ZMM_D(1), s->ZMM_D(1), &env->sse_status);
+ } else {
+ prod2 = float64_zero;
}
- d->ZMM_D(0) = (mask & (1 << 0)) ? iresult : float64_zero;
- d->ZMM_D(1) = (mask & (1 << 1)) ? iresult : float64_zero;
+ temp2 = float64_add(prod1, prod2, &env->sse_status);
+ d->ZMM_D(0) = (mask & (1 << 0)) ? temp2 : float64_zero;
+ d->ZMM_D(1) = (mask & (1 << 1)) ? temp2 : float64_zero;
}
void glue(helper_mpsadbw, SUFFIX)(CPUX86State *env, Reg *d, Reg *s,
--
2.37.1
^ permalink raw reply related [flat|nested] 3+ messages in thread
* [PATCH] i386: DPPS rounding fix
2022-08-25 16:49 [PATCH] tests/i386: SSE tests Paolo Bonzini
@ 2022-08-25 16:49 ` Paolo Bonzini
0 siblings, 0 replies; 3+ messages in thread
From: Paolo Bonzini @ 2022-08-25 16:49 UTC (permalink / raw)
To: qemu-devel; +Cc: Paul Brook, Alex Bennée
The DPPS (Dot Product) instruction is defined to first sum pairs of
intermediate results, then sum those values to get the final result.
i.e. (A+B)+(C+D)
We incrementally sum the results, i.e. ((A+B)+C)+D, which can result
in incorrect rouding.
For consistency, also change the variable names to the ones used
in the Intel SDM and implement DPPD following the manual.
Based on a patch by Paul Brook <paul@nowt.org>.
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
target/i386/ops_sse.h | 67 ++++++++++++++++++++++---------------------
1 file changed, 35 insertions(+), 32 deletions(-)
diff --git a/target/i386/ops_sse.h b/target/i386/ops_sse.h
index 2524db4c25..b12b271fcd 100644
--- a/target/i386/ops_sse.h
+++ b/target/i386/ops_sse.h
@@ -1943,56 +1943,59 @@ SSE_HELPER_I(helper_pblendw, W, 8, FBLENDP)
void glue(helper_dpps, SUFFIX)(CPUX86State *env, Reg *d, Reg *s, uint32_t mask)
{
- float32 iresult = float32_zero;
+ float32 prod1, prod2, temp2, temp3, temp4;
+ /*
+ * We must evaluate (A+B)+(C+D), not ((A+B)+C)+D
+ * to correctly round the intermediate results
+ */
if (mask & (1 << 4)) {
- iresult = float32_add(iresult,
- float32_mul(d->ZMM_S(0), s->ZMM_S(0),
- &env->sse_status),
- &env->sse_status);
+ prod1 = float32_mul(d->ZMM_S(0), s->ZMM_S(0), &env->sse_status);
+ } else {
+ prod1 = float32_zero;
}
if (mask & (1 << 5)) {
- iresult = float32_add(iresult,
- float32_mul(d->ZMM_S(1), s->ZMM_S(1),
- &env->sse_status),
- &env->sse_status);
+ prod2 = float32_mul(d->ZMM_S(1), s->ZMM_S(1), &env->sse_status);
+ } else {
+ prod2 = float32_zero;
}
+ temp2 = float32_add(prod1, prod2, &env->sse_status);
if (mask & (1 << 6)) {
- iresult = float32_add(iresult,
- float32_mul(d->ZMM_S(2), s->ZMM_S(2),
- &env->sse_status),
- &env->sse_status);
+ prod1 = float32_mul(d->ZMM_S(2), s->ZMM_S(2), &env->sse_status);
+ } else {
+ prod1 = float32_zero;
}
if (mask & (1 << 7)) {
- iresult = float32_add(iresult,
- float32_mul(d->ZMM_S(3), s->ZMM_S(3),
- &env->sse_status),
- &env->sse_status);
+ prod2 = float32_mul(d->ZMM_S(3), s->ZMM_S(3), &env->sse_status);
+ } else {
+ prod2 = float32_zero;
}
- d->ZMM_S(0) = (mask & (1 << 0)) ? iresult : float32_zero;
- d->ZMM_S(1) = (mask & (1 << 1)) ? iresult : float32_zero;
- d->ZMM_S(2) = (mask & (1 << 2)) ? iresult : float32_zero;
- d->ZMM_S(3) = (mask & (1 << 3)) ? iresult : float32_zero;
+ temp3 = float32_add(prod1, prod2, &env->sse_status);
+ temp4 = float32_add(temp2, temp3, &env->sse_status);
+
+ d->ZMM_S(0) = (mask & (1 << 0)) ? temp4 : float32_zero;
+ d->ZMM_S(1) = (mask & (1 << 1)) ? temp4 : float32_zero;
+ d->ZMM_S(2) = (mask & (1 << 2)) ? temp4 : float32_zero;
+ d->ZMM_S(3) = (mask & (1 << 3)) ? temp4 : float32_zero;
}
void glue(helper_dppd, SUFFIX)(CPUX86State *env, Reg *d, Reg *s, uint32_t mask)
{
- float64 iresult = float64_zero;
+ float64 prod1, prod2, temp2;
if (mask & (1 << 4)) {
- iresult = float64_add(iresult,
- float64_mul(d->ZMM_D(0), s->ZMM_D(0),
- &env->sse_status),
- &env->sse_status);
+ prod1 = float64_mul(d->ZMM_D(0), s->ZMM_D(0), &env->sse_status);
+ } else {
+ prod1 = float64_zero;
}
if (mask & (1 << 5)) {
- iresult = float64_add(iresult,
- float64_mul(d->ZMM_D(1), s->ZMM_D(1),
- &env->sse_status),
- &env->sse_status);
+ prod2 = float64_mul(d->ZMM_D(1), s->ZMM_D(1), &env->sse_status);
+ } else {
+ prod2 = float64_zero;
}
- d->ZMM_D(0) = (mask & (1 << 0)) ? iresult : float64_zero;
- d->ZMM_D(1) = (mask & (1 << 1)) ? iresult : float64_zero;
+ temp2 = float64_add(prod1, prod2, &env->sse_status);
+ d->ZMM_D(0) = (mask & (1 << 0)) ? temp2 : float64_zero;
+ d->ZMM_D(1) = (mask & (1 << 1)) ? temp2 : float64_zero;
}
void glue(helper_mpsadbw, SUFFIX)(CPUX86State *env, Reg *d, Reg *s,
--
2.37.1
^ permalink raw reply related [flat|nested] 3+ messages in thread
* Re: [PATCH] i386: DPPS rounding fix
2022-08-25 16:48 [PATCH] i386: DPPS rounding fix Paolo Bonzini
@ 2022-08-25 17:08 ` Richard Henderson
0 siblings, 0 replies; 3+ messages in thread
From: Richard Henderson @ 2022-08-25 17:08 UTC (permalink / raw)
To: Paolo Bonzini, qemu-devel
On 8/25/22 09:48, Paolo Bonzini wrote:
> The DPPS (Dot Product) instruction is defined to first sum pairs of
> intermediate results, then sum those values to get the final result.
> i.e. (A+B)+(C+D)
>
> We incrementally sum the results, i.e. ((A+B)+C)+D, which can result
> in incorrect rouding.
>
> For consistency, also change the variable names to the ones used
> in the Intel SDM and implement DPPD following the manual.
>
> Based on a patch by Paul Brook<paul@nowt.org>.
>
> Signed-off-by: Paolo Bonzini<pbonzini@redhat.com>
> ---
> target/i386/ops_sse.h | 67 ++++++++++++++++++++++---------------------
> 1 file changed, 35 insertions(+), 32 deletions(-)
Reviewed-by: Richard Henderson <richard.henderson@linaro.org>
r~
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2022-08-25 17:14 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2022-08-25 16:48 [PATCH] i386: DPPS rounding fix Paolo Bonzini
2022-08-25 17:08 ` Richard Henderson
-- strict thread matches above, loose matches on Subject: below --
2022-08-25 16:49 [PATCH] tests/i386: SSE tests Paolo Bonzini
2022-08-25 16:49 ` [PATCH] i386: DPPS rounding fix Paolo Bonzini
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).