* [Qemu-devel] [PATCH v2] target/ppc: rewrite f[n]m[add, sub] using float64_muladd
@ 2017-03-02 14:10 Nikunj A Dadhania
2017-03-03 1:18 ` David Gibson
2017-03-03 3:31 ` Richard Henderson
0 siblings, 2 replies; 6+ messages in thread
From: Nikunj A Dadhania @ 2017-03-02 14:10 UTC (permalink / raw)
To: qemu-ppc, david, rth; +Cc: qemu-devel, bharata, nikunj
Use the softfloat api for fused multiply-add.
Introduce routine to set the FPSCR flags VXNAN, VXIMZ nad VMISI.
Signed-off-by: Nikunj A Dadhania <nikunj@linux.vnet.ibm.com>
---
v1:
* Removed float64_madd_set_vxisi()
* Introduced float64_maddsub_update_excp() will updated required flags
* Changed layout of error handling
v0:
* Use MADD/MSUB_FLAGS as used by VSX instructions
* Introduce helper float64_madd_set_vxisi()
---
target/ppc/fpu_helper.c | 213 +++++++++++-------------------------------------
1 file changed, 46 insertions(+), 167 deletions(-)
diff --git a/target/ppc/fpu_helper.c b/target/ppc/fpu_helper.c
index 58aee64..0535ad0 100644
--- a/target/ppc/fpu_helper.c
+++ b/target/ppc/fpu_helper.c
@@ -743,178 +743,62 @@ uint64_t helper_frim(CPUPPCState *env, uint64_t arg)
return do_fri(env, arg, float_round_down);
}
-/* fmadd - fmadd. */
-uint64_t helper_fmadd(CPUPPCState *env, uint64_t arg1, uint64_t arg2,
- uint64_t arg3)
+static void float64_maddsub_update_excp(CPUPPCState *env, float64 arg1,
+ float64 arg2, float64 arg3,
+ unsigned int madd_flags)
{
- CPU_DoubleU farg1, farg2, farg3;
-
- farg1.ll = arg1;
- farg2.ll = arg2;
- farg3.ll = arg3;
-
- if (unlikely((float64_is_infinity(farg1.d) && float64_is_zero(farg2.d)) ||
- (float64_is_zero(farg1.d) && float64_is_infinity(farg2.d)))) {
- /* Multiplication of zero by infinity */
- farg1.ll = float_invalid_op_excp(env, POWERPC_EXCP_FP_VXIMZ, 1);
- } else {
- if (unlikely(float64_is_signaling_nan(farg1.d, &env->fp_status) ||
- float64_is_signaling_nan(farg2.d, &env->fp_status) ||
- float64_is_signaling_nan(farg3.d, &env->fp_status))) {
- /* sNaN operation */
- float_invalid_op_excp(env, POWERPC_EXCP_FP_VXSNAN, 1);
- }
- /* This is the way the PowerPC specification defines it */
- float128 ft0_128, ft1_128;
-
- ft0_128 = float64_to_float128(farg1.d, &env->fp_status);
- ft1_128 = float64_to_float128(farg2.d, &env->fp_status);
- ft0_128 = float128_mul(ft0_128, ft1_128, &env->fp_status);
- if (unlikely(float128_is_infinity(ft0_128) &&
- float64_is_infinity(farg3.d) &&
- float128_is_neg(ft0_128) != float64_is_neg(farg3.d))) {
- /* Magnitude subtraction of infinities */
- farg1.ll = float_invalid_op_excp(env, POWERPC_EXCP_FP_VXISI, 1);
- } else {
- ft1_128 = float64_to_float128(farg3.d, &env->fp_status);
- ft0_128 = float128_add(ft0_128, ft1_128, &env->fp_status);
- farg1.d = float128_to_float64(ft0_128, &env->fp_status);
- }
- }
-
- return farg1.ll;
-}
-
-/* fmsub - fmsub. */
-uint64_t helper_fmsub(CPUPPCState *env, uint64_t arg1, uint64_t arg2,
- uint64_t arg3)
-{
- CPU_DoubleU farg1, farg2, farg3;
-
- farg1.ll = arg1;
- farg2.ll = arg2;
- farg3.ll = arg3;
-
- if (unlikely((float64_is_infinity(farg1.d) && float64_is_zero(farg2.d)) ||
- (float64_is_zero(farg1.d) &&
- float64_is_infinity(farg2.d)))) {
+ if (unlikely((float64_is_infinity(arg1) && float64_is_zero(arg2)) ||
+ (float64_is_zero(arg1) && float64_is_infinity(arg2)))) {
/* Multiplication of zero by infinity */
- farg1.ll = float_invalid_op_excp(env, POWERPC_EXCP_FP_VXIMZ, 1);
- } else {
- if (unlikely(float64_is_signaling_nan(farg1.d, &env->fp_status) ||
- float64_is_signaling_nan(farg2.d, &env->fp_status) ||
- float64_is_signaling_nan(farg3.d, &env->fp_status))) {
- /* sNaN operation */
- float_invalid_op_excp(env, POWERPC_EXCP_FP_VXSNAN, 1);
+ arg1 = float_invalid_op_excp(env, POWERPC_EXCP_FP_VXIMZ, 1);
+ } else if (unlikely(float64_is_signaling_nan(arg1, &env->fp_status) ||
+ float64_is_signaling_nan(arg2, &env->fp_status) ||
+ float64_is_signaling_nan(arg3, &env->fp_status))) {
+ /* sNaN operation */
+ float_invalid_op_excp(env, POWERPC_EXCP_FP_VXSNAN, 1);
+ } else if ((float64_is_infinity(arg1) || float64_is_infinity(arg2)) &&
+ float64_is_infinity(arg3)) {
+ uint8_t aSign, bSign, cSign;
+
+ aSign = float64_is_neg(arg1);
+ bSign = float64_is_neg(arg2);
+ cSign = float64_is_neg(arg3);
+ if (madd_flags & float_muladd_negate_c) {
+ cSign ^= 1;
}
- /* This is the way the PowerPC specification defines it */
- float128 ft0_128, ft1_128;
-
- ft0_128 = float64_to_float128(farg1.d, &env->fp_status);
- ft1_128 = float64_to_float128(farg2.d, &env->fp_status);
- ft0_128 = float128_mul(ft0_128, ft1_128, &env->fp_status);
- if (unlikely(float128_is_infinity(ft0_128) &&
- float64_is_infinity(farg3.d) &&
- float128_is_neg(ft0_128) == float64_is_neg(farg3.d))) {
- /* Magnitude subtraction of infinities */
- farg1.ll = float_invalid_op_excp(env, POWERPC_EXCP_FP_VXISI, 1);
- } else {
- ft1_128 = float64_to_float128(farg3.d, &env->fp_status);
- ft0_128 = float128_sub(ft0_128, ft1_128, &env->fp_status);
- farg1.d = float128_to_float64(ft0_128, &env->fp_status);
+ if (aSign ^ bSign ^ cSign) {
+ float_invalid_op_excp(env, POWERPC_EXCP_FP_VXISI, 1);
}
}
- return farg1.ll;
}
-/* fnmadd - fnmadd. */
-uint64_t helper_fnmadd(CPUPPCState *env, uint64_t arg1, uint64_t arg2,
- uint64_t arg3)
-{
- CPU_DoubleU farg1, farg2, farg3;
-
- farg1.ll = arg1;
- farg2.ll = arg2;
- farg3.ll = arg3;
-
- if (unlikely((float64_is_infinity(farg1.d) && float64_is_zero(farg2.d)) ||
- (float64_is_zero(farg1.d) && float64_is_infinity(farg2.d)))) {
- /* Multiplication of zero by infinity */
- farg1.ll = float_invalid_op_excp(env, POWERPC_EXCP_FP_VXIMZ, 1);
- } else {
- if (unlikely(float64_is_signaling_nan(farg1.d, &env->fp_status) ||
- float64_is_signaling_nan(farg2.d, &env->fp_status) ||
- float64_is_signaling_nan(farg3.d, &env->fp_status))) {
- /* sNaN operation */
- float_invalid_op_excp(env, POWERPC_EXCP_FP_VXSNAN, 1);
- }
- /* This is the way the PowerPC specification defines it */
- float128 ft0_128, ft1_128;
-
- ft0_128 = float64_to_float128(farg1.d, &env->fp_status);
- ft1_128 = float64_to_float128(farg2.d, &env->fp_status);
- ft0_128 = float128_mul(ft0_128, ft1_128, &env->fp_status);
- if (unlikely(float128_is_infinity(ft0_128) &&
- float64_is_infinity(farg3.d) &&
- float128_is_neg(ft0_128) != float64_is_neg(farg3.d))) {
- /* Magnitude subtraction of infinities */
- farg1.ll = float_invalid_op_excp(env, POWERPC_EXCP_FP_VXISI, 1);
- } else {
- ft1_128 = float64_to_float128(farg3.d, &env->fp_status);
- ft0_128 = float128_add(ft0_128, ft1_128, &env->fp_status);
- farg1.d = float128_to_float64(ft0_128, &env->fp_status);
- }
- if (likely(!float64_is_any_nan(farg1.d))) {
- farg1.d = float64_chs(farg1.d);
- }
- }
- return farg1.ll;
+#define FPU_FMADD(op, madd_flags) \
+uint64_t helper_##op(CPUPPCState *env, uint64_t arg1, \
+ uint64_t arg2, uint64_t arg3) \
+{ \
+ uint32_t flags; \
+ float64 ret = float64_muladd(arg1, arg2, arg3, madd_flags, \
+ &env->fp_status); \
+ flags = get_float_exception_flags(&env->fp_status); \
+ if (flags) { \
+ if (flags & float_flag_invalid) { \
+ float64_maddsub_update_excp(env, arg1, arg2, arg3, \
+ madd_flags); \
+ } \
+ float_check_status(env); \
+ } \
+ return ret; \
}
-/* fnmsub - fnmsub. */
-uint64_t helper_fnmsub(CPUPPCState *env, uint64_t arg1, uint64_t arg2,
- uint64_t arg3)
-{
- CPU_DoubleU farg1, farg2, farg3;
-
- farg1.ll = arg1;
- farg2.ll = arg2;
- farg3.ll = arg3;
+#define MADD_FLGS 0
+#define MSUB_FLGS float_muladd_negate_c
+#define NMADD_FLGS float_muladd_negate_result
+#define NMSUB_FLGS (float_muladd_negate_c | float_muladd_negate_result)
- if (unlikely((float64_is_infinity(farg1.d) && float64_is_zero(farg2.d)) ||
- (float64_is_zero(farg1.d) &&
- float64_is_infinity(farg2.d)))) {
- /* Multiplication of zero by infinity */
- farg1.ll = float_invalid_op_excp(env, POWERPC_EXCP_FP_VXIMZ, 1);
- } else {
- if (unlikely(float64_is_signaling_nan(farg1.d, &env->fp_status) ||
- float64_is_signaling_nan(farg2.d, &env->fp_status) ||
- float64_is_signaling_nan(farg3.d, &env->fp_status))) {
- /* sNaN operation */
- float_invalid_op_excp(env, POWERPC_EXCP_FP_VXSNAN, 1);
- }
- /* This is the way the PowerPC specification defines it */
- float128 ft0_128, ft1_128;
-
- ft0_128 = float64_to_float128(farg1.d, &env->fp_status);
- ft1_128 = float64_to_float128(farg2.d, &env->fp_status);
- ft0_128 = float128_mul(ft0_128, ft1_128, &env->fp_status);
- if (unlikely(float128_is_infinity(ft0_128) &&
- float64_is_infinity(farg3.d) &&
- float128_is_neg(ft0_128) == float64_is_neg(farg3.d))) {
- /* Magnitude subtraction of infinities */
- farg1.ll = float_invalid_op_excp(env, POWERPC_EXCP_FP_VXISI, 1);
- } else {
- ft1_128 = float64_to_float128(farg3.d, &env->fp_status);
- ft0_128 = float128_sub(ft0_128, ft1_128, &env->fp_status);
- farg1.d = float128_to_float64(ft0_128, &env->fp_status);
- }
- if (likely(!float64_is_any_nan(farg1.d))) {
- farg1.d = float64_chs(farg1.d);
- }
- }
- return farg1.ll;
-}
+FPU_FMADD(fmadd, MADD_FLGS)
+FPU_FMADD(fnmadd, NMADD_FLGS)
+FPU_FMADD(fmsub, MSUB_FLGS)
+FPU_FMADD(fnmsub, NMSUB_FLGS)
/* frsp - frsp. */
uint64_t helper_frsp(CPUPPCState *env, uint64_t arg)
@@ -2384,11 +2268,6 @@ void helper_##op(CPUPPCState *env, uint32_t opcode) \
float_check_status(env); \
}
-#define MADD_FLGS 0
-#define MSUB_FLGS float_muladd_negate_c
-#define NMADD_FLGS float_muladd_negate_result
-#define NMSUB_FLGS (float_muladd_negate_c | float_muladd_negate_result)
-
VSX_MADD(xsmaddadp, 1, float64, VsrD(0), MADD_FLGS, 1, 1, 0)
VSX_MADD(xsmaddmdp, 1, float64, VsrD(0), MADD_FLGS, 0, 1, 0)
VSX_MADD(xsmsubadp, 1, float64, VsrD(0), MSUB_FLGS, 1, 1, 0)
--
2.7.4
^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [Qemu-devel] [PATCH v2] target/ppc: rewrite f[n]m[add, sub] using float64_muladd
2017-03-02 14:10 [Qemu-devel] [PATCH v2] target/ppc: rewrite f[n]m[add, sub] using float64_muladd Nikunj A Dadhania
@ 2017-03-03 1:18 ` David Gibson
2017-03-03 3:31 ` Richard Henderson
1 sibling, 0 replies; 6+ messages in thread
From: David Gibson @ 2017-03-03 1:18 UTC (permalink / raw)
To: Nikunj A Dadhania; +Cc: qemu-ppc, rth, qemu-devel, bharata
[-- Attachment #1: Type: text/plain, Size: 12653 bytes --]
On Thu, Mar 02, 2017 at 07:40:29PM +0530, Nikunj A Dadhania wrote:
> Use the softfloat api for fused multiply-add.
> Introduce routine to set the FPSCR flags VXNAN, VXIMZ nad VMISI.
>
> Signed-off-by: Nikunj A Dadhania <nikunj@linux.vnet.ibm.com>
Merged, thanks.
>
> ---
>
> v1:
> * Removed float64_madd_set_vxisi()
> * Introduced float64_maddsub_update_excp() will updated required flags
> * Changed layout of error handling
>
> v0:
> * Use MADD/MSUB_FLAGS as used by VSX instructions
> * Introduce helper float64_madd_set_vxisi()
> ---
> target/ppc/fpu_helper.c | 213 +++++++++++-------------------------------------
> 1 file changed, 46 insertions(+), 167 deletions(-)
>
> diff --git a/target/ppc/fpu_helper.c b/target/ppc/fpu_helper.c
> index 58aee64..0535ad0 100644
> --- a/target/ppc/fpu_helper.c
> +++ b/target/ppc/fpu_helper.c
> @@ -743,178 +743,62 @@ uint64_t helper_frim(CPUPPCState *env, uint64_t arg)
> return do_fri(env, arg, float_round_down);
> }
>
> -/* fmadd - fmadd. */
> -uint64_t helper_fmadd(CPUPPCState *env, uint64_t arg1, uint64_t arg2,
> - uint64_t arg3)
> +static void float64_maddsub_update_excp(CPUPPCState *env, float64 arg1,
> + float64 arg2, float64 arg3,
> + unsigned int madd_flags)
> {
> - CPU_DoubleU farg1, farg2, farg3;
> -
> - farg1.ll = arg1;
> - farg2.ll = arg2;
> - farg3.ll = arg3;
> -
> - if (unlikely((float64_is_infinity(farg1.d) && float64_is_zero(farg2.d)) ||
> - (float64_is_zero(farg1.d) && float64_is_infinity(farg2.d)))) {
> - /* Multiplication of zero by infinity */
> - farg1.ll = float_invalid_op_excp(env, POWERPC_EXCP_FP_VXIMZ, 1);
> - } else {
> - if (unlikely(float64_is_signaling_nan(farg1.d, &env->fp_status) ||
> - float64_is_signaling_nan(farg2.d, &env->fp_status) ||
> - float64_is_signaling_nan(farg3.d, &env->fp_status))) {
> - /* sNaN operation */
> - float_invalid_op_excp(env, POWERPC_EXCP_FP_VXSNAN, 1);
> - }
> - /* This is the way the PowerPC specification defines it */
> - float128 ft0_128, ft1_128;
> -
> - ft0_128 = float64_to_float128(farg1.d, &env->fp_status);
> - ft1_128 = float64_to_float128(farg2.d, &env->fp_status);
> - ft0_128 = float128_mul(ft0_128, ft1_128, &env->fp_status);
> - if (unlikely(float128_is_infinity(ft0_128) &&
> - float64_is_infinity(farg3.d) &&
> - float128_is_neg(ft0_128) != float64_is_neg(farg3.d))) {
> - /* Magnitude subtraction of infinities */
> - farg1.ll = float_invalid_op_excp(env, POWERPC_EXCP_FP_VXISI, 1);
> - } else {
> - ft1_128 = float64_to_float128(farg3.d, &env->fp_status);
> - ft0_128 = float128_add(ft0_128, ft1_128, &env->fp_status);
> - farg1.d = float128_to_float64(ft0_128, &env->fp_status);
> - }
> - }
> -
> - return farg1.ll;
> -}
> -
> -/* fmsub - fmsub. */
> -uint64_t helper_fmsub(CPUPPCState *env, uint64_t arg1, uint64_t arg2,
> - uint64_t arg3)
> -{
> - CPU_DoubleU farg1, farg2, farg3;
> -
> - farg1.ll = arg1;
> - farg2.ll = arg2;
> - farg3.ll = arg3;
> -
> - if (unlikely((float64_is_infinity(farg1.d) && float64_is_zero(farg2.d)) ||
> - (float64_is_zero(farg1.d) &&
> - float64_is_infinity(farg2.d)))) {
> + if (unlikely((float64_is_infinity(arg1) && float64_is_zero(arg2)) ||
> + (float64_is_zero(arg1) && float64_is_infinity(arg2)))) {
> /* Multiplication of zero by infinity */
> - farg1.ll = float_invalid_op_excp(env, POWERPC_EXCP_FP_VXIMZ, 1);
> - } else {
> - if (unlikely(float64_is_signaling_nan(farg1.d, &env->fp_status) ||
> - float64_is_signaling_nan(farg2.d, &env->fp_status) ||
> - float64_is_signaling_nan(farg3.d, &env->fp_status))) {
> - /* sNaN operation */
> - float_invalid_op_excp(env, POWERPC_EXCP_FP_VXSNAN, 1);
> + arg1 = float_invalid_op_excp(env, POWERPC_EXCP_FP_VXIMZ, 1);
> + } else if (unlikely(float64_is_signaling_nan(arg1, &env->fp_status) ||
> + float64_is_signaling_nan(arg2, &env->fp_status) ||
> + float64_is_signaling_nan(arg3, &env->fp_status))) {
> + /* sNaN operation */
> + float_invalid_op_excp(env, POWERPC_EXCP_FP_VXSNAN, 1);
> + } else if ((float64_is_infinity(arg1) || float64_is_infinity(arg2)) &&
> + float64_is_infinity(arg3)) {
> + uint8_t aSign, bSign, cSign;
> +
> + aSign = float64_is_neg(arg1);
> + bSign = float64_is_neg(arg2);
> + cSign = float64_is_neg(arg3);
> + if (madd_flags & float_muladd_negate_c) {
> + cSign ^= 1;
> }
> - /* This is the way the PowerPC specification defines it */
> - float128 ft0_128, ft1_128;
> -
> - ft0_128 = float64_to_float128(farg1.d, &env->fp_status);
> - ft1_128 = float64_to_float128(farg2.d, &env->fp_status);
> - ft0_128 = float128_mul(ft0_128, ft1_128, &env->fp_status);
> - if (unlikely(float128_is_infinity(ft0_128) &&
> - float64_is_infinity(farg3.d) &&
> - float128_is_neg(ft0_128) == float64_is_neg(farg3.d))) {
> - /* Magnitude subtraction of infinities */
> - farg1.ll = float_invalid_op_excp(env, POWERPC_EXCP_FP_VXISI, 1);
> - } else {
> - ft1_128 = float64_to_float128(farg3.d, &env->fp_status);
> - ft0_128 = float128_sub(ft0_128, ft1_128, &env->fp_status);
> - farg1.d = float128_to_float64(ft0_128, &env->fp_status);
> + if (aSign ^ bSign ^ cSign) {
> + float_invalid_op_excp(env, POWERPC_EXCP_FP_VXISI, 1);
> }
> }
> - return farg1.ll;
> }
>
> -/* fnmadd - fnmadd. */
> -uint64_t helper_fnmadd(CPUPPCState *env, uint64_t arg1, uint64_t arg2,
> - uint64_t arg3)
> -{
> - CPU_DoubleU farg1, farg2, farg3;
> -
> - farg1.ll = arg1;
> - farg2.ll = arg2;
> - farg3.ll = arg3;
> -
> - if (unlikely((float64_is_infinity(farg1.d) && float64_is_zero(farg2.d)) ||
> - (float64_is_zero(farg1.d) && float64_is_infinity(farg2.d)))) {
> - /* Multiplication of zero by infinity */
> - farg1.ll = float_invalid_op_excp(env, POWERPC_EXCP_FP_VXIMZ, 1);
> - } else {
> - if (unlikely(float64_is_signaling_nan(farg1.d, &env->fp_status) ||
> - float64_is_signaling_nan(farg2.d, &env->fp_status) ||
> - float64_is_signaling_nan(farg3.d, &env->fp_status))) {
> - /* sNaN operation */
> - float_invalid_op_excp(env, POWERPC_EXCP_FP_VXSNAN, 1);
> - }
> - /* This is the way the PowerPC specification defines it */
> - float128 ft0_128, ft1_128;
> -
> - ft0_128 = float64_to_float128(farg1.d, &env->fp_status);
> - ft1_128 = float64_to_float128(farg2.d, &env->fp_status);
> - ft0_128 = float128_mul(ft0_128, ft1_128, &env->fp_status);
> - if (unlikely(float128_is_infinity(ft0_128) &&
> - float64_is_infinity(farg3.d) &&
> - float128_is_neg(ft0_128) != float64_is_neg(farg3.d))) {
> - /* Magnitude subtraction of infinities */
> - farg1.ll = float_invalid_op_excp(env, POWERPC_EXCP_FP_VXISI, 1);
> - } else {
> - ft1_128 = float64_to_float128(farg3.d, &env->fp_status);
> - ft0_128 = float128_add(ft0_128, ft1_128, &env->fp_status);
> - farg1.d = float128_to_float64(ft0_128, &env->fp_status);
> - }
> - if (likely(!float64_is_any_nan(farg1.d))) {
> - farg1.d = float64_chs(farg1.d);
> - }
> - }
> - return farg1.ll;
> +#define FPU_FMADD(op, madd_flags) \
> +uint64_t helper_##op(CPUPPCState *env, uint64_t arg1, \
> + uint64_t arg2, uint64_t arg3) \
> +{ \
> + uint32_t flags; \
> + float64 ret = float64_muladd(arg1, arg2, arg3, madd_flags, \
> + &env->fp_status); \
> + flags = get_float_exception_flags(&env->fp_status); \
> + if (flags) { \
> + if (flags & float_flag_invalid) { \
> + float64_maddsub_update_excp(env, arg1, arg2, arg3, \
> + madd_flags); \
> + } \
> + float_check_status(env); \
> + } \
> + return ret; \
> }
>
> -/* fnmsub - fnmsub. */
> -uint64_t helper_fnmsub(CPUPPCState *env, uint64_t arg1, uint64_t arg2,
> - uint64_t arg3)
> -{
> - CPU_DoubleU farg1, farg2, farg3;
> -
> - farg1.ll = arg1;
> - farg2.ll = arg2;
> - farg3.ll = arg3;
> +#define MADD_FLGS 0
> +#define MSUB_FLGS float_muladd_negate_c
> +#define NMADD_FLGS float_muladd_negate_result
> +#define NMSUB_FLGS (float_muladd_negate_c | float_muladd_negate_result)
>
> - if (unlikely((float64_is_infinity(farg1.d) && float64_is_zero(farg2.d)) ||
> - (float64_is_zero(farg1.d) &&
> - float64_is_infinity(farg2.d)))) {
> - /* Multiplication of zero by infinity */
> - farg1.ll = float_invalid_op_excp(env, POWERPC_EXCP_FP_VXIMZ, 1);
> - } else {
> - if (unlikely(float64_is_signaling_nan(farg1.d, &env->fp_status) ||
> - float64_is_signaling_nan(farg2.d, &env->fp_status) ||
> - float64_is_signaling_nan(farg3.d, &env->fp_status))) {
> - /* sNaN operation */
> - float_invalid_op_excp(env, POWERPC_EXCP_FP_VXSNAN, 1);
> - }
> - /* This is the way the PowerPC specification defines it */
> - float128 ft0_128, ft1_128;
> -
> - ft0_128 = float64_to_float128(farg1.d, &env->fp_status);
> - ft1_128 = float64_to_float128(farg2.d, &env->fp_status);
> - ft0_128 = float128_mul(ft0_128, ft1_128, &env->fp_status);
> - if (unlikely(float128_is_infinity(ft0_128) &&
> - float64_is_infinity(farg3.d) &&
> - float128_is_neg(ft0_128) == float64_is_neg(farg3.d))) {
> - /* Magnitude subtraction of infinities */
> - farg1.ll = float_invalid_op_excp(env, POWERPC_EXCP_FP_VXISI, 1);
> - } else {
> - ft1_128 = float64_to_float128(farg3.d, &env->fp_status);
> - ft0_128 = float128_sub(ft0_128, ft1_128, &env->fp_status);
> - farg1.d = float128_to_float64(ft0_128, &env->fp_status);
> - }
> - if (likely(!float64_is_any_nan(farg1.d))) {
> - farg1.d = float64_chs(farg1.d);
> - }
> - }
> - return farg1.ll;
> -}
> +FPU_FMADD(fmadd, MADD_FLGS)
> +FPU_FMADD(fnmadd, NMADD_FLGS)
> +FPU_FMADD(fmsub, MSUB_FLGS)
> +FPU_FMADD(fnmsub, NMSUB_FLGS)
>
> /* frsp - frsp. */
> uint64_t helper_frsp(CPUPPCState *env, uint64_t arg)
> @@ -2384,11 +2268,6 @@ void helper_##op(CPUPPCState *env, uint32_t opcode) \
> float_check_status(env); \
> }
>
> -#define MADD_FLGS 0
> -#define MSUB_FLGS float_muladd_negate_c
> -#define NMADD_FLGS float_muladd_negate_result
> -#define NMSUB_FLGS (float_muladd_negate_c | float_muladd_negate_result)
> -
> VSX_MADD(xsmaddadp, 1, float64, VsrD(0), MADD_FLGS, 1, 1, 0)
> VSX_MADD(xsmaddmdp, 1, float64, VsrD(0), MADD_FLGS, 0, 1, 0)
> VSX_MADD(xsmsubadp, 1, float64, VsrD(0), MSUB_FLGS, 1, 1, 0)
--
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: 819 bytes --]
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [Qemu-devel] [PATCH v2] target/ppc: rewrite f[n]m[add, sub] using float64_muladd
2017-03-02 14:10 [Qemu-devel] [PATCH v2] target/ppc: rewrite f[n]m[add, sub] using float64_muladd Nikunj A Dadhania
2017-03-03 1:18 ` David Gibson
@ 2017-03-03 3:31 ` Richard Henderson
2017-03-03 3:52 ` David Gibson
2017-03-03 4:01 ` Nikunj A Dadhania
1 sibling, 2 replies; 6+ messages in thread
From: Richard Henderson @ 2017-03-03 3:31 UTC (permalink / raw)
To: Nikunj A Dadhania, qemu-ppc, david; +Cc: qemu-devel, bharata
On 03/03/2017 01:10 AM, Nikunj A Dadhania wrote:
> +static void float64_maddsub_update_excp(CPUPPCState *env, float64 arg1,
> + float64 arg2, float64 arg3,
> + unsigned int madd_flags)
> {
> + if (unlikely((float64_is_infinity(arg1) && float64_is_zero(arg2)) ||
> + (float64_is_zero(arg1) && float64_is_infinity(arg2)))) {
> /* Multiplication of zero by infinity */
> + arg1 = float_invalid_op_excp(env, POWERPC_EXCP_FP_VXIMZ, 1);
> + } else if (unlikely(float64_is_signaling_nan(arg1, &env->fp_status) ||
> + float64_is_signaling_nan(arg2, &env->fp_status) ||
> + float64_is_signaling_nan(arg3, &env->fp_status))) {
> + /* sNaN operation */
> + float_invalid_op_excp(env, POWERPC_EXCP_FP_VXSNAN, 1);
Are you sure you shouldn't be testing for NaN first?
Do you really get VXIMZ if arg3 (the addend) is (S)NaN?
You should probably eliminate QNaN as well, before the other checks.
r~
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [Qemu-devel] [PATCH v2] target/ppc: rewrite f[n]m[add, sub] using float64_muladd
2017-03-03 3:31 ` Richard Henderson
@ 2017-03-03 3:52 ` David Gibson
2017-03-03 4:22 ` Nikunj A Dadhania
2017-03-03 4:01 ` Nikunj A Dadhania
1 sibling, 1 reply; 6+ messages in thread
From: David Gibson @ 2017-03-03 3:52 UTC (permalink / raw)
To: Richard Henderson; +Cc: Nikunj A Dadhania, qemu-ppc, qemu-devel, bharata
[-- Attachment #1: Type: text/plain, Size: 1511 bytes --]
On Fri, Mar 03, 2017 at 02:31:38PM +1100, Richard Henderson wrote:
> On 03/03/2017 01:10 AM, Nikunj A Dadhania wrote:
> > +static void float64_maddsub_update_excp(CPUPPCState *env, float64 arg1,
> > + float64 arg2, float64 arg3,
> > + unsigned int madd_flags)
> > {
> > + if (unlikely((float64_is_infinity(arg1) && float64_is_zero(arg2)) ||
> > + (float64_is_zero(arg1) && float64_is_infinity(arg2)))) {
> > /* Multiplication of zero by infinity */
> > + arg1 = float_invalid_op_excp(env, POWERPC_EXCP_FP_VXIMZ, 1);
> > + } else if (unlikely(float64_is_signaling_nan(arg1, &env->fp_status) ||
> > + float64_is_signaling_nan(arg2, &env->fp_status) ||
> > + float64_is_signaling_nan(arg3, &env->fp_status))) {
> > + /* sNaN operation */
> > + float_invalid_op_excp(env, POWERPC_EXCP_FP_VXSNAN, 1);
>
> Are you sure you shouldn't be testing for NaN first?
> Do you really get VXIMZ if arg3 (the addend) is (S)NaN?
>
> You should probably eliminate QNaN as well, before the other checks.
Oh.. I've already merged and pull requested this. I'm afraid any
further corrections will have to be done as followup patches.
--
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: 819 bytes --]
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [Qemu-devel] [PATCH v2] target/ppc: rewrite f[n]m[add, sub] using float64_muladd
2017-03-03 3:31 ` Richard Henderson
2017-03-03 3:52 ` David Gibson
@ 2017-03-03 4:01 ` Nikunj A Dadhania
1 sibling, 0 replies; 6+ messages in thread
From: Nikunj A Dadhania @ 2017-03-03 4:01 UTC (permalink / raw)
To: Richard Henderson, qemu-ppc, david; +Cc: qemu-devel, bharata
Richard Henderson <rth@twiddle.net> writes:
> On 03/03/2017 01:10 AM, Nikunj A Dadhania wrote:
>> +static void float64_maddsub_update_excp(CPUPPCState *env, float64 arg1,
>> + float64 arg2, float64 arg3,
>> + unsigned int madd_flags)
>> {
>> + if (unlikely((float64_is_infinity(arg1) && float64_is_zero(arg2)) ||
>> + (float64_is_zero(arg1) && float64_is_infinity(arg2)))) {
>> /* Multiplication of zero by infinity */
>> + arg1 = float_invalid_op_excp(env, POWERPC_EXCP_FP_VXIMZ, 1);
>> + } else if (unlikely(float64_is_signaling_nan(arg1, &env->fp_status) ||
>> + float64_is_signaling_nan(arg2, &env->fp_status) ||
>> + float64_is_signaling_nan(arg3, &env->fp_status))) {
>> + /* sNaN operation */
>> + float_invalid_op_excp(env, POWERPC_EXCP_FP_VXSNAN, 1);
>
> Are you sure you shouldn't be testing for NaN first?
I have tried to retain the old order of checking. While when I see
MultiplyAddDP(x,y,z) in ISA, following is the order:
* If x, y or z is an SNaN, vxsnan_flag is set to 1.
* If x is a Zero and y, is an Infinity or x is an Infinity and y is
an Zero, vximz_flag is set to 1.
* If the product of x and y is an Infinity and z is an Infinity of
the opposite sign, vxisi_flag is set to 1.
In that case, I need to get rid of nested if-else and make each of them
independent checks.
> Do you really get VXIMZ if arg3 (the addend) is (S)NaN?
Yes, with the above logic yes.
> You should probably eliminate QNaN as well, before the other checks.
MultiplyAddDP does not eliminate QNaN. But then, there is detailed spec
on what should be returned in various cases(Page 469 ISA 3.0)
Regards
Nikunj
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [Qemu-devel] [PATCH v2] target/ppc: rewrite f[n]m[add, sub] using float64_muladd
2017-03-03 3:52 ` David Gibson
@ 2017-03-03 4:22 ` Nikunj A Dadhania
0 siblings, 0 replies; 6+ messages in thread
From: Nikunj A Dadhania @ 2017-03-03 4:22 UTC (permalink / raw)
To: David Gibson, Richard Henderson; +Cc: qemu-ppc, qemu-devel, bharata
David Gibson <david@gibson.dropbear.id.au> writes:
> [ Unknown signature status ]
> On Fri, Mar 03, 2017 at 02:31:38PM +1100, Richard Henderson wrote:
>> On 03/03/2017 01:10 AM, Nikunj A Dadhania wrote:
>> > +static void float64_maddsub_update_excp(CPUPPCState *env, float64 arg1,
>> > + float64 arg2, float64 arg3,
>> > + unsigned int madd_flags)
>> > {
>> > + if (unlikely((float64_is_infinity(arg1) && float64_is_zero(arg2)) ||
>> > + (float64_is_zero(arg1) && float64_is_infinity(arg2)))) {
>> > /* Multiplication of zero by infinity */
>> > + arg1 = float_invalid_op_excp(env, POWERPC_EXCP_FP_VXIMZ, 1);
>> > + } else if (unlikely(float64_is_signaling_nan(arg1, &env->fp_status) ||
>> > + float64_is_signaling_nan(arg2, &env->fp_status) ||
>> > + float64_is_signaling_nan(arg3, &env->fp_status))) {
>> > + /* sNaN operation */
>> > + float_invalid_op_excp(env, POWERPC_EXCP_FP_VXSNAN, 1);
>>
>> Are you sure you shouldn't be testing for NaN first?
>> Do you really get VXIMZ if arg3 (the addend) is (S)NaN?
>>
>> You should probably eliminate QNaN as well, before the other checks.
>
> Oh.. I've already merged and pull requested this.
The behaviour has not changed as compared to previous version.
> I'm afraid any further corrections will have to be done as followup
> patches.
I will send a follow-up with fixing other multiply-add versions as well.
Regards
Nikunj
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2017-03-03 4:23 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-03-02 14:10 [Qemu-devel] [PATCH v2] target/ppc: rewrite f[n]m[add, sub] using float64_muladd Nikunj A Dadhania
2017-03-03 1:18 ` David Gibson
2017-03-03 3:31 ` Richard Henderson
2017-03-03 3:52 ` David Gibson
2017-03-03 4:22 ` Nikunj A Dadhania
2017-03-03 4:01 ` Nikunj A Dadhania
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.