[PATCH v2 0/8] tcg: Document *swap* helper implementations

[PATCH v2 0/8] tcg: Document *swap* helper implementations

[Philippe Mathieu-Daudé]

Since v1 [*]:
- Split few patches
- Dropped 'document deposit_z()'
- Document TCG_BSWAP_IZ flag (rth)
- Added R-b tags

[*] https://lore.kernel.org/qemu-devel/20230822093712.38922-1-philmd@linaro.org/

Supersedes: <20230822093712.38922-1-philmd@linaro.org>

Philippe Mathieu-Daudé (8):
  tcg/tcg-op: Document bswap16_i32() byte pattern
  tcg/tcg-op: Document bswap16_i64() byte pattern
  tcg/tcg-op: Document bswap32_i32() byte pattern
  tcg/tcg-op: Document bswap32_i64() byte pattern
  tcg/tcg-op: Document bswap64_i64() byte pattern
  tcg/tcg-op: Document hswap_i32/64() byte pattern
  tcg/tcg-op: Document wswap_i64() byte pattern
  target/cris: Fix a typo in gen_swapr()

 target/cris/translate.c |  20 ++++---
 tcg/tcg-op.c            | 124 +++++++++++++++++++++++++++++++---------
 2 files changed, 107 insertions(+), 37 deletions(-)

-- 
2.41.0

[PATCH v2 1/8] tcg/tcg-op: Document bswap16_i32() byte pattern

[Philippe Mathieu-Daudé]

Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
---
 tcg/tcg-op.c | 32 ++++++++++++++++++++++++--------
 1 file changed, 24 insertions(+), 8 deletions(-)

diff --git a/tcg/tcg-op.c b/tcg/tcg-op.c
index 7aadb37756..c1b9a3e34c 100644
--- a/tcg/tcg-op.c
+++ b/tcg/tcg-op.c
@@ -1021,6 +1021,15 @@ void tcg_gen_ext16u_i32(TCGv_i32 ret, TCGv_i32 arg)
     }
 }
 
+/*
+ * bswap16_i32: 16-bit byte swap on the low bits of a 32-bit value.
+ *
+ *                                                           flags
+ * Byte pattern:  bswap16_i32(..ab) -> .aba             (TCG_BSWAP_IZ)
+ *                bswap16_i32(xxab) -> ..ba             (TCG_BSWAP_OZ)
+ *                bswap16_i32(xxab) -> ssba             (TCG_BSWAP_OS)
+ *                bswap16_i32(xxab) -> xaba
+ */
 void tcg_gen_bswap16_i32(TCGv_i32 ret, TCGv_i32 arg, int flags)
 {
     /* Only one extension flag may be present. */
@@ -1032,22 +1041,29 @@ void tcg_gen_bswap16_i32(TCGv_i32 ret, TCGv_i32 arg, int flags)
         TCGv_i32 t0 = tcg_temp_ebb_new_i32();
         TCGv_i32 t1 = tcg_temp_ebb_new_i32();
 
-        tcg_gen_shri_i32(t0, arg, 8);
+                                            /* arg = xxab (IZ=0)    */
+                                            /*     = ..ab (IZ=1)    */
+        tcg_gen_shri_i32(t0, arg, 8);       /*  t0 = .xxa (IZ=0)    */
+                                            /*     = ...a (IZ=1)    */
         if (!(flags & TCG_BSWAP_IZ)) {
-            tcg_gen_ext8u_i32(t0, t0);
+            tcg_gen_ext8u_i32(t0, t0);      /*  t0 = ...a (IZ=0)    */
         }
 
         if (flags & TCG_BSWAP_OS) {
-            tcg_gen_shli_i32(t1, arg, 24);
-            tcg_gen_sari_i32(t1, t1, 16);
+            tcg_gen_shli_i32(t1, arg, 24);  /*  t1 = b... (OS=1)    */
+            tcg_gen_sari_i32(t1, t1, 16);   /*  t1 = ssb. (OS=1)    */
         } else if (flags & TCG_BSWAP_OZ) {
-            tcg_gen_ext8u_i32(t1, arg);
-            tcg_gen_shli_i32(t1, t1, 8);
+            tcg_gen_ext8u_i32(t1, arg);     /*  t1 = ...b (OZ=1)    */
+            tcg_gen_shli_i32(t1, t1, 8);    /*  t1 = ..b. (OZ=1)    */
         } else {
-            tcg_gen_shli_i32(t1, arg, 8);
+            tcg_gen_shli_i32(t1, arg, 8);   /*  t1 = xab. (IZ=0)    */
+                                            /*     = .ab. (IZ=1)    */
         }
 
-        tcg_gen_or_i32(ret, t0, t1);
+        tcg_gen_or_i32(ret, t0, t1);        /* ret = ..ba (OZ=1)    */
+                                            /*     = ssba (OS=1)    */
+                                            /*     = .aba (IZ=1)    */
+                                            /*     = xaba (no flag) */
         tcg_temp_free_i32(t0);
         tcg_temp_free_i32(t1);
     }
-- 
2.41.0

[PATCH v2 2/8] tcg/tcg-op: Document bswap16_i64() byte pattern

[Philippe Mathieu-Daudé]

Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
---
 tcg/tcg-op.c | 31 +++++++++++++++++++++++--------
 1 file changed, 23 insertions(+), 8 deletions(-)

diff --git a/tcg/tcg-op.c b/tcg/tcg-op.c
index c1b9a3e34c..80ec6f01c4 100644
--- a/tcg/tcg-op.c
+++ b/tcg/tcg-op.c
@@ -1737,6 +1737,15 @@ void tcg_gen_ext32u_i64(TCGv_i64 ret, TCGv_i64 arg)
     }
 }
 
+/*
+ * bswap16_i64: 16-bit byte swap on the low bits of a 64-bit value.
+ *
+ *                                                           flags
+ * Byte pattern:  bswap16_i32(......ab) -> ......ba     (TCG_BSWAP_IZ)
+ *                bswap16_i32(xxxxxxab) -> ......ba     (TCG_BSWAP_OZ)
+ *                bswap16_i32(xxxxxxab) -> ssssssba     (TCG_BSWAP_OS)
+ *                bswap16_i32(xxxxxxab) -> xxxxxxba
+ */
 void tcg_gen_bswap16_i64(TCGv_i64 ret, TCGv_i64 arg, int flags)
 {
     /* Only one extension flag may be present. */
@@ -1755,22 +1764,28 @@ void tcg_gen_bswap16_i64(TCGv_i64 ret, TCGv_i64 arg, int flags)
         TCGv_i64 t0 = tcg_temp_ebb_new_i64();
         TCGv_i64 t1 = tcg_temp_ebb_new_i64();
 
-        tcg_gen_shri_i64(t0, arg, 8);
+                                            /* arg = xxxxxxab (IZ=0) */
+                                            /*     = ......ab (IZ=1) */
+        tcg_gen_shri_i64(t0, arg, 8);       /*  t0 = .xxxxxxa (IZ=0) */
+                                            /*     = .......a (IZ=1) */
         if (!(flags & TCG_BSWAP_IZ)) {
-            tcg_gen_ext8u_i64(t0, t0);
+            tcg_gen_ext8u_i64(t0, t0);      /*  t0 = .......a (IZ=0) */
         }
 
         if (flags & TCG_BSWAP_OS) {
-            tcg_gen_shli_i64(t1, arg, 56);
-            tcg_gen_sari_i64(t1, t1, 48);
+            tcg_gen_shli_i64(t1, arg, 56);  /*  t1 = b....... (OS=1) */
+            tcg_gen_sari_i64(t1, t1, 48);   /*  t1 = ssssssb. (OS=1) */
         } else if (flags & TCG_BSWAP_OZ) {
-            tcg_gen_ext8u_i64(t1, arg);
-            tcg_gen_shli_i64(t1, t1, 8);
+            tcg_gen_ext8u_i64(t1, arg);     /*  t1 = .......b (OZ=1) */
+            tcg_gen_shli_i64(t1, t1, 8);    /*  t1 = ......b. (OZ=1) */
         } else {
-            tcg_gen_shli_i64(t1, arg, 8);
+            tcg_gen_shli_i64(t1, arg, 8);   /*  t1 = xxxxxxb. (IZ=0) */
+                                            /*       ......b. (IZ=1) */
         }
 
-        tcg_gen_or_i64(ret, t0, t1);
+        tcg_gen_or_i64(ret, t0, t1);        /* ret = ......ba (IZ=1 or OZ=1) */
+                                            /*       ssssssba (OS=1)    */
+                                            /*       xxxxxxba (no flag) */
         tcg_temp_free_i64(t0);
         tcg_temp_free_i64(t1);
     }
-- 
2.41.0

[PATCH v2 3/8] tcg/tcg-op: Document bswap32_i32() byte pattern

[Philippe Mathieu-Daudé]

Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
---
 tcg/tcg-op.c | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/tcg/tcg-op.c b/tcg/tcg-op.c
index 80ec6f01c4..fc9b21d30c 100644
--- a/tcg/tcg-op.c
+++ b/tcg/tcg-op.c
@@ -1069,6 +1069,11 @@ void tcg_gen_bswap16_i32(TCGv_i32 ret, TCGv_i32 arg, int flags)
     }
 }
 
+/*
+ * bswap32_i32: 32-bit byte swap on a 32-bit value.
+ *
+ * Byte pattern:  bswap32_i32(abcd) -> dcba
+ */
 void tcg_gen_bswap32_i32(TCGv_i32 ret, TCGv_i32 arg)
 {
     if (TCG_TARGET_HAS_bswap32_i32) {
-- 
2.41.0

[PATCH v2 4/8] tcg/tcg-op: Document bswap32_i64() byte pattern

[Philippe Mathieu-Daudé]

Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
---
 tcg/tcg-op.c | 20 +++++++++++++++-----
 1 file changed, 15 insertions(+), 5 deletions(-)

diff --git a/tcg/tcg-op.c b/tcg/tcg-op.c
index fc9b21d30c..e6e62c4278 100644
--- a/tcg/tcg-op.c
+++ b/tcg/tcg-op.c
@@ -1796,6 +1796,13 @@ void tcg_gen_bswap16_i64(TCGv_i64 ret, TCGv_i64 arg, int flags)
     }
 }
 
+/*
+ * bswap32_i64: 32-bit byte swap on the low bits of a 64-bit value.
+ *
+ *                                                          flags
+ * Byte pattern:  bswap32_i64(xxxxabcd) -> ssssdcba     (TCG_BSWAP_OS)
+ *                bswap32_i64(xxxxabcd) -> ....dcba
+ */
 void tcg_gen_bswap32_i64(TCGv_i64 ret, TCGv_i64 arg, int flags)
 {
     /* Only one extension flag may be present. */
@@ -1815,8 +1822,10 @@ void tcg_gen_bswap32_i64(TCGv_i64 ret, TCGv_i64 arg, int flags)
         TCGv_i64 t1 = tcg_temp_ebb_new_i64();
         TCGv_i64 t2 = tcg_constant_i64(0x00ff00ff);
 
-                                            /* arg = xxxxabcd */
-        tcg_gen_shri_i64(t0, arg, 8);       /*  t0 = .xxxxabc */
+                                            /* arg = xxxxabcd (IZ=0) */
+                                            /*       ....abcd (IZ=1) */
+        tcg_gen_shri_i64(t0, arg, 8);       /*  t0 = .xxxxabc (IZ=0) */
+                                            /*       .....abc (IZ=1) */
         tcg_gen_and_i64(t1, arg, t2);       /*  t1 = .....b.d */
         tcg_gen_and_i64(t0, t0, t2);        /*  t0 = .....a.c */
         tcg_gen_shli_i64(t1, t1, 8);        /*  t1 = ....b.d. */
@@ -1825,11 +1834,12 @@ void tcg_gen_bswap32_i64(TCGv_i64 ret, TCGv_i64 arg, int flags)
         tcg_gen_shli_i64(t1, ret, 48);      /*  t1 = dc...... */
         tcg_gen_shri_i64(t0, ret, 16);      /*  t0 = ......ba */
         if (flags & TCG_BSWAP_OS) {
-            tcg_gen_sari_i64(t1, t1, 32);   /*  t1 = ssssdc.. */
+            tcg_gen_sari_i64(t1, t1, 32);   /*  t1 = ssssdc.. (OS=1) */
         } else {
-            tcg_gen_shri_i64(t1, t1, 32);   /*  t1 = ....dc.. */
+            tcg_gen_shri_i64(t1, t1, 32);   /*  t1 = ....dc.. (OS=0) */
         }
-        tcg_gen_or_i64(ret, t0, t1);        /* ret = ssssdcba */
+        tcg_gen_or_i64(ret, t0, t1);        /* ret = ssssdcba (OS=1) */
+                                            /*       ....dcba (OS=0) */
 
         tcg_temp_free_i64(t0);
         tcg_temp_free_i64(t1);
-- 
2.41.0

[PATCH v2 5/8] tcg/tcg-op: Document bswap64_i64() byte pattern

[Philippe Mathieu-Daudé]

Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
Reviewed-by: Richard Henderson <richard.henderson@linaro.org>
---
 tcg/tcg-op.c | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/tcg/tcg-op.c b/tcg/tcg-op.c
index e6e62c4278..45290cccad 100644
--- a/tcg/tcg-op.c
+++ b/tcg/tcg-op.c
@@ -1846,6 +1846,11 @@ void tcg_gen_bswap32_i64(TCGv_i64 ret, TCGv_i64 arg, int flags)
     }
 }
 
+/*
+ * bswap64_i64: 64-bit byte swap on a 64-bit value.
+ *
+ * Byte pattern:  bswap64_i64(abcdefgh) -> hgfedcba
+ */
 void tcg_gen_bswap64_i64(TCGv_i64 ret, TCGv_i64 arg)
 {
     if (TCG_TARGET_REG_BITS == 32) {
-- 
2.41.0

[PATCH v2 6/8] tcg/tcg-op: Document hswap_i32/64() byte pattern

[Philippe Mathieu-Daudé]

Document hswap_i32() and hswap_i64(), added in commit
46be8425ff ("tcg: Implement tcg_gen_{h,w}swap_{i32,i64}").

Reviewed-by: Richard Henderson <richard.henderson@linaro.org>
Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
---
 tcg/tcg-op.c | 26 +++++++++++++++++++-------
 1 file changed, 19 insertions(+), 7 deletions(-)

diff --git a/tcg/tcg-op.c b/tcg/tcg-op.c
index 45290cccad..94134981a9 100644
--- a/tcg/tcg-op.c
+++ b/tcg/tcg-op.c
@@ -1099,6 +1099,11 @@ void tcg_gen_bswap32_i32(TCGv_i32 ret, TCGv_i32 arg)
     }
 }
 
+/*
+ * hswap_i32: Swap 16-bit halfwords within a 32-bit value.
+ *
+ * Byte pattern:  hswap_i32(abcd) -> cdab
+ */
 void tcg_gen_hswap_i32(TCGv_i32 ret, TCGv_i32 arg)
 {
     /* Swapping 2 16-bit elements is a rotate. */
@@ -1896,19 +1901,26 @@ void tcg_gen_bswap64_i64(TCGv_i64 ret, TCGv_i64 arg)
     }
 }
 
+/*
+ * hswap_i64: Swap 16-bit halfwords within a 64-bit value.
+ *
+ * See hswap64() in include/qemu/bitops.h
+ *
+ * Byte pattern:  hswap_i64(abcdefgh) -> ghefcdab
+ */
 void tcg_gen_hswap_i64(TCGv_i64 ret, TCGv_i64 arg)
 {
     uint64_t m = 0x0000ffff0000ffffull;
     TCGv_i64 t0 = tcg_temp_ebb_new_i64();
     TCGv_i64 t1 = tcg_temp_ebb_new_i64();
 
-    /* See include/qemu/bitops.h, hswap64. */
-    tcg_gen_rotli_i64(t1, arg, 32);
-    tcg_gen_andi_i64(t0, t1, m);
-    tcg_gen_shli_i64(t0, t0, 16);
-    tcg_gen_shri_i64(t1, t1, 16);
-    tcg_gen_andi_i64(t1, t1, m);
-    tcg_gen_or_i64(ret, t0, t1);
+                                        /* arg = abcdefgh */
+    tcg_gen_rotli_i64(t1, arg, 32);     /*  t1 = efghabcd */
+    tcg_gen_andi_i64(t0, t1, m);        /*  t0 = ..gh..cd */
+    tcg_gen_shli_i64(t0, t0, 16);       /*  t0 = gh..cd.. */
+    tcg_gen_shri_i64(t1, t1, 16);       /*  t1 = ..efghab */
+    tcg_gen_andi_i64(t1, t1, m);        /*  t1 = ..ef..ab */
+    tcg_gen_or_i64(ret, t0, t1);        /* ret = ghefcdab */
 
     tcg_temp_free_i64(t0);
     tcg_temp_free_i64(t1);
-- 
2.41.0

[PATCH v2 7/8] tcg/tcg-op: Document wswap_i64() byte pattern

[Philippe Mathieu-Daudé]

Document wswap_i64(), added in commit 46be8425ff
("tcg: Implement tcg_gen_{h,w}swap_{i32,i64}").

Reviewed-by: Richard Henderson <richard.henderson@linaro.org>
Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
---
 tcg/tcg-op.c | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/tcg/tcg-op.c b/tcg/tcg-op.c
index 94134981a9..65a2e5462e 100644
--- a/tcg/tcg-op.c
+++ b/tcg/tcg-op.c
@@ -1926,6 +1926,11 @@ void tcg_gen_hswap_i64(TCGv_i64 ret, TCGv_i64 arg)
     tcg_temp_free_i64(t1);
 }
 
+/*
+ * wswap_i64: Swap 32-bit words within a 64-bit value.
+ *
+ * Byte pattern:  wswap_i64(abcdefgh) -> efghabcd
+ */
 void tcg_gen_wswap_i64(TCGv_i64 ret, TCGv_i64 arg)
 {
     /* Swapping 2 32-bit elements is a rotate. */
-- 
2.41.0

[PATCH v2 8/8] target/cris: Fix a typo in gen_swapr()

[Philippe Mathieu-Daudé]

Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
---
 target/cris/translate.c | 20 +++++++++++---------
 1 file changed, 11 insertions(+), 9 deletions(-)

diff --git a/target/cris/translate.c b/target/cris/translate.c
index 0b3d724281..42103b5558 100644
--- a/target/cris/translate.c
+++ b/target/cris/translate.c
@@ -411,15 +411,17 @@ static inline void t_gen_swapw(TCGv d, TCGv s)
     tcg_gen_or_tl(d, d, t);
 }
 
-/* Reverse the within each byte.
-   T0 = (((T0 << 7) & 0x80808080) |
-   ((T0 << 5) & 0x40404040) |
-   ((T0 << 3) & 0x20202020) |
-   ((T0 << 1) & 0x10101010) |
-   ((T0 >> 1) & 0x08080808) |
-   ((T0 >> 3) & 0x04040404) |
-   ((T0 >> 5) & 0x02020202) |
-   ((T0 >> 7) & 0x01010101));
+/*
+ * Reverse the bits within each byte.
+ *
+ *  T0 = ((T0 << 7) & 0x80808080)
+ *     | ((T0 << 5) & 0x40404040)
+ *     | ((T0 << 3) & 0x20202020)
+ *     | ((T0 << 1) & 0x10101010)
+ *     | ((T0 >> 1) & 0x08080808)
+ *     | ((T0 >> 3) & 0x04040404)
+ *     | ((T0 >> 5) & 0x02020202)
+ *     | ((T0 >> 7) & 0x01010101);
  */
 static void t_gen_swapr(TCGv d, TCGv s)
 {
-- 
2.41.0

Re: [PATCH v2 8/8] target/cris: Fix a typo in gen_swapr()

[Edgar E. Iglesias]

[-- Attachment #1: Type: text/plain, Size: 1370 bytes --]

On Wed, Aug 23, 2023 at 4:56 PM Philippe Mathieu-Daudé <philmd@linaro.org>
wrote:

> Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
>

Reviewed-by: Edgar E. Iglesias <edgar@zeroasic.com>


> ---
>  target/cris/translate.c | 20 +++++++++++---------
>  1 file changed, 11 insertions(+), 9 deletions(-)
>
> diff --git a/target/cris/translate.c b/target/cris/translate.c
> index 0b3d724281..42103b5558 100644
> --- a/target/cris/translate.c
> +++ b/target/cris/translate.c
> @@ -411,15 +411,17 @@ static inline void t_gen_swapw(TCGv d, TCGv s)
>      tcg_gen_or_tl(d, d, t);
>  }
>
> -/* Reverse the within each byte.
> -   T0 = (((T0 << 7) & 0x80808080) |
> -   ((T0 << 5) & 0x40404040) |
> -   ((T0 << 3) & 0x20202020) |
> -   ((T0 << 1) & 0x10101010) |
> -   ((T0 >> 1) & 0x08080808) |
> -   ((T0 >> 3) & 0x04040404) |
> -   ((T0 >> 5) & 0x02020202) |
> -   ((T0 >> 7) & 0x01010101));
> +/*
> + * Reverse the bits within each byte.
> + *
> + *  T0 = ((T0 << 7) & 0x80808080)
> + *     | ((T0 << 5) & 0x40404040)
> + *     | ((T0 << 3) & 0x20202020)
> + *     | ((T0 << 1) & 0x10101010)
> + *     | ((T0 >> 1) & 0x08080808)
> + *     | ((T0 >> 3) & 0x04040404)
> + *     | ((T0 >> 5) & 0x02020202)
> + *     | ((T0 >> 7) & 0x01010101);
>   */
>  static void t_gen_swapr(TCGv d, TCGv s)
>  {
> --
> 2.41.0
>
>

[-- Attachment #2: Type: text/html, Size: 2182 bytes --]

Re: [PATCH v2 0/8] tcg: Document *swap* helper implementations

[Richard Henderson]

On 8/23/23 07:55, Philippe Mathieu-Daudé wrote:
> Philippe Mathieu-Daudé (8):
>    tcg/tcg-op: Document bswap16_i32() byte pattern
>    tcg/tcg-op: Document bswap16_i64() byte pattern
>    tcg/tcg-op: Document bswap32_i32() byte pattern
>    tcg/tcg-op: Document bswap32_i64() byte pattern
>    tcg/tcg-op: Document bswap64_i64() byte pattern
>    tcg/tcg-op: Document hswap_i32/64() byte pattern
>    tcg/tcg-op: Document wswap_i64() byte pattern
>    target/cris: Fix a typo in gen_swapr()

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

and queued to tcg-next with a few tweaks.


r~

Re: [PATCH v2 0/8] tcg: Document *swap* helper implementations

[Philippe Mathieu-Daudé]

On 23/8/23 20:46, Richard Henderson wrote:
> On 8/23/23 07:55, Philippe Mathieu-Daudé wrote:
>> Philippe Mathieu-Daudé (8):
>>    tcg/tcg-op: Document bswap16_i32() byte pattern
>>    tcg/tcg-op: Document bswap16_i64() byte pattern
>>    tcg/tcg-op: Document bswap32_i32() byte pattern
>>    tcg/tcg-op: Document bswap32_i64() byte pattern
>>    tcg/tcg-op: Document bswap64_i64() byte pattern
>>    tcg/tcg-op: Document hswap_i32/64() byte pattern
>>    tcg/tcg-op: Document wswap_i64() byte pattern
>>    target/cris: Fix a typo in gen_swapr()
> 
> Reviewed-by: Richard Henderson <richard.henderson@linaro.org>
> 
> and queued to tcg-next with a few tweaks.

Thanks!