[Qemu-devel] [V2 PATCH 0/8] target-ppc: Bug Fixes for 64 Bit FXU Instructions

[Qemu-devel] [V2 PATCH 0/8] target-ppc: Bug Fixes for 64 Bit FXU Instructions

[Tom Musta]

These patches fix assorted bugs in the emulation of Power Fixed Point Unit
instructions.

All instructions have been thorougly tested by running millions of random
patterns through actual hardware and comparing the results against QEMU.
The bugs all appear to be limited to 64 bit implementations.

V2: Added example data patterns to commit messages (no functional change from V1).

Tom Musta (8):
  target-ppc: Bug Fix: rlwinm
  target-ppc: Bug Fix: rlwnm
  target-ppc: Bug Fix: rlwimi
  target-ppc: Bug Fix: mullw
  target-ppc: Bug Fix: mullwo
  target-ppc: Bug Fix: mulldo OV Detection
  target-ppc: Bug Fix: srawi
  target-ppc: Bug Fix: srad

 target-ppc/int_helper.c |   16 ++++++++++--
 target-ppc/translate.c  |   60 +++++++++++++++++++++++++++++++---------------
 2 files changed, 53 insertions(+), 23 deletions(-)

[Qemu-devel] [V2 PATCH 1/8] target-ppc: Bug Fix: rlwinm

[Tom Musta]

The rlwinm specification includes the ROTL32 operation, which is defined
to be a left rotation of two copies of the least significant 32 bits of
the source GPR.

The current implementation is incorrect on 64-bit implementations in that
it rotates a single copy of the least significant 32 bits, padding with
zeroes in the most significant bits.

Fix the code to properly implement this ROTL32 operation.

Example:
R3 = F7487D82EC6F75DF
rlwinm 3,3,5,12,4

R3 expected : 8DEEBBFD880EBBFD
R3 actual   : 00000000880EBBFD (without this fix)

Signed-off-by: Tom Musta <tommusta@gmail.com>
---
 target-ppc/translate.c |    8 +++-----
 1 files changed, 3 insertions(+), 5 deletions(-)

diff --git a/target-ppc/translate.c b/target-ppc/translate.c
index b23933f..a27d063 100644
--- a/target-ppc/translate.c
+++ b/target-ppc/translate.c
@@ -1672,11 +1672,9 @@ static void gen_rlwinm(DisasContext *ctx)
     } else {
         TCGv t0 = tcg_temp_new();
 #if defined(TARGET_PPC64)
-        TCGv_i32 t1 = tcg_temp_new_i32();
-        tcg_gen_trunc_i64_i32(t1, cpu_gpr[rS(ctx->opcode)]);
-        tcg_gen_rotli_i32(t1, t1, sh);
-        tcg_gen_extu_i32_i64(t0, t1);
-        tcg_temp_free_i32(t1);
+        tcg_gen_deposit_i64(t0, cpu_gpr[rS(ctx->opcode)],
+            cpu_gpr[rS(ctx->opcode)], 32, 32);
+        tcg_gen_rotli_i64(t0, t0, sh);
 #else
         tcg_gen_rotli_i32(t0, cpu_gpr[rS(ctx->opcode)], sh);
 #endif
-- 
1.7.1

[Qemu-devel] [V2 PATCH 2/8] target-ppc: Bug Fix: rlwnm

[Tom Musta]

The rlwnm specification includes the ROTL32 operation, which is defined
to be a left rotation of two copies of the least significant 32 bits of
the source GPR.

The current implementation is incorrect on 64-bit implementations in that
it rotates a single copy of the least significant 32 bits, padding with
zeroes in the most significant bits.

Fix the code to properly implement this ROTL32 operation.

Example:

R3 = 0000000000000002
R4 = 7FFFFFFFFFFFFFFF
rlwnm 3,3,4,31,16
R3 expected : 0000000100000001
R3 actual   : 0000000000000001 (without this patch)

Signed-off-by: Tom Musta <tommusta@gmail.com>
---
 target-ppc/translate.c |   18 +++++++++---------
 1 files changed, 9 insertions(+), 9 deletions(-)

diff --git a/target-ppc/translate.c b/target-ppc/translate.c
index a27d063..48f13a9 100644
--- a/target-ppc/translate.c
+++ b/target-ppc/translate.c
@@ -1695,7 +1695,7 @@ static void gen_rlwnm(DisasContext *ctx)
     uint32_t mb, me;
     TCGv t0;
 #if defined(TARGET_PPC64)
-    TCGv_i32 t1, t2;
+    TCGv t1;
 #endif
 
     mb = MB(ctx->opcode);
@@ -1703,14 +1703,11 @@ static void gen_rlwnm(DisasContext *ctx)
     t0 = tcg_temp_new();
     tcg_gen_andi_tl(t0, cpu_gpr[rB(ctx->opcode)], 0x1f);
 #if defined(TARGET_PPC64)
-    t1 = tcg_temp_new_i32();
-    t2 = tcg_temp_new_i32();
-    tcg_gen_trunc_i64_i32(t1, cpu_gpr[rS(ctx->opcode)]);
-    tcg_gen_trunc_i64_i32(t2, t0);
-    tcg_gen_rotl_i32(t1, t1, t2);
-    tcg_gen_extu_i32_i64(t0, t1);
-    tcg_temp_free_i32(t1);
-    tcg_temp_free_i32(t2);
+    t1 = tcg_temp_new_i64();
+    tcg_gen_deposit_i64(t1, cpu_gpr[rS(ctx->opcode)],
+        cpu_gpr[rS(ctx->opcode)], 32, 32);
+    tcg_gen_rotl_i64(t0, t1, t0);
+    tcg_temp_free_i64(t1);
 #else
     tcg_gen_rotl_i32(t0, cpu_gpr[rS(ctx->opcode)], t0);
 #endif
@@ -1721,6 +1718,9 @@ static void gen_rlwnm(DisasContext *ctx)
 #endif
         tcg_gen_andi_tl(cpu_gpr[rA(ctx->opcode)], t0, MASK(mb, me));
     } else {
+#if defined(TARGET_PPC64)
+        tcg_gen_andi_tl(t0, t0, MASK(32, 63));
+#endif
         tcg_gen_mov_tl(cpu_gpr[rA(ctx->opcode)], t0);
     }
     tcg_temp_free(t0);
-- 
1.7.1

[Qemu-devel] [V2 PATCH 3/8] target-ppc: Bug Fix: rlwimi

[Tom Musta]

The rlwimi specification includes the ROTL32 operation, which is defined
to be a left rotation of two copies of the least significant 32 bits of
the source GPR.

The current implementation is incorrect on 64-bit implementations in that
it rotates a single copy of the least significant 32 bits, padding with
zeroes in the most significant bits.

Fix the code to properly implement this ROTL32 operation.

Also fix the special case of MB=31 and ME=0 to copy the entire contents
of the source GPR.

Examples:

R3 FFFFFFFFFFFFFFF0
rlwimi 3,3,29,14,1
R3 expected : 1FFFFFFE3FFFFFFE
R3 actual   : 000000003FFFFFFE (without this patch)

R3 ED7EB4DD824F0853
rlwimi 3,3,10,31,0
R3 expected : 3C214E09024F0853
R3 actual   : 00000000024F0853 (without this patch)

Signed-off-by: Tom Musta <tommusta@gmail.com>
---
 target-ppc/translate.c |   12 +++++++-----
 1 files changed, 7 insertions(+), 5 deletions(-)

diff --git a/target-ppc/translate.c b/target-ppc/translate.c
index 48f13a9..f4cc495 100644
--- a/target-ppc/translate.c
+++ b/target-ppc/translate.c
@@ -1614,17 +1614,19 @@ static void gen_rlwimi(DisasContext *ctx)
     me = ME(ctx->opcode);
     sh = SH(ctx->opcode);
     if (likely(sh == 0 && mb == 0 && me == 31)) {
+#if defined(TARGET_PPC64)
+        tcg_gen_mov_i64(cpu_gpr[rA(ctx->opcode)], cpu_gpr[rS(ctx->opcode)]);
+#else
         tcg_gen_ext32u_tl(cpu_gpr[rA(ctx->opcode)], cpu_gpr[rS(ctx->opcode)]);
+#endif
     } else {
         target_ulong mask;
         TCGv t1;
         TCGv t0 = tcg_temp_new();
 #if defined(TARGET_PPC64)
-        TCGv_i32 t2 = tcg_temp_new_i32();
-        tcg_gen_trunc_i64_i32(t2, cpu_gpr[rS(ctx->opcode)]);
-        tcg_gen_rotli_i32(t2, t2, sh);
-        tcg_gen_extu_i32_i64(t0, t2);
-        tcg_temp_free_i32(t2);
+        tcg_gen_deposit_i64(t0, cpu_gpr[rS(ctx->opcode)],
+            cpu_gpr[rS(ctx->opcode)], 32, 32);
+        tcg_gen_rotli_i64(t0, t0, sh);
 #else
         tcg_gen_rotli_i32(t0, cpu_gpr[rS(ctx->opcode)], sh);
 #endif
-- 
1.7.1

[Qemu-devel] [V2 PATCH 4/8] target-ppc: Bug Fix: mullw

[Tom Musta]

For 64-bit implementations, the mullw result is the 64 bit product
of the sign-extended least significant 32 bits of the source
registers.

Fix the code to properly sign extend the source operands and produce
a 64 bit product.

Example:
R3 00000000002F37A0
R4 41C33D242F816715
mullw 3,3,4
R3 expected : 0008C3146AE0F020
R3 actual   : 000000006AE0F020 (without this patch)

Signed-off-by: Tom Musta <tommusta@gmail.com>
---
 target-ppc/translate.c |   11 +++++++++++
 1 files changed, 11 insertions(+), 0 deletions(-)

diff --git a/target-ppc/translate.c b/target-ppc/translate.c
index f4cc495..41a5aea 100644
--- a/target-ppc/translate.c
+++ b/target-ppc/translate.c
@@ -1125,9 +1125,20 @@ static void gen_mulhwu(DisasContext *ctx)
 /* mullw  mullw. */
 static void gen_mullw(DisasContext *ctx)
 {
+#if defined(TARGET_PPC64)
+    TCGv_i64 t0, t1;
+    t0 = tcg_temp_new_i64();
+    t1 = tcg_temp_new_i64();
+    tcg_gen_ext32s_tl(t0, cpu_gpr[rA(ctx->opcode)]);
+    tcg_gen_ext32s_tl(t1, cpu_gpr[rB(ctx->opcode)]);
+    tcg_gen_mul_i64(cpu_gpr[rD(ctx->opcode)], t0, t1);
+    tcg_temp_free(t0);
+    tcg_temp_free(t1);
+#else
     tcg_gen_mul_tl(cpu_gpr[rD(ctx->opcode)], cpu_gpr[rA(ctx->opcode)],
                    cpu_gpr[rB(ctx->opcode)]);
     tcg_gen_ext32s_tl(cpu_gpr[rD(ctx->opcode)], cpu_gpr[rD(ctx->opcode)]);
+#endif
     if (unlikely(Rc(ctx->opcode) != 0))
         gen_set_Rc0(ctx, cpu_gpr[rD(ctx->opcode)]);
 }
-- 
1.7.1

[Qemu-devel] [V2 PATCH 5/8] target-ppc: Bug Fix: mullwo

[Tom Musta]

On 64-bit implementations, the mullwo result is the 64 bit product of
the signed 32 bit operands.  Fix the implementation to properly deposit
the upper 32 bits into the target register.

Example:

R3 0407DED115077586
R4 53778DF3CA992E09
mullwo 3,3,4
R3 expected : FB9D02730D7735B6
R3 actual   : 000000000D7735B6 (without this patch)

Signed-off-by: Tom Musta <tommusta@gmail.com>
---
 target-ppc/translate.c |    9 +++++++++
 1 files changed, 9 insertions(+), 0 deletions(-)

diff --git a/target-ppc/translate.c b/target-ppc/translate.c
index 41a5aea..4904665 100644
--- a/target-ppc/translate.c
+++ b/target-ppc/translate.c
@@ -1148,11 +1148,20 @@ static void gen_mullwo(DisasContext *ctx)
 {
     TCGv_i32 t0 = tcg_temp_new_i32();
     TCGv_i32 t1 = tcg_temp_new_i32();
+#if defined(TARGET_PPC64)
+    TCGv_i64 t2 = tcg_temp_new_i64();
+#endif
 
     tcg_gen_trunc_tl_i32(t0, cpu_gpr[rA(ctx->opcode)]);
     tcg_gen_trunc_tl_i32(t1, cpu_gpr[rB(ctx->opcode)]);
     tcg_gen_muls2_i32(t0, t1, t0, t1);
     tcg_gen_ext_i32_tl(cpu_gpr[rD(ctx->opcode)], t0);
+#if defined(TARGET_PPC64)
+    tcg_gen_ext_i32_tl(t2, t1);
+    tcg_gen_deposit_i64(cpu_gpr[rD(ctx->opcode)],
+                        cpu_gpr[rD(ctx->opcode)], t2, 32, 32);
+    tcg_temp_free(t2);
+#endif
 
     tcg_gen_sari_i32(t0, t0, 31);
     tcg_gen_setcond_i32(TCG_COND_NE, t0, t0, t1);
-- 
1.7.1

[Qemu-devel] [V2 PATCH 6/8] target-ppc: Bug Fix: mulldo OV Detection

[Tom Musta]

Fix the code to properly detect overflow; the 128 bit signed
product must have all zeroes or all ones in the first 65 bits
otherwise OV should be set.

Example:

R3 45F086A5D5887509
R4 0000000000000002
mulldo 3,3,4

Should set XER[OV].

Signed-off-by: Tom Musta <tommusta@gmail.com>
---
 target-ppc/int_helper.c |   14 ++++++++++++--
 1 files changed, 12 insertions(+), 2 deletions(-)

diff --git a/target-ppc/int_helper.c b/target-ppc/int_helper.c
index f6e8846..e83a25d 100644
--- a/target-ppc/int_helper.c
+++ b/target-ppc/int_helper.c
@@ -32,12 +32,22 @@ uint64_t helper_mulldo(CPUPPCState *env, uint64_t arg1, uint64_t arg2)
     uint64_t tl;
 
     muls64(&tl, (uint64_t *)&th, arg1, arg2);
-    /* If th != 0 && th != -1, then we had an overflow */
-    if (likely((uint64_t)(th + 1) <= 1)) {
+
+    /* th should either contain all 1 bits or all 0 bits and should
+     * match the sign bit of tl; otherwise we have overflowed. */
+
+    if ((int64_t)tl < 0) {
+        if (likely(th == -1LL)) {
+            env->ov = 0;
+        } else {
+            env->so = env->ov = 1;
+        }
+    } else if (likely(th == 0LL)) {
         env->ov = 0;
     } else {
         env->so = env->ov = 1;
     }
+
     return (int64_t)tl;
 }
 #endif
-- 
1.7.1

[Qemu-devel] [V2 PATCH 7/8] target-ppc: Bug Fix: srawi

[Tom Musta]

For 64 bit implementations, the special case of a shift by zero
should result in the sign extension of the least significant 32 bits
of the source GPR (not a direct copy of the 64 bit source GPR).

Example:

R3 A6212433228F41DC
srawi 3,3,0
R3 expected : 00000000228F41DC
R3 actual   : A6212433228F41DC (without this patch)

Signed-off-by: Tom Musta <tommusta@gmail.com>
---
 target-ppc/translate.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/target-ppc/translate.c b/target-ppc/translate.c
index 4904665..61fa42d 100644
--- a/target-ppc/translate.c
+++ b/target-ppc/translate.c
@@ -1941,7 +1941,7 @@ static void gen_srawi(DisasContext *ctx)
     TCGv dst = cpu_gpr[rA(ctx->opcode)];
     TCGv src = cpu_gpr[rS(ctx->opcode)];
     if (sh == 0) {
-        tcg_gen_mov_tl(dst, src);
+        tcg_gen_ext32s_tl(dst, src);
         tcg_gen_movi_tl(cpu_ca, 0);
     } else {
         TCGv t0;
-- 
1.7.1

[Qemu-devel] [V2 PATCH 8/8] target-ppc: Bug Fix: srad

[Tom Musta]

Fix the check for carry in the srad helper to properly construct
the mask -- a "1ULL" must be used (instead of "1") in order to
get the desired result.

Example:

R3 8000000000000000
R4 F3511AD4A2CD4C38
srad 3,3,4

Should *not* set XER[CA] but does without this patch.

Signed-off-by: Tom Musta <tommusta@gmail.com>
---
 target-ppc/int_helper.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/target-ppc/int_helper.c b/target-ppc/int_helper.c
index e83a25d..e5b103b 100644
--- a/target-ppc/int_helper.c
+++ b/target-ppc/int_helper.c
@@ -248,7 +248,7 @@ target_ulong helper_srad(CPUPPCState *env, target_ulong value,
         if (likely((uint64_t)shift != 0)) {
             shift &= 0x3f;
             ret = (int64_t)value >> shift;
-            if (likely(ret >= 0 || (value & ((1 << shift) - 1)) == 0)) {
+            if (likely(ret >= 0 || (value & ((1ULL << shift) - 1)) == 0)) {
                 env->ca = 0;
             } else {
                 env->ca = 1;
-- 
1.7.1

Re: [Qemu-devel] [V2 PATCH 0/8] target-ppc: Bug Fixes for 64 Bit FXU Instructions

[Alexander Graf]

On 12.08.14 15:45, Tom Musta wrote:
> These patches fix assorted bugs in the emulation of Power Fixed Point Unit
> instructions.
>
> All instructions have been thorougly tested by running millions of random
> patterns through actual hardware and comparing the results against QEMU.
> The bugs all appear to be limited to 64 bit implementations.
>
> V2: Added example data patterns to commit messages (no functional change from V1).

Thanks, applied all to ppc-next.


Alex

Re: [Qemu-devel] [V2 PATCH 1/8] target-ppc: Bug Fix: rlwinm

[Richard Henderson]

On 08/12/2014 03:45 AM, Tom Musta wrote:
> The rlwinm specification includes the ROTL32 operation, which is defined
> to be a left rotation of two copies of the least significant 32 bits of
> the source GPR.
> 
> The current implementation is incorrect on 64-bit implementations in that
> it rotates a single copy of the least significant 32 bits, padding with
> zeroes in the most significant bits.
> 
> Fix the code to properly implement this ROTL32 operation.
> 
> Example:
> R3 = F7487D82EC6F75DF
> rlwinm 3,3,5,12,4
> 
> R3 expected : 8DEEBBFD880EBBFD
> R3 actual   : 00000000880EBBFD (without this fix)
> 
> Signed-off-by: Tom Musta <tommusta@gmail.com>
> ---
>  target-ppc/translate.c |    8 +++-----
>  1 files changed, 3 insertions(+), 5 deletions(-)
> 
> diff --git a/target-ppc/translate.c b/target-ppc/translate.c
> index b23933f..a27d063 100644
> --- a/target-ppc/translate.c
> +++ b/target-ppc/translate.c
> @@ -1672,11 +1672,9 @@ static void gen_rlwinm(DisasContext *ctx)
>      } else {
>          TCGv t0 = tcg_temp_new();
>  #if defined(TARGET_PPC64)
> -        TCGv_i32 t1 = tcg_temp_new_i32();
> -        tcg_gen_trunc_i64_i32(t1, cpu_gpr[rS(ctx->opcode)]);
> -        tcg_gen_rotli_i32(t1, t1, sh);
> -        tcg_gen_extu_i32_i64(t0, t1);
> -        tcg_temp_free_i32(t1);
> +        tcg_gen_deposit_i64(t0, cpu_gpr[rS(ctx->opcode)],
> +            cpu_gpr[rS(ctx->opcode)], 32, 32);
> +        tcg_gen_rotli_i64(t0, t0, sh);
>  #else
>          tcg_gen_rotli_i32(t0, cpu_gpr[rS(ctx->opcode)], sh);
>  #endif
> 

You might want to retain a special case for mb==0 and me==31 which would be a
straight 32-bit rotate with subsequent zero-extension.  Just like we've got
special cases for the normal shifts.


r~

Re: [Qemu-devel] [V2 PATCH 2/8] target-ppc: Bug Fix: rlwnm

[Richard Henderson]

On 08/12/2014 03:45 AM, Tom Musta wrote:
> The rlwnm specification includes the ROTL32 operation, which is defined
> to be a left rotation of two copies of the least significant 32 bits of
> the source GPR.
> 
> The current implementation is incorrect on 64-bit implementations in that
> it rotates a single copy of the least significant 32 bits, padding with
> zeroes in the most significant bits.
> 
> Fix the code to properly implement this ROTL32 operation.
> 
> Example:
> 
> R3 = 0000000000000002
> R4 = 7FFFFFFFFFFFFFFF
> rlwnm 3,3,4,31,16
> R3 expected : 0000000100000001
> R3 actual   : 0000000000000001 (without this patch)
> 
> Signed-off-by: Tom Musta <tommusta@gmail.com>
> ---
>  target-ppc/translate.c |   18 +++++++++---------
>  1 files changed, 9 insertions(+), 9 deletions(-)
> 
> diff --git a/target-ppc/translate.c b/target-ppc/translate.c
> index a27d063..48f13a9 100644
> --- a/target-ppc/translate.c
> +++ b/target-ppc/translate.c
> @@ -1695,7 +1695,7 @@ static void gen_rlwnm(DisasContext *ctx)
>      uint32_t mb, me;
>      TCGv t0;
>  #if defined(TARGET_PPC64)
> -    TCGv_i32 t1, t2;
> +    TCGv t1;
>  #endif
>  
>      mb = MB(ctx->opcode);
> @@ -1703,14 +1703,11 @@ static void gen_rlwnm(DisasContext *ctx)
>      t0 = tcg_temp_new();
>      tcg_gen_andi_tl(t0, cpu_gpr[rB(ctx->opcode)], 0x1f);
>  #if defined(TARGET_PPC64)
> -    t1 = tcg_temp_new_i32();
> -    t2 = tcg_temp_new_i32();
> -    tcg_gen_trunc_i64_i32(t1, cpu_gpr[rS(ctx->opcode)]);
> -    tcg_gen_trunc_i64_i32(t2, t0);
> -    tcg_gen_rotl_i32(t1, t1, t2);
> -    tcg_gen_extu_i32_i64(t0, t1);
> -    tcg_temp_free_i32(t1);
> -    tcg_temp_free_i32(t2);
> +    t1 = tcg_temp_new_i64();
> +    tcg_gen_deposit_i64(t1, cpu_gpr[rS(ctx->opcode)],
> +        cpu_gpr[rS(ctx->opcode)], 32, 32);
> +    tcg_gen_rotl_i64(t0, t1, t0);
> +    tcg_temp_free_i64(t1);
>  #else
>      tcg_gen_rotl_i32(t0, cpu_gpr[rS(ctx->opcode)], t0);
>  #endif
> @@ -1721,6 +1718,9 @@ static void gen_rlwnm(DisasContext *ctx)
>  #endif
>          tcg_gen_andi_tl(cpu_gpr[rA(ctx->opcode)], t0, MASK(mb, me));
>      } else {
> +#if defined(TARGET_PPC64)
> +        tcg_gen_andi_tl(t0, t0, MASK(32, 63));
> +#endif

Similarly, checking early for the MB==0, ME==31 case and generating a straight
32-bit rotate would be good.  Note that ANDI will already special case for the
constant -1, so there's no need to check for that as a mov.


r~

Re: [Qemu-devel] [V2 PATCH 4/8] target-ppc: Bug Fix: mullw

[Richard Henderson]

On 08/12/2014 03:45 AM, Tom Musta wrote:
> +#else
>      tcg_gen_mul_tl(cpu_gpr[rD(ctx->opcode)], cpu_gpr[rA(ctx->opcode)],
>                     cpu_gpr[rB(ctx->opcode)]);
>      tcg_gen_ext32s_tl(cpu_gpr[rD(ctx->opcode)], cpu_gpr[rD(ctx->opcode)]);
> +#endif

Note that the sign-extension can be dropped, since this is ifdef PPC32.
Perhaps even better to use tcg_gen_mul_i32 explicitly?  That at least
type-checks the arguments are all 32-bit as well.


r~

Re: [Qemu-devel] [V2 PATCH 7/8] target-ppc: Bug Fix: srawi

[Richard Henderson]

On 08/12/2014 03:45 AM, Tom Musta wrote:
> For 64 bit implementations, the special case of a shift by zero
> should result in the sign extension of the least significant 32 bits
> of the source GPR (not a direct copy of the 64 bit source GPR).
> 
> Example:
> 
> R3 A6212433228F41DC
> srawi 3,3,0
> R3 expected : 00000000228F41DC
> R3 actual   : A6212433228F41DC (without this patch)

Reviewed-by: Richard Henderson <rth@twiddle.net>


r~