[Qemu-devel] [PATCH 0/2] target/arm: Minor bit field improvements

[Qemu-devel] [PATCH 0/2] target/arm: Minor bit field improvements

[Richard Henderson]

The tcg extract2 patch on which this depended is now in master.


r~


Richard Henderson (2):
  target/arm: Use extract2 for EXTR
  target/arm: Simplify BFXIL expansion

 target/arm/translate-a64.c | 44 ++++++++++++++++++++------------------
 1 file changed, 23 insertions(+), 21 deletions(-)

-- 
2.17.1

[Qemu-devel] [PATCH 1/2] target/arm: Use extract2 for EXTR

[Richard Henderson]

This is, after all, how we implement extract2 in tcg/aarch64.

Reviewed-by: Peter Maydell <peter.maydell@linaro.org>
Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
---
 target/arm/translate-a64.c | 38 ++++++++++++++++++++------------------
 1 file changed, 20 insertions(+), 18 deletions(-)

diff --git a/target/arm/translate-a64.c b/target/arm/translate-a64.c
index 9dcc5ff3a3..c4bee74ce5 100644
--- a/target/arm/translate-a64.c
+++ b/target/arm/translate-a64.c
@@ -4114,25 +4114,27 @@ static void disas_extract(DisasContext *s, uint32_t insn)
             } else {
                 tcg_gen_ext32u_i64(tcg_rd, cpu_reg(s, rm));
             }
-        } else if (rm == rn) { /* ROR */
-            tcg_rm = cpu_reg(s, rm);
-            if (sf) {
-                tcg_gen_rotri_i64(tcg_rd, tcg_rm, imm);
-            } else {
-                TCGv_i32 tmp = tcg_temp_new_i32();
-                tcg_gen_extrl_i64_i32(tmp, tcg_rm);
-                tcg_gen_rotri_i32(tmp, tmp, imm);
-                tcg_gen_extu_i32_i64(tcg_rd, tmp);
-                tcg_temp_free_i32(tmp);
-            }
         } else {
-            tcg_rm = read_cpu_reg(s, rm, sf);
-            tcg_rn = read_cpu_reg(s, rn, sf);
-            tcg_gen_shri_i64(tcg_rm, tcg_rm, imm);
-            tcg_gen_shli_i64(tcg_rn, tcg_rn, bitsize - imm);
-            tcg_gen_or_i64(tcg_rd, tcg_rm, tcg_rn);
-            if (!sf) {
-                tcg_gen_ext32u_i64(tcg_rd, tcg_rd);
+            tcg_rm = cpu_reg(s, rm);
+            tcg_rn = cpu_reg(s, rn);
+
+            if (sf) {
+                /* Specialization to ROR happens in EXTRACT2.  */
+                tcg_gen_extract2_i64(tcg_rd, tcg_rm, tcg_rn, imm);
+            } else {
+                TCGv_i32 t0 = tcg_temp_new_i32();
+
+                tcg_gen_extrl_i64_i32(t0, tcg_rm);
+                if (rm == rn) {
+                    tcg_gen_rotri_i32(t0, t0, imm);
+                } else {
+                    TCGv_i32 t1 = tcg_temp_new_i32();
+                    tcg_gen_extrl_i64_i32(t1, tcg_rn);
+                    tcg_gen_extract2_i32(t0, t0, t1, imm);
+                    tcg_temp_free_i32(t1);
+                }
+                tcg_gen_extu_i32_i64(tcg_rd, t0);
+                tcg_temp_free_i32(t0);
             }
         }
     }
-- 
2.17.1

[Qemu-devel] [PATCH 2/2] target/arm: Simplify BFXIL expansion

[Richard Henderson]

The mask implied by the extract is redundant with the one
implied by the deposit.  Also, fix spelling of BFXIL.

Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
---
 target/arm/translate-a64.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/target/arm/translate-a64.c b/target/arm/translate-a64.c
index c4bee74ce5..472d898096 100644
--- a/target/arm/translate-a64.c
+++ b/target/arm/translate-a64.c
@@ -4043,8 +4043,8 @@ static void disas_bitfield(DisasContext *s, uint32_t insn)
             tcg_gen_extract_i64(tcg_rd, tcg_tmp, ri, len);
             return;
         }
-        /* opc == 1, BXFIL fall through to deposit */
-        tcg_gen_extract_i64(tcg_tmp, tcg_tmp, ri, len);
+        /* opc == 1, BFXIL fall through to deposit */
+        tcg_gen_shri_i64(tcg_tmp, tcg_tmp, ri);
         pos = 0;
     } else {
         /* Handle the ri > si case with a deposit
@@ -4062,7 +4062,7 @@ static void disas_bitfield(DisasContext *s, uint32_t insn)
         len = ri;
     }
 
-    if (opc == 1) { /* BFM, BXFIL */
+    if (opc == 1) { /* BFM, BFXIL */
         tcg_gen_deposit_i64(tcg_rd, tcg_rd, tcg_tmp, pos, len);
     } else {
         /* SBFM or UBFM: We start with zero, and we haven't modified
-- 
2.17.1

Re: [Qemu-devel] [PATCH 0/2] target/arm: Minor bit field improvements

[Peter Maydell]

On Tue, 14 May 2019 at 02:11, Richard Henderson
<richard.henderson@linaro.org> wrote:
>
> The tcg extract2 patch on which this depended is now in master.
>
>
> r~
>
>
> Richard Henderson (2):
>   target/arm: Use extract2 for EXTR
>   target/arm: Simplify BFXIL expansion



Applied to target-arm.next, thanks.

-- PMM

Re: [Qemu-devel] [PATCH 1/2] target/arm: Use extract2 for EXTR

[Peter Maydell]

On Tue, 14 May 2019 at 02:11, Richard Henderson
<richard.henderson@linaro.org> wrote:
>
> This is, after all, how we implement extract2 in tcg/aarch64.
>
> Reviewed-by: Peter Maydell <peter.maydell@linaro.org>
> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
> ---
>  target/arm/translate-a64.c | 38 ++++++++++++++++++++------------------
>  1 file changed, 20 insertions(+), 18 deletions(-)

It turns out that we have a regression in booting at least
some Linux kernels with TCG on aarch64 hosts (the same
config works fine on x86-64 hosts). Git bisect points to
this commit (80ac954c369e7e61bd1ed0) as the cause, and
reverting this commit on top of current master also fixes
the problem.

thanks
-- PMM

Re: [Qemu-devel] [PATCH 1/2] target/arm: Use extract2 for EXTR

[Richard Henderson]

On 7/9/19 6:40 PM, Peter Maydell wrote:
> On Tue, 14 May 2019 at 02:11, Richard Henderson
> <richard.henderson@linaro.org> wrote:
>>
>> This is, after all, how we implement extract2 in tcg/aarch64.
>>
>> Reviewed-by: Peter Maydell <peter.maydell@linaro.org>
>> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
>> ---
>>  target/arm/translate-a64.c | 38 ++++++++++++++++++++------------------
>>  1 file changed, 20 insertions(+), 18 deletions(-)
> 
> It turns out that we have a regression in booting at least
> some Linux kernels with TCG on aarch64 hosts (the same
> config works fine on x86-64 hosts). Git bisect points to
> this commit (80ac954c369e7e61bd1ed0) as the cause...

Bisect would finger that one, since this second commit is the only method by
which an extract2 operation would be emitted by the aarch64 target + aarch64
host combination.

(The other place that extract2 might be used are deposits, but aarch64 host has
all of those covered with the bfi instruction.)

Fix for tcg/aarch64 coming up...


r~