[Qemu-devel] [PATCH 0/2] fp-bench fixes

[Qemu-devel] [PATCH 0/2] fp-bench fixes

[Emilio G. Cota]

Two small fixes for fp-bench. They fix the fact that we weren't
generating normals, even though we intended to.

You can pull this series from:
  https://github.com/cota/qemu/tree/fp-bench-fixes

Thanks,

		Emilio

[Qemu-devel] [PATCH 1/2] fp-bench: fix update_random_ops

[Emilio G. Cota]

The second test in the branches is wrong; fix while converting
to a switch statement, which is easier to get right.

Signed-off-by: Emilio G. Cota <cota@braap.org>
---
 tests/fp/fp-bench.c | 11 ++++++++---
 1 file changed, 8 insertions(+), 3 deletions(-)

diff --git a/tests/fp/fp-bench.c b/tests/fp/fp-bench.c
index f5bc5edebf..546bac9c9c 100644
--- a/tests/fp/fp-bench.c
+++ b/tests/fp/fp-bench.c
@@ -143,15 +143,20 @@ static void update_random_ops(int n_ops, enum precision prec)
     for (i = 0; i < n_ops; i++) {
         uint64_t r = random_ops[i];
 
-        if (prec == PREC_SINGLE || PREC_FLOAT32) {
+        switch (prec) {
+        case PREC_SINGLE:
+        case PREC_FLOAT32:
             do {
                 r = xorshift64star(r);
             } while (!float32_is_normal(r));
-        } else if (prec == PREC_DOUBLE || PREC_FLOAT64) {
+            break;
+        case PREC_DOUBLE:
+        case PREC_FLOAT64:
             do {
                 r = xorshift64star(r);
             } while (!float64_is_normal(r));
-        } else {
+            break;
+        default:
             g_assert_not_reached();
         }
         random_ops[i] = r;
-- 
2.17.1

Re: [Qemu-devel] [PATCH 1/2] fp-bench: fix update_random_ops

[Richard Henderson]

On 12/22/18 6:51 AM, Emilio G. Cota wrote:
> The second test in the branches is wrong; fix while converting
> to a switch statement, which is easier to get right.
> 
> Signed-off-by: Emilio G. Cota <cota@braap.org>
> ---
>  tests/fp/fp-bench.c | 11 ++++++++---
>  1 file changed, 8 insertions(+), 3 deletions(-)

Whoops.

Reviewed-by: Richard Henderson <richard.henderson@linaro.org>

r~

Re: [Qemu-devel] [PATCH 1/2] fp-bench: fix update_random_ops

[Philippe Mathieu-Daudé]

On 12/21/18 8:51 PM, Emilio G. Cota wrote:
> The second test in the branches is wrong; fix while converting
> to a switch statement, which is easier to get right.
> 
> Signed-off-by: Emilio G. Cota <cota@braap.org>
> ---
>  tests/fp/fp-bench.c | 11 ++++++++---
>  1 file changed, 8 insertions(+), 3 deletions(-)
> 
> diff --git a/tests/fp/fp-bench.c b/tests/fp/fp-bench.c
> index f5bc5edebf..546bac9c9c 100644
> --- a/tests/fp/fp-bench.c
> +++ b/tests/fp/fp-bench.c
> @@ -143,15 +143,20 @@ static void update_random_ops(int n_ops, enum precision prec)
>      for (i = 0; i < n_ops; i++) {
>          uint64_t r = random_ops[i];
>  
> -        if (prec == PREC_SINGLE || PREC_FLOAT32) {

:)

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

> +        switch (prec) {
> +        case PREC_SINGLE:
> +        case PREC_FLOAT32:
>              do {
>                  r = xorshift64star(r);
>              } while (!float32_is_normal(r));
> -        } else if (prec == PREC_DOUBLE || PREC_FLOAT64) {
> +            break;
> +        case PREC_DOUBLE:
> +        case PREC_FLOAT64:
>              do {
>                  r = xorshift64star(r);
>              } while (!float64_is_normal(r));
> -        } else {
> +            break;
> +        default:
>              g_assert_not_reached();
>          }
>          random_ops[i] = r;
> 

[Qemu-devel] [PATCH 2/2] fp-bench: remove wrong exponent raise in fill_random

[Emilio G. Cota]

At this point random_ops[] only contains normals, so there's
no need to do anything to them. In fact, raising the exponent
here can make the output !normal, which is precisely
what the comment says we want to avoid.

Signed-off-by: Emilio G. Cota <cota@braap.org>
---
 tests/fp/fp-bench.c | 4 ----
 1 file changed, 4 deletions(-)

diff --git a/tests/fp/fp-bench.c b/tests/fp/fp-bench.c
index 546bac9c9c..4ba5e1d2d4 100644
--- a/tests/fp/fp-bench.c
+++ b/tests/fp/fp-bench.c
@@ -176,8 +176,6 @@ static void fill_random(union fp *ops, int n_ops, enum precision prec,
             if (no_neg && float32_is_neg(ops[i].f32)) {
                 ops[i].f32 = float32_chs(ops[i].f32);
             }
-            /* raise the exponent to limit the frequency of denormal results */
-            ops[i].f32 |= 0x40000000;
             break;
         case PREC_DOUBLE:
         case PREC_FLOAT64:
@@ -185,8 +183,6 @@ static void fill_random(union fp *ops, int n_ops, enum precision prec,
             if (no_neg && float64_is_neg(ops[i].f64)) {
                 ops[i].f64 = float64_chs(ops[i].f64);
             }
-            /* raise the exponent to limit the frequency of denormal results */
-            ops[i].f64 |= LIT64(0x4000000000000000);
             break;
         default:
             g_assert_not_reached();
-- 
2.17.1

Re: [Qemu-devel] [PATCH 2/2] fp-bench: remove wrong exponent raise in fill_random

[Richard Henderson]

On 12/22/18 6:52 AM, Emilio G. Cota wrote:
> At this point random_ops[] only contains normals, so there's
> no need to do anything to them. In fact, raising the exponent
> here can make the output !normal, which is precisely
> what the comment says we want to avoid.
> 
> Signed-off-by: Emilio G. Cota <cota@braap.org>
> ---
>  tests/fp/fp-bench.c | 4 ----
>  1 file changed, 4 deletions(-)

Reviewed-by: Richard Henderson <richard.henderson@linaro.org>

r~

Re: [Qemu-devel] [PATCH 0/2] fp-bench fixes

[Alex Bennée]

Emilio G. Cota <cota@braap.org> writes:

> Two small fixes for fp-bench. They fix the fact that we weren't
> generating normals, even though we intended to.
>
> You can pull this series from:
>   https://github.com/cota/qemu/tree/fp-bench-fixes
>
> Thanks,
>
> 		Emilio

Queued to fpu/next, thanks.

--
Alex Bennée