[v1,08/15] s390x/tcg: Handle SET FPC AND LOAD FPC 3-bit BFP rounding modes

Message ID 20190212110308.13707-10-david@redhat.com
State New
Headers show
Series
  • [v1] s390x: Add floating-point extension facility to "qemu" cpu model
Related show

Commit Message

David Hildenbrand Feb. 12, 2019, 11:03 a.m.
We already forward the 3 bits correctly in the translation functions. We
also have to handle them properly and check for specification
exceptions.

Setting an invalid rounding mode (BFP only, all DFP rounding modes)
results in a specification exception. Setting unassigned bits in the
fpc, results in a specification exception.

This fixes LOAD FPC (AND SIGNAL), SET FPC (AND SIGNAL). Also for,
SET BFP ROUNDING MODE, 3-bit rounding mode is now explicitly checked.

Notes:
1. Use "float_round_to_zero" for now to handle "Round to prepare for
   shorter precision". Looking at the PoP "Summary of Rounding and Range
   Actions" for BFP. They differ when it comes to tiny values.
2. TCG_CALL_NO_WG is required for sfpc handler, as we now inject
   exceptions.

We won't be modeling abscence of the "floating-point extension facility"
for now, not necessary as most take the facility for granted without
checking.

z14 PoP, 9-23, "LOAD FPC"
    When the floating-point extension facility is
    installed, bits 29-31 of the second operand must
    specify a valid BFP rounding mode and bits 6-7,
    14-15, 24, and 28 must be zero; otherwise, a
    specification exception is recognized.

Signed-off-by: David Hildenbrand <david@redhat.com>
---
 target/s390x/fpu_helper.c | 24 ++++++++++++++++++++----
 target/s390x/helper.h     |  2 +-
 2 files changed, 21 insertions(+), 5 deletions(-)

Comments

Richard Henderson Feb. 12, 2019, 7:07 p.m. | #1
On 2/12/19 3:03 AM, David Hildenbrand wrote:
> We already forward the 3 bits correctly in the translation functions. We
> also have to handle them properly and check for specification
> exceptions.
> 
> Setting an invalid rounding mode (BFP only, all DFP rounding modes)
> results in a specification exception. Setting unassigned bits in the
> fpc, results in a specification exception.
> 
> This fixes LOAD FPC (AND SIGNAL), SET FPC (AND SIGNAL). Also for,
> SET BFP ROUNDING MODE, 3-bit rounding mode is now explicitly checked.
> 
> Notes:
> 1. Use "float_round_to_zero" for now to handle "Round to prepare for
>    shorter precision". Looking at the PoP "Summary of Rounding and Range
>    Actions" for BFP. They differ when it comes to tiny values.
> 2. TCG_CALL_NO_WG is required for sfpc handler, as we now inject
>    exceptions.
> 
> We won't be modeling abscence of the "floating-point extension facility"
> for now, not necessary as most take the facility for granted without
> checking.
> 
> z14 PoP, 9-23, "LOAD FPC"
>     When the floating-point extension facility is
>     installed, bits 29-31 of the second operand must
>     specify a valid BFP rounding mode and bits 6-7,
>     14-15, 24, and 28 must be zero; otherwise, a
>     specification exception is recognized.


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

> +    /*
> +     * FIXME: we actually want something like round_to_odd, but that does not
> +     * support all data types yet.
> +     */
> +    float_round_to_zero,

Yes, you want round_to_odd.  I suppose that's not valid for float128 right now?


r~
David Hildenbrand Feb. 12, 2019, 7:32 p.m. | #2
On 12.02.19 20:07, Richard Henderson wrote:
> On 2/12/19 3:03 AM, David Hildenbrand wrote:
>> We already forward the 3 bits correctly in the translation functions. We
>> also have to handle them properly and check for specification
>> exceptions.
>>
>> Setting an invalid rounding mode (BFP only, all DFP rounding modes)
>> results in a specification exception. Setting unassigned bits in the
>> fpc, results in a specification exception.
>>
>> This fixes LOAD FPC (AND SIGNAL), SET FPC (AND SIGNAL). Also for,
>> SET BFP ROUNDING MODE, 3-bit rounding mode is now explicitly checked.
>>
>> Notes:
>> 1. Use "float_round_to_zero" for now to handle "Round to prepare for
>>    shorter precision". Looking at the PoP "Summary of Rounding and Range
>>    Actions" for BFP. They differ when it comes to tiny values.
>> 2. TCG_CALL_NO_WG is required for sfpc handler, as we now inject
>>    exceptions.
>>
>> We won't be modeling abscence of the "floating-point extension facility"
>> for now, not necessary as most take the facility for granted without
>> checking.
>>
>> z14 PoP, 9-23, "LOAD FPC"
>>     When the floating-point extension facility is
>>     installed, bits 29-31 of the second operand must
>>     specify a valid BFP rounding mode and bits 6-7,
>>     14-15, 24, and 28 must be zero; otherwise, a
>>     specification exception is recognized.
> 
> 
> Reviewed-by: Richard Henderson <richard.henderson@linaro.org>
> 
>> +    /*
>> +     * FIXME: we actually want something like round_to_odd, but that does not
>> +     * support all data types yet.
>> +     */
>> +    float_round_to_zero,
> 
> Yes, you want round_to_odd.  I suppose that's not valid for float128 right now?

roundAndPackFloat64()

as well as

roundAndPackFloat128()

support it.

It's not implemented for round_canonical(), round_to_int(),
roundAndPackInt32(), roundAndPackInt64(), roundAndPackUint64(),
roundAndPackFloat32() (and roundAndPackFloatx80()).

I assume at least 32bit is missing. I can't judge if the other functions
are relevant (x80 clearly not).

> 
> 
> r~
>
Richard Henderson Feb. 12, 2019, 7:56 p.m. | #3
On 2/12/19 11:32 AM, David Hildenbrand wrote:
>> Yes, you want round_to_odd.  I suppose that's not valid for float128 right now?
> 
> roundAndPackFloat64()
> 
> as well as
> 
> roundAndPackFloat128()
> 
> support it.
> 
> It's not implemented for round_canonical(), round_to_int(),

These two are easy.  I think Alex and I didn't implement it in the float32/64
rewrite because we didn't have a user.  Now we will.

> roundAndPackInt32(), roundAndPackInt64(), roundAndPackUint64(),
> roundAndPackFloat32()

Right, these would be from the float128 side.  It shouldn't be too hard...


r~
David Hildenbrand Feb. 13, 2019, 12:49 p.m. | #4
On 12.02.19 20:56, Richard Henderson wrote:
> On 2/12/19 11:32 AM, David Hildenbrand wrote:
>>> Yes, you want round_to_odd.  I suppose that's not valid for float128 right now?
>>
>> roundAndPackFloat64()
>>
>> as well as
>>
>> roundAndPackFloat128()
>>
>> support it.
>>
>> It's not implemented for round_canonical(), round_to_int(),
> 
> These two are easy.  I think Alex and I didn't implement it in the float32/64
> rewrite because we didn't have a user.  Now we will.
> 
>> roundAndPackInt32(), roundAndPackInt64(), roundAndPackUint64(),
>> roundAndPackFloat32()
> 
> Right, these would be from the float128 side.  It shouldn't be too hard...
> 
> 
> r~
> 

Sounds like you are volunteering to implement them? ;)
Richard Henderson Feb. 13, 2019, 7:11 p.m. | #5
On 2/13/19 4:49 AM, David Hildenbrand wrote:
>>> It's not implemented for round_canonical(), round_to_int(),
>>
>> These two are easy.  I think Alex and I didn't implement it in the float32/64
>> rewrite because we didn't have a user.  Now we will.
>>
>>> roundAndPackInt32(), roundAndPackInt64(), roundAndPackUint64(),
>>> roundAndPackFloat32()
>>
>> Right, these would be from the float128 side.  It shouldn't be too hard...
>>
>>
>> r~
>>
> 
> Sounds like you are volunteering to implement them? ;)

Yep.


r~

Patch

diff --git a/target/s390x/fpu_helper.c b/target/s390x/fpu_helper.c
index 15ede530d8..a578f849ad 100644
--- a/target/s390x/fpu_helper.c
+++ b/target/s390x/fpu_helper.c
@@ -802,21 +802,33 @@  uint64_t HELPER(sqxb)(CPUS390XState *env, uint64_t ah, uint64_t al)
     return RET128(ret);
 }
 
-static const int fpc_to_rnd[4] = {
+static const int fpc_to_rnd[8] = {
     float_round_nearest_even,
     float_round_to_zero,
     float_round_up,
-    float_round_down
+    float_round_down,
+    -1,
+    -1,
+    -1,
+    /*
+     * FIXME: we actually want something like round_to_odd, but that does not
+     * support all data types yet.
+     */
+    float_round_to_zero,
 };
 
 /* set fpc */
 void HELPER(sfpc)(CPUS390XState *env, uint64_t fpc)
 {
+    if (fpc_to_rnd[fpc & 0x7] == -1 || fpc & 0x03030088u) {
+        s390_program_interrupt(env, PGM_SPECIFICATION, ILEN_AUTO, GETPC());
+    }
+
     /* Install everything in the main FPC.  */
     env->fpc = fpc;
 
     /* Install the rounding mode in the shadow fpu_status.  */
-    set_float_rounding_mode(fpc_to_rnd[fpc & 3], &env->fpu_status);
+    set_float_rounding_mode(fpc_to_rnd[fpc & 0x7], &env->fpu_status);
 }
 
 /* set fpc and signal */
@@ -825,12 +837,16 @@  void HELPER(sfas)(CPUS390XState *env, uint64_t fpc)
     uint32_t signalling = env->fpc;
     uint32_t s390_exc;
 
+    if (fpc_to_rnd[fpc & 0x7] == -1 || fpc & 0x03030088u) {
+        s390_program_interrupt(env, PGM_SPECIFICATION, ILEN_AUTO, GETPC());
+    }
+
     /*
      * FPC is set to the FPC operand with a bitwise OR of the signalling
      * flags.
      */
     env->fpc = fpc | (signalling & 0x00ff0000);
-    set_float_rounding_mode(fpc_to_rnd[fpc & 3], &env->fpu_status);
+    set_float_rounding_mode(fpc_to_rnd[fpc & 0x7], &env->fpu_status);
 
     /*
      * If any signaling flag is enabled in the new FPC mask, a
diff --git a/target/s390x/helper.h b/target/s390x/helper.h
index 018e9dd414..974f6bac2a 100644
--- a/target/s390x/helper.h
+++ b/target/s390x/helper.h
@@ -104,7 +104,7 @@  DEF_HELPER_4(trtr, i32, env, i32, i64, i64)
 DEF_HELPER_5(trXX, i32, env, i32, i32, i32, i32)
 DEF_HELPER_4(cksm, i64, env, i64, i64, i64)
 DEF_HELPER_FLAGS_5(calc_cc, TCG_CALL_NO_RWG_SE, i32, env, i32, i64, i64, i64)
-DEF_HELPER_FLAGS_2(sfpc, TCG_CALL_NO_RWG, void, env, i64)
+DEF_HELPER_FLAGS_2(sfpc, TCG_CALL_NO_WG, void, env, i64)
 DEF_HELPER_FLAGS_2(sfas, TCG_CALL_NO_WG, void, env, i64)
 DEF_HELPER_FLAGS_1(popcnt, TCG_CALL_NO_RWG_SE, i64, i64)
 DEF_HELPER_2(stfle, i32, env, i64)