[Qemu-devel] [PATCH 0/3] Add and use bit rotate functions

[Qemu-devel] [PATCH 0/3] Add and use bit rotate functions

[Stefan Weil]

The first patch was already sent to qemu-devel and is only included
here because patch 3 is based on it. Only patch 1 is needed for qemu-stable.

It looks like shift values of 0 or 32/64 work as expected because
the compiler "knows" the pattern used to implement the rotate operation,
so the code does not need special handling of some shift values.

rol8 and ror8 are currently unused. I added them nevertheless,
so they can be used when someone needs them.

[PATCH 1/3] tci: Add implementation of rotl_i64, rotr_i64
[PATCH 2/3] bitops: Add rotate functions (rol8, ror8, ...)
[PATCH 3/3] misc: Use new rotate functions

[Qemu-devel] [PATCH 1/3] tci: Add implementation of rotl_i64, rotr_i64

[Stefan Weil]

It is used by qemu-ppc64 when running Debian's busybox-static.

Cc: qemu-stable <qemu-stable@nongnu.org>
Signed-off-by: Stefan Weil <sw@weilnetz.de>
Reviewed-by: Richard Henderson <rth@twiddle.net>
---
 tcg/tci/tcg-target.c |    1 -
 tci.c                |   10 +++++++++-
 2 files changed, 9 insertions(+), 2 deletions(-)

diff --git a/tcg/tci/tcg-target.c b/tcg/tci/tcg-target.c
index 233ab3b..4976bec 100644
--- a/tcg/tci/tcg-target.c
+++ b/tcg/tci/tcg-target.c
@@ -670,7 +670,6 @@ static void tcg_out_op(TCGContext *s, TCGOpcode opc, const TCGArg *args,
     case INDEX_op_shl_i64:
     case INDEX_op_shr_i64:
     case INDEX_op_sar_i64:
-        /* TODO: Implementation of rotl_i64, rotr_i64 missing in tci.c. */
     case INDEX_op_rotl_i64:     /* Optional (TCG_TARGET_HAS_rot_i64). */
     case INDEX_op_rotr_i64:     /* Optional (TCG_TARGET_HAS_rot_i64). */
         tcg_out_r(s, args[0]);
diff --git a/tci.c b/tci.c
index 18c888e..94b7851 100644
--- a/tci.c
+++ b/tci.c
@@ -952,8 +952,16 @@ uintptr_t tcg_qemu_tb_exec(CPUArchState *env, uint8_t *tb_ptr)
             break;
 #if TCG_TARGET_HAS_rot_i64
         case INDEX_op_rotl_i64:
+            t0 = *tb_ptr++;
+            t1 = tci_read_ri64(&tb_ptr);
+            t2 = tci_read_ri64(&tb_ptr);
+            tci_write_reg64(t0, (t1 << t2) | (t1 >> (64 - t2)));
+            break;
         case INDEX_op_rotr_i64:
-            TODO();
+            t0 = *tb_ptr++;
+            t1 = tci_read_ri64(&tb_ptr);
+            t2 = tci_read_ri64(&tb_ptr);
+            tci_write_reg64(t0, (t1 >> t2) | (t1 << (64 - t2)));
             break;
 #endif
 #if TCG_TARGET_HAS_deposit_i64
-- 
1.7.10.4

[Qemu-devel] [PATCH 2/3] bitops: Add rotate functions (rol8, ror8, ...)

[Stefan Weil]

These functions were copies from include/linux/bitopts.h.

Signed-off-by: Stefan Weil <sw@weilnetz.de>
---
 include/qemu/bitops.h |   82 +++++++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 82 insertions(+)

diff --git a/include/qemu/bitops.h b/include/qemu/bitops.h
index 06e2e6f..5c0fa0a 100644
--- a/include/qemu/bitops.h
+++ b/include/qemu/bitops.h
@@ -184,6 +184,86 @@ static inline unsigned long hweight_long(unsigned long w)
 }
 
 /**
+ * rol8 - rotate an 8-bit value left
+ * @word: value to rotate
+ * @shift: bits to roll
+ */
+static inline uint8_t rol8(uint8_t word, unsigned int shift)
+{
+    return (word << shift) | (word >> (8 - shift));
+}
+
+/**
+ * ror8 - rotate an 8-bit value right
+ * @word: value to rotate
+ * @shift: bits to roll
+ */
+static inline uint8_t ror8(uint8_t word, unsigned int shift)
+{
+    return (word >> shift) | (word << (8 - shift));
+}
+
+/**
+ * rol16 - rotate a 16-bit value left
+ * @word: value to rotate
+ * @shift: bits to roll
+ */
+static inline uint16_t rol16(uint16_t word, unsigned int shift)
+{
+    return (word << shift) | (word >> (16 - shift));
+}
+
+/**
+ * ror16 - rotate a 16-bit value right
+ * @word: value to rotate
+ * @shift: bits to roll
+ */
+static inline uint16_t ror16(uint16_t word, unsigned int shift)
+{
+    return (word >> shift) | (word << (16 - shift));
+}
+
+/**
+ * rol32 - rotate a 32-bit value left
+ * @word: value to rotate
+ * @shift: bits to roll
+ */
+static inline uint32_t rol32(uint32_t word, unsigned int shift)
+{
+    return (word << shift) | (word >> (32 - shift));
+}
+
+/**
+ * ror32 - rotate a 32-bit value right
+ * @word: value to rotate
+ * @shift: bits to roll
+ */
+static inline uint32_t ror32(uint32_t word, unsigned int shift)
+{
+    return (word >> shift) | (word << (32 - shift));
+}
+
+/**
+ * rol64 - rotate a 64-bit value left
+ * @word: value to rotate
+ * @shift: bits to roll
+ */
+static inline uint64_t rol64(uint64_t word, unsigned int shift)
+{
+    return (word << shift) | (word >> (64 - shift));
+}
+
+/**
+ * ror64 - rotate a 64-bit value right
+ * @word: value to rotate
+ * @shift: bits to roll
+ */
+static inline uint64_t ror64(uint64_t word, unsigned int shift)
+{
+    return (word >> shift) | (word << (64 - shift));
+}
+
+/**
  * extract32:
  * @value: the value to extract the bit field from
  * @start: the lowest bit in the bit field (numbered from 0)
-- 
1.7.10.4

[Qemu-devel] [PATCH 3/3] misc: Use new rotate functions

[Stefan Weil]

Signed-off-by: Stefan Weil <sw@weilnetz.de>
---
 target-arm/iwmmxt_helper.c |    2 +-
 tcg/optimize.c             |   12 ++++--------
 tci.c                      |    8 ++++----
 3 files changed, 9 insertions(+), 13 deletions(-)

diff --git a/target-arm/iwmmxt_helper.c b/target-arm/iwmmxt_helper.c
index 7953b53..e6cfa62 100644
--- a/target-arm/iwmmxt_helper.c
+++ b/target-arm/iwmmxt_helper.c
@@ -577,7 +577,7 @@ uint64_t HELPER(iwmmxt_rorl)(CPUARMState *env, uint64_t x, uint32_t n)
 
 uint64_t HELPER(iwmmxt_rorq)(CPUARMState *env, uint64_t x, uint32_t n)
 {
-    x = (x >> n) | (x << (64 - n));
+    x = ror64(x, n);
     env->iwmmxt.cregs[ARM_IWMMXT_wCASF] = NZBIT64(x);
     return x;
 }
diff --git a/tcg/optimize.c b/tcg/optimize.c
index b29bf25..89e2d6a 100644
--- a/tcg/optimize.c
+++ b/tcg/optimize.c
@@ -238,20 +238,16 @@ static TCGArg do_constant_folding_2(TCGOpcode op, TCGArg x, TCGArg y)
         return (int64_t)x >> (int64_t)y;
 
     case INDEX_op_rotr_i32:
-        x = ((uint32_t)x << (32 - y)) | ((uint32_t)x >> y);
-        return x;
+        return ror32(x, y);
 
     case INDEX_op_rotr_i64:
-        x = ((uint64_t)x << (64 - y)) | ((uint64_t)x >> y);
-        return x;
+        return ror64(x, y);
 
     case INDEX_op_rotl_i32:
-        x = ((uint32_t)x << y) | ((uint32_t)x >> (32 - y));
-        return x;
+        return rol32(x, y);
 
     case INDEX_op_rotl_i64:
-        x = ((uint64_t)x << y) | ((uint64_t)x >> (64 - y));
-        return x;
+        return rol64(x, y);
 
     CASE_OP_32_64(not):
         return ~x;
diff --git a/tci.c b/tci.c
index 94b7851..cea7234 100644
--- a/tci.c
+++ b/tci.c
@@ -688,13 +688,13 @@ uintptr_t tcg_qemu_tb_exec(CPUArchState *env, uint8_t *tb_ptr)
             t0 = *tb_ptr++;
             t1 = tci_read_ri32(&tb_ptr);
             t2 = tci_read_ri32(&tb_ptr);
-            tci_write_reg32(t0, (t1 << t2) | (t1 >> (32 - t2)));
+            tci_write_reg32(t0, rol32(t1, t2));
             break;
         case INDEX_op_rotr_i32:
             t0 = *tb_ptr++;
             t1 = tci_read_ri32(&tb_ptr);
             t2 = tci_read_ri32(&tb_ptr);
-            tci_write_reg32(t0, (t1 >> t2) | (t1 << (32 - t2)));
+            tci_write_reg32(t0, ror32(t1, t2));
             break;
 #endif
 #if TCG_TARGET_HAS_deposit_i32
@@ -955,13 +955,13 @@ uintptr_t tcg_qemu_tb_exec(CPUArchState *env, uint8_t *tb_ptr)
             t0 = *tb_ptr++;
             t1 = tci_read_ri64(&tb_ptr);
             t2 = tci_read_ri64(&tb_ptr);
-            tci_write_reg64(t0, (t1 << t2) | (t1 >> (64 - t2)));
+            tci_write_reg64(t0, rol64(t1, t2));
             break;
         case INDEX_op_rotr_i64:
             t0 = *tb_ptr++;
             t1 = tci_read_ri64(&tb_ptr);
             t2 = tci_read_ri64(&tb_ptr);
-            tci_write_reg64(t0, (t1 >> t2) | (t1 << (64 - t2)));
+            tci_write_reg64(t0, ror64(t1, t2));
             break;
 #endif
 #if TCG_TARGET_HAS_deposit_i64
-- 
1.7.10.4

Re: [Qemu-devel] [PATCH 0/3] Add and use bit rotate functions

[Richard Henderson]

On 09/12/2013 12:13 PM, Stefan Weil wrote:
> The first patch was already sent to qemu-devel and is only included
> here because patch 3 is based on it. Only patch 1 is needed for qemu-stable.
> 
> It looks like shift values of 0 or 32/64 work as expected because
> the compiler "knows" the pattern used to implement the rotate operation,
> so the code does not need special handling of some shift values.

Thinking about this closer, the only two behaviors I know of for
x >> n, n >= w, w = width of x, are: shift modulo w, or zero.

Both cases work for this usage:

   x << 0 | x >> 32
   = x | (x >> 0)	modulo
   = x | x
   = x

   = x | 0		zero
   = x

AFAIK we never actually observe missile launch as a side effect
of an out of range shift.  And with that in mind, I think the
existing implementation of rotate is fine.


r~

Re: [Qemu-devel] [PATCH 2/3] bitops: Add rotate functions (rol8, ror8, ...)

[Richard Henderson]

On 09/12/2013 12:13 PM, Stefan Weil wrote:
> These functions were copies from include/linux/bitopts.h.
> 
> Signed-off-by: Stefan Weil <sw@weilnetz.de>
> ---
>  include/qemu/bitops.h |   82 +++++++++++++++++++++++++++++++++++++++++++++++++
>  1 file changed, 82 insertions(+)

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


r~

Re: [Qemu-devel] [PATCH 3/3] misc: Use new rotate functions

[Richard Henderson]

On 09/12/2013 12:13 PM, Stefan Weil wrote:
> Signed-off-by: Stefan Weil <sw@weilnetz.de>
> ---
>  target-arm/iwmmxt_helper.c |    2 +-
>  tcg/optimize.c             |   12 ++++--------
>  tci.c                      |    8 ++++----
>  3 files changed, 9 insertions(+), 13 deletions(-)

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


r~