[PATCH 0/7] target/alpha: Fix linux-user exception for CVTTQ

qemu-devel.nongnu.org archive mirror
 help / color / mirror / Atom feed

* [PATCH 0/7] target/alpha: Fix linux-user exception for CVTTQ
@ 2019-09-21  4:32 Richard Henderson
  2019-09-21  4:32 ` [PATCH 1/7] target/alpha: Use array for FPCR_DYN conversion Richard Henderson
                   ` (7 more replies)
  0 siblings, 8 replies; 17+ messages in thread
From: Richard Henderson @ 2019-09-21  4:32 UTC (permalink / raw)
  To: qemu-devel; +Cc: alex.bennee

Alex's new float_convs test dies with SIGFPE for alpha-linux-user.

This is fixed in patch 6, where I mask the exception similar to how
the kernel would, when passed through the software completion handler.

Patches 2 & 3 fix bugs that I noticed in the process; the rest are
simple cleanup, trying to make the code better or easier.


r~


Richard Henderson (7):
  target/alpha: Use array for FPCR_DYN conversion
  target/alpha: Fix SWCR_MAP_UMZ
  target/alpha: Fix SWCR_TRAP_ENABLE_MASK
  target/alpha: Handle SWCR_MAP_DMZ earlier
  target/alpha: Write to fpcr_flush_to_zero once
  target/alpha: Mask IOV exception with INV for user-only
  target/alpha: Tidy helper_fp_exc_raise_s

 target/alpha/fpu_helper.c | 15 +++------
 target/alpha/helper.c     | 68 +++++++++++++++++++--------------------
 2 files changed, 38 insertions(+), 45 deletions(-)

-- 
2.17.1



^ permalink raw reply	[flat|nested] 17+ messages in thread

* [PATCH 1/7] target/alpha: Use array for FPCR_DYN conversion
  2019-09-21  4:32 [PATCH 0/7] target/alpha: Fix linux-user exception for CVTTQ Richard Henderson
@ 2019-09-21  4:32 ` Richard Henderson
  2019-09-21  8:53   ` Philippe Mathieu-Daudé
  2019-09-23 16:23   ` Alex Bennée
  2019-09-21  4:32 ` [PATCH 2/7] target/alpha: Fix SWCR_MAP_UMZ Richard Henderson
                   ` (6 subsequent siblings)
  7 siblings, 2 replies; 17+ messages in thread
From: Richard Henderson @ 2019-09-21  4:32 UTC (permalink / raw)
  To: qemu-devel; +Cc: alex.bennee

This is a bit more straight-forward than using a switch statement.
No functional change.

Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
---
 target/alpha/helper.c | 24 ++++++++----------------
 1 file changed, 8 insertions(+), 16 deletions(-)

diff --git a/target/alpha/helper.c b/target/alpha/helper.c
index 19cda0a2db..6c1703682e 100644
--- a/target/alpha/helper.c
+++ b/target/alpha/helper.c
@@ -36,6 +36,13 @@ uint64_t cpu_alpha_load_fpcr(CPUAlphaState *env)
 
 void cpu_alpha_store_fpcr(CPUAlphaState *env, uint64_t val)
 {
+    static const uint8_t rm_map[] = {
+        [FPCR_DYN_NORMAL >> FPCR_DYN_SHIFT] = float_round_nearest_even,
+        [FPCR_DYN_CHOPPED >> FPCR_DYN_SHIFT] = float_round_to_zero,
+        [FPCR_DYN_MINUS >> FPCR_DYN_SHIFT] = float_round_down,
+        [FPCR_DYN_PLUS >> FPCR_DYN_SHIFT] = float_round_up,
+    };
+
     uint32_t fpcr = val >> 32;
     uint32_t t = 0;
 
@@ -48,22 +55,7 @@ void cpu_alpha_store_fpcr(CPUAlphaState *env, uint64_t val)
     env->fpcr = fpcr;
     env->fpcr_exc_enable = ~t & FPCR_STATUS_MASK;
 
-    switch (fpcr & FPCR_DYN_MASK) {
-    case FPCR_DYN_NORMAL:
-    default:
-        t = float_round_nearest_even;
-        break;
-    case FPCR_DYN_CHOPPED:
-        t = float_round_to_zero;
-        break;
-    case FPCR_DYN_MINUS:
-        t = float_round_down;
-        break;
-    case FPCR_DYN_PLUS:
-        t = float_round_up;
-        break;
-    }
-    env->fpcr_dyn_round = t;
+    env->fpcr_dyn_round = rm_map[(fpcr & FPCR_DYN_MASK) >> FPCR_DYN_SHIFT];
 
     env->fpcr_flush_to_zero = (fpcr & FPCR_UNFD) && (fpcr & FPCR_UNDZ);
     env->fp_status.flush_inputs_to_zero = (fpcr & FPCR_DNZ) != 0;
-- 
2.17.1



^ permalink raw reply related	[flat|nested] 17+ messages in thread

* Re: [PATCH 1/7] target/alpha: Use array for FPCR_DYN conversion
  2019-09-21  4:32 ` [PATCH 1/7] target/alpha: Use array for FPCR_DYN conversion Richard Henderson
@ 2019-09-21  8:53   ` Philippe Mathieu-Daudé
  2019-09-23 16:23   ` Alex Bennée
  1 sibling, 0 replies; 17+ messages in thread
From: Philippe Mathieu-Daudé @ 2019-09-21  8:53 UTC (permalink / raw)
  To: Richard Henderson, qemu-devel; +Cc: alex.bennee

On 9/21/19 6:32 AM, Richard Henderson wrote:
> This is a bit more straight-forward than using a switch statement.
> No functional change.
> 
> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>

Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com>

> ---
>  target/alpha/helper.c | 24 ++++++++----------------
>  1 file changed, 8 insertions(+), 16 deletions(-)
> 
> diff --git a/target/alpha/helper.c b/target/alpha/helper.c
> index 19cda0a2db..6c1703682e 100644
> --- a/target/alpha/helper.c
> +++ b/target/alpha/helper.c
> @@ -36,6 +36,13 @@ uint64_t cpu_alpha_load_fpcr(CPUAlphaState *env)
>  
>  void cpu_alpha_store_fpcr(CPUAlphaState *env, uint64_t val)
>  {
> +    static const uint8_t rm_map[] = {
> +        [FPCR_DYN_NORMAL >> FPCR_DYN_SHIFT] = float_round_nearest_even,
> +        [FPCR_DYN_CHOPPED >> FPCR_DYN_SHIFT] = float_round_to_zero,
> +        [FPCR_DYN_MINUS >> FPCR_DYN_SHIFT] = float_round_down,
> +        [FPCR_DYN_PLUS >> FPCR_DYN_SHIFT] = float_round_up,
> +    };
> +
>      uint32_t fpcr = val >> 32;
>      uint32_t t = 0;
>  
> @@ -48,22 +55,7 @@ void cpu_alpha_store_fpcr(CPUAlphaState *env, uint64_t val)
>      env->fpcr = fpcr;
>      env->fpcr_exc_enable = ~t & FPCR_STATUS_MASK;
>  
> -    switch (fpcr & FPCR_DYN_MASK) {
> -    case FPCR_DYN_NORMAL:
> -    default:
> -        t = float_round_nearest_even;
> -        break;
> -    case FPCR_DYN_CHOPPED:
> -        t = float_round_to_zero;
> -        break;
> -    case FPCR_DYN_MINUS:
> -        t = float_round_down;
> -        break;
> -    case FPCR_DYN_PLUS:
> -        t = float_round_up;
> -        break;
> -    }
> -    env->fpcr_dyn_round = t;
> +    env->fpcr_dyn_round = rm_map[(fpcr & FPCR_DYN_MASK) >> FPCR_DYN_SHIFT];
>  
>      env->fpcr_flush_to_zero = (fpcr & FPCR_UNFD) && (fpcr & FPCR_UNDZ);
>      env->fp_status.flush_inputs_to_zero = (fpcr & FPCR_DNZ) != 0;
> 



^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH 1/7] target/alpha: Use array for FPCR_DYN conversion
  2019-09-21  4:32 ` [PATCH 1/7] target/alpha: Use array for FPCR_DYN conversion Richard Henderson
  2019-09-21  8:53   ` Philippe Mathieu-Daudé
@ 2019-09-23 16:23   ` Alex Bennée
  1 sibling, 0 replies; 17+ messages in thread
From: Alex Bennée @ 2019-09-23 16:23 UTC (permalink / raw)
  To: Richard Henderson; +Cc: qemu-devel


Richard Henderson <richard.henderson@linaro.org> writes:

> This is a bit more straight-forward than using a switch statement.
> No functional change.
>
> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>

Reviewed-by: Alex Bennée <alex.bennee@linaro.org>


> ---
>  target/alpha/helper.c | 24 ++++++++----------------
>  1 file changed, 8 insertions(+), 16 deletions(-)
>
> diff --git a/target/alpha/helper.c b/target/alpha/helper.c
> index 19cda0a2db..6c1703682e 100644
> --- a/target/alpha/helper.c
> +++ b/target/alpha/helper.c
> @@ -36,6 +36,13 @@ uint64_t cpu_alpha_load_fpcr(CPUAlphaState *env)
>
>  void cpu_alpha_store_fpcr(CPUAlphaState *env, uint64_t val)
>  {
> +    static const uint8_t rm_map[] = {
> +        [FPCR_DYN_NORMAL >> FPCR_DYN_SHIFT] = float_round_nearest_even,
> +        [FPCR_DYN_CHOPPED >> FPCR_DYN_SHIFT] = float_round_to_zero,
> +        [FPCR_DYN_MINUS >> FPCR_DYN_SHIFT] = float_round_down,
> +        [FPCR_DYN_PLUS >> FPCR_DYN_SHIFT] = float_round_up,
> +    };
> +
>      uint32_t fpcr = val >> 32;
>      uint32_t t = 0;
>
> @@ -48,22 +55,7 @@ void cpu_alpha_store_fpcr(CPUAlphaState *env, uint64_t val)
>      env->fpcr = fpcr;
>      env->fpcr_exc_enable = ~t & FPCR_STATUS_MASK;
>
> -    switch (fpcr & FPCR_DYN_MASK) {
> -    case FPCR_DYN_NORMAL:
> -    default:
> -        t = float_round_nearest_even;
> -        break;
> -    case FPCR_DYN_CHOPPED:
> -        t = float_round_to_zero;
> -        break;
> -    case FPCR_DYN_MINUS:
> -        t = float_round_down;
> -        break;
> -    case FPCR_DYN_PLUS:
> -        t = float_round_up;
> -        break;
> -    }
> -    env->fpcr_dyn_round = t;
> +    env->fpcr_dyn_round = rm_map[(fpcr & FPCR_DYN_MASK) >> FPCR_DYN_SHIFT];
>
>      env->fpcr_flush_to_zero = (fpcr & FPCR_UNFD) && (fpcr & FPCR_UNDZ);
>      env->fp_status.flush_inputs_to_zero = (fpcr & FPCR_DNZ) != 0;


--
Alex Bennée


^ permalink raw reply	[flat|nested] 17+ messages in thread

* [PATCH 2/7] target/alpha: Fix SWCR_MAP_UMZ
  2019-09-21  4:32 [PATCH 0/7] target/alpha: Fix linux-user exception for CVTTQ Richard Henderson
  2019-09-21  4:32 ` [PATCH 1/7] target/alpha: Use array for FPCR_DYN conversion Richard Henderson
@ 2019-09-21  4:32 ` Richard Henderson
  2019-09-23 16:33   ` Alex Bennée
  2019-09-21  4:32 ` [PATCH 3/7] target/alpha: Fix SWCR_TRAP_ENABLE_MASK Richard Henderson
                   ` (5 subsequent siblings)
  7 siblings, 1 reply; 17+ messages in thread
From: Richard Henderson @ 2019-09-21  4:32 UTC (permalink / raw)
  To: qemu-devel; +Cc: alex.bennee

We were setting the wrong bit.  The fp_status.flush_to_zero
setting is overwritten by either the constant 1 or the value
of fpcr_flush_to_zero depending on bits within an fp insn.

Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
---
 target/alpha/helper.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/target/alpha/helper.c b/target/alpha/helper.c
index 6c1703682e..10602fb339 100644
--- a/target/alpha/helper.c
+++ b/target/alpha/helper.c
@@ -71,7 +71,7 @@ void cpu_alpha_store_fpcr(CPUAlphaState *env, uint64_t val)
         env->fp_status.flush_inputs_to_zero = 1;
     }
     if (env->swcr & SWCR_MAP_UMZ) {
-        env->fp_status.flush_to_zero = 1;
+        env->fpcr_flush_to_zero = 1;
     }
     env->fpcr_exc_enable &= ~(alpha_ieee_swcr_to_fpcr(env->swcr) >> 32);
 #endif
-- 
2.17.1



^ permalink raw reply related	[flat|nested] 17+ messages in thread

* Re: [PATCH 2/7] target/alpha: Fix SWCR_MAP_UMZ
  2019-09-21  4:32 ` [PATCH 2/7] target/alpha: Fix SWCR_MAP_UMZ Richard Henderson
@ 2019-09-23 16:33   ` Alex Bennée
  0 siblings, 0 replies; 17+ messages in thread
From: Alex Bennée @ 2019-09-23 16:33 UTC (permalink / raw)
  To: Richard Henderson; +Cc: qemu-devel


Richard Henderson <richard.henderson@linaro.org> writes:

> We were setting the wrong bit.  The fp_status.flush_to_zero
> setting is overwritten

".. in the generated code.."?

> by either the constant 1 or the value
> of fpcr_flush_to_zero depending on bits within an fp insn.
>
> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>

Reviewed-by: Alex Bennée <alex.bennee@linaro.org>

> ---
>  target/alpha/helper.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/target/alpha/helper.c b/target/alpha/helper.c
> index 6c1703682e..10602fb339 100644
> --- a/target/alpha/helper.c
> +++ b/target/alpha/helper.c
> @@ -71,7 +71,7 @@ void cpu_alpha_store_fpcr(CPUAlphaState *env, uint64_t val)
>          env->fp_status.flush_inputs_to_zero = 1;
>      }
>      if (env->swcr & SWCR_MAP_UMZ) {
> -        env->fp_status.flush_to_zero = 1;
> +        env->fpcr_flush_to_zero = 1;
>      }
>      env->fpcr_exc_enable &= ~(alpha_ieee_swcr_to_fpcr(env->swcr) >> 32);
>  #endif


--
Alex Bennée


^ permalink raw reply	[flat|nested] 17+ messages in thread

* [PATCH 3/7] target/alpha: Fix SWCR_TRAP_ENABLE_MASK
  2019-09-21  4:32 [PATCH 0/7] target/alpha: Fix linux-user exception for CVTTQ Richard Henderson
  2019-09-21  4:32 ` [PATCH 1/7] target/alpha: Use array for FPCR_DYN conversion Richard Henderson
  2019-09-21  4:32 ` [PATCH 2/7] target/alpha: Fix SWCR_MAP_UMZ Richard Henderson
@ 2019-09-21  4:32 ` Richard Henderson
  2019-09-21  4:32 ` [PATCH 4/7] target/alpha: Handle SWCR_MAP_DMZ earlier Richard Henderson
                   ` (4 subsequent siblings)
  7 siblings, 0 replies; 17+ messages in thread
From: Richard Henderson @ 2019-09-21  4:32 UTC (permalink / raw)
  To: qemu-devel; +Cc: alex.bennee

The CONFIG_USER_ONLY adjustment blindly mashed the swcr
exception enable bits into the fpcr exception disable bits.

However, fpcr_exc_enable has already converted the exception
disable bits into the exception status bits in order to make
it easier to mask status bits at runtime.

Instead, merge the swcr enable bits with the fpcr before we
convert to status bits.

Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
---
 target/alpha/helper.c | 31 ++++++++++++++++++-------------
 1 file changed, 18 insertions(+), 13 deletions(-)

diff --git a/target/alpha/helper.c b/target/alpha/helper.c
index 10602fb339..e21c488aa3 100644
--- a/target/alpha/helper.c
+++ b/target/alpha/helper.c
@@ -46,19 +46,8 @@ void cpu_alpha_store_fpcr(CPUAlphaState *env, uint64_t val)
     uint32_t fpcr = val >> 32;
     uint32_t t = 0;
 
-    t |= CONVERT_BIT(fpcr, FPCR_INED, FPCR_INE);
-    t |= CONVERT_BIT(fpcr, FPCR_UNFD, FPCR_UNF);
-    t |= CONVERT_BIT(fpcr, FPCR_OVFD, FPCR_OVF);
-    t |= CONVERT_BIT(fpcr, FPCR_DZED, FPCR_DZE);
-    t |= CONVERT_BIT(fpcr, FPCR_INVD, FPCR_INV);
-
+    /* Record the raw value before adjusting for linux-user.  */
     env->fpcr = fpcr;
-    env->fpcr_exc_enable = ~t & FPCR_STATUS_MASK;
-
-    env->fpcr_dyn_round = rm_map[(fpcr & FPCR_DYN_MASK) >> FPCR_DYN_SHIFT];
-
-    env->fpcr_flush_to_zero = (fpcr & FPCR_UNFD) && (fpcr & FPCR_UNDZ);
-    env->fp_status.flush_inputs_to_zero = (fpcr & FPCR_DNZ) != 0;
 
 #ifdef CONFIG_USER_ONLY
     /*
@@ -67,13 +56,29 @@ void cpu_alpha_store_fpcr(CPUAlphaState *env, uint64_t val)
      * which point the kernel's handler would emulate and apply
      * the software exception mask.
      */
+    uint32_t soft_fpcr = alpha_ieee_swcr_to_fpcr(env->swcr) >> 32;
+    fpcr |= soft_fpcr & FPCR_STATUS_MASK;
+#endif
+
+    t |= CONVERT_BIT(fpcr, FPCR_INED, FPCR_INE);
+    t |= CONVERT_BIT(fpcr, FPCR_UNFD, FPCR_UNF);
+    t |= CONVERT_BIT(fpcr, FPCR_OVFD, FPCR_OVF);
+    t |= CONVERT_BIT(fpcr, FPCR_DZED, FPCR_DZE);
+    t |= CONVERT_BIT(fpcr, FPCR_INVD, FPCR_INV);
+
+    env->fpcr_exc_enable = ~t & FPCR_STATUS_MASK;
+
+    env->fpcr_dyn_round = rm_map[(fpcr & FPCR_DYN_MASK) >> FPCR_DYN_SHIFT];
+
+    env->fpcr_flush_to_zero = (fpcr & FPCR_UNFD) && (fpcr & FPCR_UNDZ);
+    env->fp_status.flush_inputs_to_zero = (fpcr & FPCR_DNZ) != 0;
+#ifdef CONFIG_USER_ONLY
     if (env->swcr & SWCR_MAP_DMZ) {
         env->fp_status.flush_inputs_to_zero = 1;
     }
     if (env->swcr & SWCR_MAP_UMZ) {
         env->fpcr_flush_to_zero = 1;
     }
-    env->fpcr_exc_enable &= ~(alpha_ieee_swcr_to_fpcr(env->swcr) >> 32);
 #endif
 }
 
-- 
2.17.1



^ permalink raw reply related	[flat|nested] 17+ messages in thread

* [PATCH 4/7] target/alpha: Handle SWCR_MAP_DMZ earlier
  2019-09-21  4:32 [PATCH 0/7] target/alpha: Fix linux-user exception for CVTTQ Richard Henderson
                   ` (2 preceding siblings ...)
  2019-09-21  4:32 ` [PATCH 3/7] target/alpha: Fix SWCR_TRAP_ENABLE_MASK Richard Henderson
@ 2019-09-21  4:32 ` Richard Henderson
  2019-09-21  4:32 ` [PATCH 5/7] target/alpha: Write to fpcr_flush_to_zero once Richard Henderson
                   ` (3 subsequent siblings)
  7 siblings, 0 replies; 17+ messages in thread
From: Richard Henderson @ 2019-09-21  4:32 UTC (permalink / raw)
  To: qemu-devel; +Cc: alex.bennee

Since we're converting the swcr to fpcr format for exceptions,
it's trivial to add FPCR_DNZ to the set of fpcr bits overriden.
No functional change.

Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
---
 target/alpha/helper.c | 5 +----
 1 file changed, 1 insertion(+), 4 deletions(-)

diff --git a/target/alpha/helper.c b/target/alpha/helper.c
index e21c488aa3..2f959c65ef 100644
--- a/target/alpha/helper.c
+++ b/target/alpha/helper.c
@@ -57,7 +57,7 @@ void cpu_alpha_store_fpcr(CPUAlphaState *env, uint64_t val)
      * the software exception mask.
      */
     uint32_t soft_fpcr = alpha_ieee_swcr_to_fpcr(env->swcr) >> 32;
-    fpcr |= soft_fpcr & FPCR_STATUS_MASK;
+    fpcr |= soft_fpcr & (FPCR_STATUS_MASK | FPCR_DNZ);
 #endif
 
     t |= CONVERT_BIT(fpcr, FPCR_INED, FPCR_INE);
@@ -73,9 +73,6 @@ void cpu_alpha_store_fpcr(CPUAlphaState *env, uint64_t val)
     env->fpcr_flush_to_zero = (fpcr & FPCR_UNFD) && (fpcr & FPCR_UNDZ);
     env->fp_status.flush_inputs_to_zero = (fpcr & FPCR_DNZ) != 0;
 #ifdef CONFIG_USER_ONLY
-    if (env->swcr & SWCR_MAP_DMZ) {
-        env->fp_status.flush_inputs_to_zero = 1;
-    }
     if (env->swcr & SWCR_MAP_UMZ) {
         env->fpcr_flush_to_zero = 1;
     }
-- 
2.17.1



^ permalink raw reply related	[flat|nested] 17+ messages in thread

* [PATCH 5/7] target/alpha: Write to fpcr_flush_to_zero once
  2019-09-21  4:32 [PATCH 0/7] target/alpha: Fix linux-user exception for CVTTQ Richard Henderson
                   ` (3 preceding siblings ...)
  2019-09-21  4:32 ` [PATCH 4/7] target/alpha: Handle SWCR_MAP_DMZ earlier Richard Henderson
@ 2019-09-21  4:32 ` Richard Henderson
  2019-09-21  8:54   ` Philippe Mathieu-Daudé
  2019-09-23 16:38   ` Alex Bennée
  2019-09-21  4:32 ` [PATCH 6/7] target/alpha: Mask IOV exception with INV for user-only Richard Henderson
                   ` (2 subsequent siblings)
  7 siblings, 2 replies; 17+ messages in thread
From: Richard Henderson @ 2019-09-21  4:32 UTC (permalink / raw)
  To: qemu-devel; +Cc: alex.bennee

Tidy the computation of the value; no functional change.

Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
---
 target/alpha/helper.c | 9 ++++-----
 1 file changed, 4 insertions(+), 5 deletions(-)

diff --git a/target/alpha/helper.c b/target/alpha/helper.c
index 2f959c65ef..1b3479738b 100644
--- a/target/alpha/helper.c
+++ b/target/alpha/helper.c
@@ -69,14 +69,13 @@ void cpu_alpha_store_fpcr(CPUAlphaState *env, uint64_t val)
     env->fpcr_exc_enable = ~t & FPCR_STATUS_MASK;
 
     env->fpcr_dyn_round = rm_map[(fpcr & FPCR_DYN_MASK) >> FPCR_DYN_SHIFT];
-
-    env->fpcr_flush_to_zero = (fpcr & FPCR_UNFD) && (fpcr & FPCR_UNDZ);
     env->fp_status.flush_inputs_to_zero = (fpcr & FPCR_DNZ) != 0;
+
+    t = (fpcr & FPCR_UNFD) && (fpcr & FPCR_UNDZ);
 #ifdef CONFIG_USER_ONLY
-    if (env->swcr & SWCR_MAP_UMZ) {
-        env->fpcr_flush_to_zero = 1;
-    }
+    t |= (env->swcr & SWCR_MAP_UMZ) != 0;
 #endif
+    env->fpcr_flush_to_zero = t;
 }
 
 uint64_t helper_load_fpcr(CPUAlphaState *env)
-- 
2.17.1



^ permalink raw reply related	[flat|nested] 17+ messages in thread

* Re: [PATCH 5/7] target/alpha: Write to fpcr_flush_to_zero once
  2019-09-21  4:32 ` [PATCH 5/7] target/alpha: Write to fpcr_flush_to_zero once Richard Henderson
@ 2019-09-21  8:54   ` Philippe Mathieu-Daudé
  2019-09-23 16:38   ` Alex Bennée
  1 sibling, 0 replies; 17+ messages in thread
From: Philippe Mathieu-Daudé @ 2019-09-21  8:54 UTC (permalink / raw)
  To: Richard Henderson, qemu-devel; +Cc: alex.bennee

On 9/21/19 6:32 AM, Richard Henderson wrote:
> Tidy the computation of the value; no functional change.
> 
> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>

Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com>

> ---
>  target/alpha/helper.c | 9 ++++-----
>  1 file changed, 4 insertions(+), 5 deletions(-)
> 
> diff --git a/target/alpha/helper.c b/target/alpha/helper.c
> index 2f959c65ef..1b3479738b 100644
> --- a/target/alpha/helper.c
> +++ b/target/alpha/helper.c
> @@ -69,14 +69,13 @@ void cpu_alpha_store_fpcr(CPUAlphaState *env, uint64_t val)
>      env->fpcr_exc_enable = ~t & FPCR_STATUS_MASK;
>  
>      env->fpcr_dyn_round = rm_map[(fpcr & FPCR_DYN_MASK) >> FPCR_DYN_SHIFT];
> -
> -    env->fpcr_flush_to_zero = (fpcr & FPCR_UNFD) && (fpcr & FPCR_UNDZ);
>      env->fp_status.flush_inputs_to_zero = (fpcr & FPCR_DNZ) != 0;
> +
> +    t = (fpcr & FPCR_UNFD) && (fpcr & FPCR_UNDZ);
>  #ifdef CONFIG_USER_ONLY
> -    if (env->swcr & SWCR_MAP_UMZ) {
> -        env->fpcr_flush_to_zero = 1;
> -    }
> +    t |= (env->swcr & SWCR_MAP_UMZ) != 0;
>  #endif
> +    env->fpcr_flush_to_zero = t;
>  }
>  
>  uint64_t helper_load_fpcr(CPUAlphaState *env)
> 



^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH 5/7] target/alpha: Write to fpcr_flush_to_zero once
  2019-09-21  4:32 ` [PATCH 5/7] target/alpha: Write to fpcr_flush_to_zero once Richard Henderson
  2019-09-21  8:54   ` Philippe Mathieu-Daudé
@ 2019-09-23 16:38   ` Alex Bennée
  1 sibling, 0 replies; 17+ messages in thread
From: Alex Bennée @ 2019-09-23 16:38 UTC (permalink / raw)
  To: Richard Henderson; +Cc: qemu-devel


Richard Henderson <richard.henderson@linaro.org> writes:

> Tidy the computation of the value; no functional change.
>
> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>

Reviewed-by: Alex Bennée <alex.bennee@linaro.org>

> ---
>  target/alpha/helper.c | 9 ++++-----
>  1 file changed, 4 insertions(+), 5 deletions(-)
>
> diff --git a/target/alpha/helper.c b/target/alpha/helper.c
> index 2f959c65ef..1b3479738b 100644
> --- a/target/alpha/helper.c
> +++ b/target/alpha/helper.c
> @@ -69,14 +69,13 @@ void cpu_alpha_store_fpcr(CPUAlphaState *env, uint64_t val)
>      env->fpcr_exc_enable = ~t & FPCR_STATUS_MASK;
>
>      env->fpcr_dyn_round = rm_map[(fpcr & FPCR_DYN_MASK) >> FPCR_DYN_SHIFT];
> -
> -    env->fpcr_flush_to_zero = (fpcr & FPCR_UNFD) && (fpcr & FPCR_UNDZ);
>      env->fp_status.flush_inputs_to_zero = (fpcr & FPCR_DNZ) != 0;
> +
> +    t = (fpcr & FPCR_UNFD) && (fpcr & FPCR_UNDZ);
>  #ifdef CONFIG_USER_ONLY
> -    if (env->swcr & SWCR_MAP_UMZ) {
> -        env->fpcr_flush_to_zero = 1;
> -    }
> +    t |= (env->swcr & SWCR_MAP_UMZ) != 0;
>  #endif
> +    env->fpcr_flush_to_zero = t;
>  }
>
>  uint64_t helper_load_fpcr(CPUAlphaState *env)


--
Alex Bennée


^ permalink raw reply	[flat|nested] 17+ messages in thread

* [PATCH 6/7] target/alpha: Mask IOV exception with INV for user-only
  2019-09-21  4:32 [PATCH 0/7] target/alpha: Fix linux-user exception for CVTTQ Richard Henderson
                   ` (4 preceding siblings ...)
  2019-09-21  4:32 ` [PATCH 5/7] target/alpha: Write to fpcr_flush_to_zero once Richard Henderson
@ 2019-09-21  4:32 ` Richard Henderson
  2019-09-23 16:39   ` Alex Bennée
  2019-09-21  4:32 ` [PATCH 7/7] target/alpha: Tidy helper_fp_exc_raise_s Richard Henderson
  2019-09-23 16:44 ` [PATCH 0/7] target/alpha: Fix linux-user exception for CVTTQ Alex Bennée
  7 siblings, 1 reply; 17+ messages in thread
From: Richard Henderson @ 2019-09-21  4:32 UTC (permalink / raw)
  To: qemu-devel; +Cc: alex.bennee

The kernel masks the integer overflow exception with the
software invalid exception mask.  Include IOV in the set
of exception bits masked by fpcr_exc_enable.

Fixes the new float_convs test.

Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
---
 target/alpha/helper.c | 7 +++++++
 1 file changed, 7 insertions(+)

diff --git a/target/alpha/helper.c b/target/alpha/helper.c
index 1b3479738b..55d7274d94 100644
--- a/target/alpha/helper.c
+++ b/target/alpha/helper.c
@@ -58,6 +58,13 @@ void cpu_alpha_store_fpcr(CPUAlphaState *env, uint64_t val)
      */
     uint32_t soft_fpcr = alpha_ieee_swcr_to_fpcr(env->swcr) >> 32;
     fpcr |= soft_fpcr & (FPCR_STATUS_MASK | FPCR_DNZ);
+
+    /*
+     * The IOV exception is disabled by the kernel with SWCR_TRAP_ENABLE_INV,
+     * which got mapped by alpha_ieee_swcr_to_fpcr to FPCR_INVD.
+     * Add FPCR_IOV to fpcr_exc_enable so that it is handled identically.
+     */
+    t |= CONVERT_BIT(soft_fpcr, FPCR_INVD, FPCR_IOV);
 #endif
 
     t |= CONVERT_BIT(fpcr, FPCR_INED, FPCR_INE);
-- 
2.17.1



^ permalink raw reply related	[flat|nested] 17+ messages in thread

* Re: [PATCH 6/7] target/alpha: Mask IOV exception with INV for user-only
  2019-09-21  4:32 ` [PATCH 6/7] target/alpha: Mask IOV exception with INV for user-only Richard Henderson
@ 2019-09-23 16:39   ` Alex Bennée
  0 siblings, 0 replies; 17+ messages in thread
From: Alex Bennée @ 2019-09-23 16:39 UTC (permalink / raw)
  To: Richard Henderson; +Cc: qemu-devel


Richard Henderson <richard.henderson@linaro.org> writes:

> The kernel masks the integer overflow exception with the
> software invalid exception mask.  Include IOV in the set
> of exception bits masked by fpcr_exc_enable.
>
> Fixes the new float_convs test.
>
> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>

Reviewed-by: Alex Bennée <alex.bennee@linaro.org>
Tested-by: Alex Bennée <alex.bennee@linaro.org>

> ---
>  target/alpha/helper.c | 7 +++++++
>  1 file changed, 7 insertions(+)
>
> diff --git a/target/alpha/helper.c b/target/alpha/helper.c
> index 1b3479738b..55d7274d94 100644
> --- a/target/alpha/helper.c
> +++ b/target/alpha/helper.c
> @@ -58,6 +58,13 @@ void cpu_alpha_store_fpcr(CPUAlphaState *env, uint64_t val)
>       */
>      uint32_t soft_fpcr = alpha_ieee_swcr_to_fpcr(env->swcr) >> 32;
>      fpcr |= soft_fpcr & (FPCR_STATUS_MASK | FPCR_DNZ);
> +
> +    /*
> +     * The IOV exception is disabled by the kernel with SWCR_TRAP_ENABLE_INV,
> +     * which got mapped by alpha_ieee_swcr_to_fpcr to FPCR_INVD.
> +     * Add FPCR_IOV to fpcr_exc_enable so that it is handled identically.
> +     */
> +    t |= CONVERT_BIT(soft_fpcr, FPCR_INVD, FPCR_IOV);
>  #endif
>
>      t |= CONVERT_BIT(fpcr, FPCR_INED, FPCR_INE);


--
Alex Bennée


^ permalink raw reply	[flat|nested] 17+ messages in thread

* [PATCH 7/7] target/alpha: Tidy helper_fp_exc_raise_s
  2019-09-21  4:32 [PATCH 0/7] target/alpha: Fix linux-user exception for CVTTQ Richard Henderson
                   ` (5 preceding siblings ...)
  2019-09-21  4:32 ` [PATCH 6/7] target/alpha: Mask IOV exception with INV for user-only Richard Henderson
@ 2019-09-21  4:32 ` Richard Henderson
  2019-09-21  8:57   ` Philippe Mathieu-Daudé
  2019-09-23 16:40   ` Alex Bennée
  2019-09-23 16:44 ` [PATCH 0/7] target/alpha: Fix linux-user exception for CVTTQ Alex Bennée
  7 siblings, 2 replies; 17+ messages in thread
From: Richard Henderson @ 2019-09-21  4:32 UTC (permalink / raw)
  To: qemu-devel; +Cc: alex.bennee

Remove a redundant masking of ignore.  Once that's gone it is
obvious that the system-mode inner test is redundant with the
outer test.  Move the fpcr_exc_enable masking up and tidy.

No functional change.

Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
---
 target/alpha/fpu_helper.c | 15 ++++-----------
 1 file changed, 4 insertions(+), 11 deletions(-)

diff --git a/target/alpha/fpu_helper.c b/target/alpha/fpu_helper.c
index 62a066d902..df8b58963b 100644
--- a/target/alpha/fpu_helper.c
+++ b/target/alpha/fpu_helper.c
@@ -90,25 +90,18 @@ void helper_fp_exc_raise_s(CPUAlphaState *env, uint32_t ignore, uint32_t regno)
     uint32_t exc = env->error_code & ~ignore;
     if (exc) {
         env->fpcr |= exc;
-        exc &= ~ignore;
-#ifdef CONFIG_USER_ONLY
-        /*
-         * In user mode, the kernel's software handler only
-         * delivers a signal if the exception is enabled.
-         */
-        if (!(exc & env->fpcr_exc_enable)) {
-            return;
-        }
-#else
+        exc &= env->fpcr_exc_enable;
         /*
          * In system mode, the software handler gets invoked
          * for any non-ignored exception.
+         * In user mode, the kernel's software handler only
+         * delivers a signal if the exception is enabled.
          */
+#ifdef CONFIG_USER_ONLY
         if (!exc) {
             return;
         }
 #endif
-        exc &= env->fpcr_exc_enable;
         fp_exc_raise1(env, GETPC(), exc, regno, EXC_M_SWC);
     }
 }
-- 
2.17.1



^ permalink raw reply related	[flat|nested] 17+ messages in thread

* Re: [PATCH 7/7] target/alpha: Tidy helper_fp_exc_raise_s
  2019-09-21  4:32 ` [PATCH 7/7] target/alpha: Tidy helper_fp_exc_raise_s Richard Henderson
@ 2019-09-21  8:57   ` Philippe Mathieu-Daudé
  2019-09-23 16:40   ` Alex Bennée
  1 sibling, 0 replies; 17+ messages in thread
From: Philippe Mathieu-Daudé @ 2019-09-21  8:57 UTC (permalink / raw)
  To: Richard Henderson, qemu-devel; +Cc: alex.bennee

On 9/21/19 6:32 AM, Richard Henderson wrote:
> Remove a redundant masking of ignore.  Once that's gone it is
> obvious that the system-mode inner test is redundant with the
> outer test.  Move the fpcr_exc_enable masking up and tidy.
> 
> No functional change.
> 
> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>

Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com>

> ---
>  target/alpha/fpu_helper.c | 15 ++++-----------
>  1 file changed, 4 insertions(+), 11 deletions(-)
> 
> diff --git a/target/alpha/fpu_helper.c b/target/alpha/fpu_helper.c
> index 62a066d902..df8b58963b 100644
> --- a/target/alpha/fpu_helper.c
> +++ b/target/alpha/fpu_helper.c
> @@ -90,25 +90,18 @@ void helper_fp_exc_raise_s(CPUAlphaState *env, uint32_t ignore, uint32_t regno)
>      uint32_t exc = env->error_code & ~ignore;
>      if (exc) {
>          env->fpcr |= exc;
> -        exc &= ~ignore;
> -#ifdef CONFIG_USER_ONLY
> -        /*
> -         * In user mode, the kernel's software handler only
> -         * delivers a signal if the exception is enabled.
> -         */
> -        if (!(exc & env->fpcr_exc_enable)) {
> -            return;
> -        }
> -#else
> +        exc &= env->fpcr_exc_enable;
>          /*
>           * In system mode, the software handler gets invoked
>           * for any non-ignored exception.
> +         * In user mode, the kernel's software handler only
> +         * delivers a signal if the exception is enabled.
>           */
> +#ifdef CONFIG_USER_ONLY
>          if (!exc) {
>              return;
>          }
>  #endif
> -        exc &= env->fpcr_exc_enable;
>          fp_exc_raise1(env, GETPC(), exc, regno, EXC_M_SWC);
>      }
>  }
> 



^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH 7/7] target/alpha: Tidy helper_fp_exc_raise_s
  2019-09-21  4:32 ` [PATCH 7/7] target/alpha: Tidy helper_fp_exc_raise_s Richard Henderson
  2019-09-21  8:57   ` Philippe Mathieu-Daudé
@ 2019-09-23 16:40   ` Alex Bennée
  1 sibling, 0 replies; 17+ messages in thread
From: Alex Bennée @ 2019-09-23 16:40 UTC (permalink / raw)
  To: Richard Henderson; +Cc: qemu-devel


Richard Henderson <richard.henderson@linaro.org> writes:

> Remove a redundant masking of ignore.  Once that's gone it is
> obvious that the system-mode inner test is redundant with the
> outer test.  Move the fpcr_exc_enable masking up and tidy.
>
> No functional change.
>
> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>

Reviewed-by: Alex Bennée <alex.bennee@linaro.org>

> ---
>  target/alpha/fpu_helper.c | 15 ++++-----------
>  1 file changed, 4 insertions(+), 11 deletions(-)
>
> diff --git a/target/alpha/fpu_helper.c b/target/alpha/fpu_helper.c
> index 62a066d902..df8b58963b 100644
> --- a/target/alpha/fpu_helper.c
> +++ b/target/alpha/fpu_helper.c
> @@ -90,25 +90,18 @@ void helper_fp_exc_raise_s(CPUAlphaState *env, uint32_t ignore, uint32_t regno)
>      uint32_t exc = env->error_code & ~ignore;
>      if (exc) {
>          env->fpcr |= exc;
> -        exc &= ~ignore;
> -#ifdef CONFIG_USER_ONLY
> -        /*
> -         * In user mode, the kernel's software handler only
> -         * delivers a signal if the exception is enabled.
> -         */
> -        if (!(exc & env->fpcr_exc_enable)) {
> -            return;
> -        }
> -#else
> +        exc &= env->fpcr_exc_enable;
>          /*
>           * In system mode, the software handler gets invoked
>           * for any non-ignored exception.
> +         * In user mode, the kernel's software handler only
> +         * delivers a signal if the exception is enabled.
>           */
> +#ifdef CONFIG_USER_ONLY
>          if (!exc) {
>              return;
>          }
>  #endif
> -        exc &= env->fpcr_exc_enable;
>          fp_exc_raise1(env, GETPC(), exc, regno, EXC_M_SWC);
>      }
>  }


--
Alex Bennée


^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH 0/7] target/alpha: Fix linux-user exception for CVTTQ
  2019-09-21  4:32 [PATCH 0/7] target/alpha: Fix linux-user exception for CVTTQ Richard Henderson
                   ` (6 preceding siblings ...)
  2019-09-21  4:32 ` [PATCH 7/7] target/alpha: Tidy helper_fp_exc_raise_s Richard Henderson
@ 2019-09-23 16:44 ` Alex Bennée
  7 siblings, 0 replies; 17+ messages in thread
From: Alex Bennée @ 2019-09-23 16:44 UTC (permalink / raw)
  To: Richard Henderson; +Cc: qemu-devel


Richard Henderson <richard.henderson@linaro.org> writes:

> Alex's new float_convs test dies with SIGFPE for alpha-linux-user.
>
> This is fixed in patch 6, where I mask the exception similar to how
> the kernel would, when passed through the software completion handler.
>
> Patches 2 & 3 fix bugs that I noticed in the process; the rest are
> simple cleanup, trying to make the code better or easier.

Now Richard has eli5'd the CONVERT_BITS magic for me>

Reviewed-by: Alex Bennée <alex.bennee@linaro.org>

the rest of the series.

>
>
> r~
>
>
> Richard Henderson (7):
>   target/alpha: Use array for FPCR_DYN conversion
>   target/alpha: Fix SWCR_MAP_UMZ
>   target/alpha: Fix SWCR_TRAP_ENABLE_MASK
>   target/alpha: Handle SWCR_MAP_DMZ earlier
>   target/alpha: Write to fpcr_flush_to_zero once
>   target/alpha: Mask IOV exception with INV for user-only
>   target/alpha: Tidy helper_fp_exc_raise_s
>
>  target/alpha/fpu_helper.c | 15 +++------
>  target/alpha/helper.c     | 68 +++++++++++++++++++--------------------
>  2 files changed, 38 insertions(+), 45 deletions(-)


--
Alex Bennée


^ permalink raw reply	[flat|nested] 17+ messages in thread

end of thread, other threads:[~2019-09-23 16:57 UTC | newest]

Thread overview: 17+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-09-21  4:32 [PATCH 0/7] target/alpha: Fix linux-user exception for CVTTQ Richard Henderson
2019-09-21  4:32 ` [PATCH 1/7] target/alpha: Use array for FPCR_DYN conversion Richard Henderson
2019-09-21  8:53   ` Philippe Mathieu-Daudé
2019-09-23 16:23   ` Alex Bennée
2019-09-21  4:32 ` [PATCH 2/7] target/alpha: Fix SWCR_MAP_UMZ Richard Henderson
2019-09-23 16:33   ` Alex Bennée
2019-09-21  4:32 ` [PATCH 3/7] target/alpha: Fix SWCR_TRAP_ENABLE_MASK Richard Henderson
2019-09-21  4:32 ` [PATCH 4/7] target/alpha: Handle SWCR_MAP_DMZ earlier Richard Henderson
2019-09-21  4:32 ` [PATCH 5/7] target/alpha: Write to fpcr_flush_to_zero once Richard Henderson
2019-09-21  8:54   ` Philippe Mathieu-Daudé
2019-09-23 16:38   ` Alex Bennée
2019-09-21  4:32 ` [PATCH 6/7] target/alpha: Mask IOV exception with INV for user-only Richard Henderson
2019-09-23 16:39   ` Alex Bennée
2019-09-21  4:32 ` [PATCH 7/7] target/alpha: Tidy helper_fp_exc_raise_s Richard Henderson
2019-09-21  8:57   ` Philippe Mathieu-Daudé
2019-09-23 16:40   ` Alex Bennée
2019-09-23 16:44 ` [PATCH 0/7] target/alpha: Fix linux-user exception for CVTTQ Alex Bennée

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).