diff mbox series

[v3,14/15] target/riscv: rewrite slli.uw implementation to mirror formal spec

Message ID 20210823164038.2195113-15-philipp.tomsich@vrull.eu
State New
Headers show
Series target/riscv: Update QEmu for Zb[abcs] 1.0.0 | expand

Commit Message

Philipp Tomsich Aug. 23, 2021, 4:40 p.m. UTC
The slli.uw instruction is defined as
    X(rd) = (EXTZ(X(rs)[31..0]) << shamt);

This rewrites its implementation to directly match this, allowing
for the small optimisation of not emitting the zero-extension if
the immediate shift is greater than 32.

Signed-off-by: Philipp Tomsich <philipp.tomsich@vrull.eu>
---

(no changes since v1)

 target/riscv/insn_trans/trans_rvb.c.inc | 15 ++++++---------
 1 file changed, 6 insertions(+), 9 deletions(-)

Comments

Richard Henderson Aug. 23, 2021, 5:36 p.m. UTC | #1
On 8/23/21 9:40 AM, Philipp Tomsich wrote:
> The slli.uw instruction is defined as
>      X(rd) = (EXTZ(X(rs)[31..0]) << shamt);
> 
> This rewrites its implementation to directly match this, allowing
> for the small optimisation of not emitting the zero-extension if
> the immediate shift is greater than 32.
> 
> Signed-off-by: Philipp Tomsich <philipp.tomsich@vrull.eu>
> ---
> 
> (no changes since v1)
> 
>   target/riscv/insn_trans/trans_rvb.c.inc | 15 ++++++---------
>   1 file changed, 6 insertions(+), 9 deletions(-)
> 
> diff --git a/target/riscv/insn_trans/trans_rvb.c.inc b/target/riscv/insn_trans/trans_rvb.c.inc
> index b134abd263..3419dcf1bb 100644
> --- a/target/riscv/insn_trans/trans_rvb.c.inc
> +++ b/target/riscv/insn_trans/trans_rvb.c.inc
> @@ -308,17 +308,14 @@ static bool trans_slli_uw(DisasContext *ctx, arg_slli_uw *a)
>           return false;
>       }
>   
> -    TCGv source1 = tcg_temp_new();
> -    gen_get_gpr(source1, a->rs1);
> -
> +    TCGv source = tcg_temp_new();
> +    gen_get_gpr(source, a->rs1);
>       if (a->shamt < 32) {
> -        tcg_gen_deposit_z_tl(source1, source1, a->shamt, 32);
> -    } else {
> -        tcg_gen_shli_tl(source1, source1, a->shamt);
> +        tcg_gen_ext32u_tl(source, source);
>       }

Why?  You're replacing one operation with two, when the host supports this deposit operation.


r~


> -
> -    gen_set_gpr(a->rd, source1);
> -    tcg_temp_free(source1);
> +    tcg_gen_shli_tl(source, source, a->shamt);
> +    gen_set_gpr(a->rd, source);
> +    tcg_temp_free(source);
>       return true;
>   }
>   
>
diff mbox series

Patch

diff --git a/target/riscv/insn_trans/trans_rvb.c.inc b/target/riscv/insn_trans/trans_rvb.c.inc
index b134abd263..3419dcf1bb 100644
--- a/target/riscv/insn_trans/trans_rvb.c.inc
+++ b/target/riscv/insn_trans/trans_rvb.c.inc
@@ -308,17 +308,14 @@  static bool trans_slli_uw(DisasContext *ctx, arg_slli_uw *a)
         return false;
     }
 
-    TCGv source1 = tcg_temp_new();
-    gen_get_gpr(source1, a->rs1);
-
+    TCGv source = tcg_temp_new();
+    gen_get_gpr(source, a->rs1);
     if (a->shamt < 32) {
-        tcg_gen_deposit_z_tl(source1, source1, a->shamt, 32);
-    } else {
-        tcg_gen_shli_tl(source1, source1, a->shamt);
+        tcg_gen_ext32u_tl(source, source);
     }
-
-    gen_set_gpr(a->rd, source1);
-    tcg_temp_free(source1);
+    tcg_gen_shli_tl(source, source, a->shamt);
+    gen_set_gpr(a->rd, source);
+    tcg_temp_free(source);
     return true;
 }