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

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

[Richard Henderson]

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

[PATCH 1/7] target/alpha: Use array for FPCR_DYN conversion

[Richard Henderson]

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

[PATCH 2/7] target/alpha: Fix SWCR_MAP_UMZ

[Richard Henderson]

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

[PATCH 3/7] target/alpha: Fix SWCR_TRAP_ENABLE_MASK

[Richard Henderson]

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

[PATCH 4/7] target/alpha: Handle SWCR_MAP_DMZ earlier

[Richard Henderson]

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

[PATCH 5/7] target/alpha: Write to fpcr_flush_to_zero once

[Richard Henderson]

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

[PATCH 6/7] target/alpha: Mask IOV exception with INV for user-only

[Richard Henderson]

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

[PATCH 7/7] target/alpha: Tidy helper_fp_exc_raise_s

[Richard Henderson]

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

Re: [PATCH 1/7] target/alpha: Use array for FPCR_DYN conversion

[Philippe Mathieu-Daudé]

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;
> 

Re: [PATCH 5/7] target/alpha: Write to fpcr_flush_to_zero once

[Philippe Mathieu-Daudé]

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

Re: [PATCH 7/7] target/alpha: Tidy helper_fp_exc_raise_s

[Philippe Mathieu-Daudé]

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);
>      }
>  }
> 

Re: [PATCH 1/7] target/alpha: Use array for FPCR_DYN conversion

[Alex Bennée]

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

Re: [PATCH 2/7] target/alpha: Fix SWCR_MAP_UMZ

[Alex Bennée]

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

Re: [PATCH 5/7] target/alpha: Write to fpcr_flush_to_zero once

[Alex Bennée]

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

Re: [PATCH 6/7] target/alpha: Mask IOV exception with INV for user-only

[Alex Bennée]

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

Re: [PATCH 7/7] target/alpha: Tidy helper_fp_exc_raise_s

[Alex Bennée]

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

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

[Alex Bennée]

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