[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [PATCH v2 12/22] target/loongarch: Add fixed point extra instruction
From: |
Richard Henderson |
Subject: |
Re: [PATCH v2 12/22] target/loongarch: Add fixed point extra instruction translation |
Date: |
Thu, 22 Jul 2021 19:12:54 -1000 |
User-agent: |
Mozilla/5.0 (X11; Linux x86_64; rv:78.0) Gecko/20100101 Thunderbird/78.11.0 |
On 7/20/21 11:53 PM, Song Gao wrote:
+target_ulong helper_cpucfg(CPULoongArchState *env, target_ulong rj)
+{
+ target_ulong r = 0;
+
+ switch (rj) {
+ case 0:
+ r = env->CSR_MCSR0 & 0xffffffff;
+ break;
+ case 1:
+ r = (env->CSR_MCSR0 & 0xffffffff00000000) >> 32;
+ break;
Why do you represent all of these as high and low portions of a 64-bit internal value,
when the manual describes them as 32-bit values?
+/* Fixed point extra instruction translation */
+static bool trans_crc_w_b_w(DisasContext *ctx, arg_crc_w_b_w *a)
+{
+ TCGv t0, t1;
+ TCGv Rd = cpu_gpr[a->rd];
+ TCGv_i32 tsz = tcg_const_i32(1 << 1);
This size is wrong. It should be 1, not 1 << 1 (2).
+static bool trans_crc_w_w_w(DisasContext *ctx, arg_crc_w_w_w *a)
+{
+ TCGv t0, t1;
+ TCGv Rd = cpu_gpr[a->rd];
+ TCGv_i32 tsz = tcg_const_i32(1 << 4);
Because this size most certainly should not be 16...
+static bool trans_crc_w_d_w(DisasContext *ctx, arg_crc_w_d_w *a)
+{
+ TCGv t0, t1;
+ TCGv Rd = cpu_gpr[a->rd];
+ TCGv_i32 tsz = tcg_const_i32(1 << 8);
... and this size should not be 256. Both well larger than the 8 byte buffer that you've
allocated.
Also, you need a helper so that you don't have 8 copies of this code.
+static bool trans_asrtle_d(DisasContext *ctx, arg_asrtle_d * a)
+{
+ TCGv t0, t1;
+
+ t0 = get_gpr(a->rj);
+ t1 = get_gpr(a->rk);
+
+ gen_helper_asrtle_d(cpu_env, t0, t1);
+
+ return true;
+}
+
+static bool trans_asrtgt_d(DisasContext *ctx, arg_asrtgt_d * a)
+{
+ TCGv t0, t1;
+
+ t0 = get_gpr(a->rj);
+ t1 = get_gpr(a->rk);
+
+ gen_helper_asrtgt_d(cpu_env, t0, t1);
+
+ return true;
+}
I'm not sure why both of these instructions are in the ISA, since
ASRTLE X,Y <-> ASRTGT Y,X
but we certainly don't need two different helpers.
Just swap the arguments for one of them.
+static bool trans_rdtimel_w(DisasContext *ctx, arg_rdtimel_w *a)
+{
+ /* Nop */
+ return true;
+}
+
+static bool trans_rdtimeh_w(DisasContext *ctx, arg_rdtimeh_w *a)
+{
+ /* Nop */
+ return true;
+}
+
+static bool trans_rdtime_d(DisasContext *ctx, arg_rdtime_d *a)
+{
+ /* Nop */
+ return true;
+}
If you don't want to implement these right now, you should at least initialize the
destination register to 0, or something.
r~
- Re: [PATCH v2 09/22] target/loongarch: Add fixed point bit instruction translation, (continued)
[PATCH v2 10/22] target/loongarch: Add fixed point load/store instruction translation, Song Gao, 2021/07/21
[PATCH v2 11/22] target/loongarch: Add fixed point atomic instruction translation, Song Gao, 2021/07/21
[PATCH v2 12/22] target/loongarch: Add fixed point extra instruction translation, Song Gao, 2021/07/21
- Re: [PATCH v2 12/22] target/loongarch: Add fixed point extra instruction translation,
Richard Henderson <=
[PATCH v2 13/22] target/loongarch: Add floating point arithmetic instruction translation, Song Gao, 2021/07/21
[PATCH v2 15/22] target/loongarch: Add floating point conversion instruction translation, Song Gao, 2021/07/21