qemu-riscv
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [Qemu-riscv] [Qemu-devel] [PATCH v3 26/35] target/riscv: Remove shif


From: Palmer Dabbelt
Subject: Re: [Qemu-riscv] [Qemu-devel] [PATCH v3 26/35] target/riscv: Remove shift and slt insn manual decoding
Date: Tue, 06 Nov 2018 16:56:49 -0800 (PST)

On Mon, 05 Nov 2018 09:00:10 PST (-0800), Bastian Koppelmann wrote:

On 11/1/18 4:59 PM, Palmer Dabbelt wrote:
On Wed, 31 Oct 2018 15:38:08 PDT (-0700), address@hidden
wrote:
On 10/31/18 1:20 PM, Bastian Koppelmann wrote:
 static bool trans_slt(DisasContext *ctx, arg_slt *a)
 {
-    gen_arith(ctx, OPC_RISC_SLT, a->rd, a->rs1, a->rs2);
+    TCGv source1 = tcg_temp_new();
+    TCGv source2 = tcg_temp_new();
+
+    gen_get_gpr(source1, a->rs1);
+    gen_get_gpr(source2, a->rs2);
+
+    tcg_gen_setcond_tl(TCG_COND_LT, source1, source1, source2);

I do wonder about extracting this one line to gen_slt so that you can
re-use
gen_arith and gen_arithi.

+    tcg_gen_setcond_tl(TCG_COND_LTU, source1, source1, source2);

Similarly.

 static bool trans_sllw(DisasContext *ctx, arg_sllw *a)
 {
-    gen_arith(ctx, OPC_RISC_SLLW, a->rd, a->rs1, a->rs2);
+    TCGv source1 = tcg_temp_new();
+    TCGv source2 = tcg_temp_new();
+
+    gen_get_gpr(source1, a->rs1);
+    gen_get_gpr(source2, a->rs2);
+
+    tcg_gen_andi_tl(source2, source2, 0x1F);
+    tcg_gen_shl_tl(source1, source1, source2);
+
+    gen_set_gpr(a->rd, source1);

Missing the ext32s after the shift.

 static bool trans_srlw(DisasContext *ctx, arg_srlw *a)
 {
-    gen_arith(ctx, OPC_RISC_SRLW, a->rd, a->rs1, a->rs2);
+    TCGv source1 = tcg_temp_new();
+    TCGv source2 = tcg_temp_new();
+
+    gen_get_gpr(source1, a->rs1);
+    gen_get_gpr(source2, a->rs2);
+
+    /* clear upper 32 */
+    tcg_gen_ext32u_tl(source1, source1);
+    tcg_gen_andi_tl(source2, source2, 0x1F);
+    tcg_gen_shr_tl(source1, source1, source2);

Likewise.  (Consider source2 == 0.)

-    case OPC_RISC_SRL:
-        tcg_gen_andi_tl(source2, source2, TARGET_LONG_BITS - 1);
-        tcg_gen_shr_tl(source1, source1, source2);
-        break;
...
-    case OPC_RISC_SRA:
-        tcg_gen_andi_tl(source2, source2, TARGET_LONG_BITS - 1);
-        tcg_gen_sar_tl(source1, source1, source2);
-        break;

I see that the bugs are in the original though, so fixing them in a
separate
patch is certainly ok.

Since we're in the soft freeze now, I think it would be great to get
the bug fixes broken out as separate patches at the start of this
series that we can pick up for this release.  It would be great if the
decodetree conversion was a non-functional change, as that will make
it easier to review.


Sounds good. Unfortunately, I don't have much time right now. I try at
least to extract the bugfixes, such that they make it for this release.

Thanks. You're welcome to just enumerate the bug fixes so I can dig them out if that's easier.



reply via email to

[Prev in Thread] Current Thread [Next in Thread]