[PATCH v6] Add macros to get opcode and register of insns appropriately
Lulu Cai
cailulu@loongson.cn
Tue Aug 27 07:05:52 GMT 2024
Hi, Sorry for the delayed reply. Some comments follow.
I thought it would make sense to check some GNU style formatting rules
in a set of
patches. This could be done with a script before each commit.
Additionally, it's a good idea to run Binutils test suite to avoid
regressions. I found
two FAILs when running make check-gas .
On 8/21/24 3:40 PM, Xin Wang wrote:
> LoongArch: Add macros to get opcode and register of instructions appropriately
>
> ---
> bfd/elfnn-loongarch.c | 54 +++++++++++++++++--------------------
> gas/config/tc-loongarch.c | 30 ++++++++++-----------
> include/opcode/loongarch.h | 55 ++++++++++++++++++++++++++++++++++----
> 3 files changed, 89 insertions(+), 50 deletions(-)
/* Snip. */
>
> diff --git a/gas/config/tc-loongarch.c b/gas/config/tc-loongarch.c
> index 5ca33c6e2e8..e28c3344437 100644
> --- a/gas/config/tc-loongarch.c
> +++ b/gas/config/tc-loongarch.c
> @@ -1078,34 +1078,32 @@ check_this_insn_before_appending (struct loongarch_cl_insn *ip)
> ip->reloc_info[ip->reloc_num].value = const_0;
> ip->reloc_num++;
> }
> - else if (ip->insn->mask == 0xffff8000
> - /* amcas.b rd, rk, rj */
> - && ((ip->insn_bin & 0xfff80000) == 0x38580000
> - /* amswap.w rd, rk, rj */
> - || (ip->insn_bin & 0xfff00000) == 0x38600000
> - /* ammax_db.wu rd, rk, rj */
> - || (ip->insn_bin & 0xffff0000) == 0x38700000
> - /* ammin_db.wu rd, rk, rj */
> - || (ip->insn_bin & 0xffff0000) == 0x38710000))
> + // check all atomic memory insns
Use /* */ more like GNU style comments than //.
> + else if (ip->insn->mask == LARCH_MK_ATOMIC_MEM
> + && LARCH_INSN_ATOMIC_MEM(ip->insn_bin))
> {
> /* For AMO insn amswap.[wd], amadd.[wd], etc. */
> if (ip->args[0] != 0
> && (ip->args[0] == ip->args[1] || ip->args[0] == ip->args[2]))
> - as_bad (_("automic memory operations insns require rd != rj"
> + as_bad (_("atomic memory operations insns require rd != rj"
> " && rd != rk when rd isn't r0"));
> }
> - else if ((ip->insn->mask == 0xffe08000
> + else if ((ip->insn->mask == LARCH_MK_BSTRINS_W
> /* bstrins.w rd, rj, msbw, lsbw */
> - && (ip->insn_bin & 0xffe00000) == 0x00600000)
> - || (ip->insn->mask == 0xffc00000
> + && LARCH_INSN_BSTRINS_W(ip->insn_bin))
> + || (ip->insn->mask == LARCH_MK_BSTRINS_D
> /* bstrins.d rd, rj, msbd, lsbd */
> - && (ip->insn_bin & 0xff800000) == 0x00800000))
> + && LARCH_INSN_BSTRINS_D(ip->insn_bin)))
> {
> /* For bstr(ins|pick).[wd]. */
> if (ip->args[2] < ip->args[3])
> as_bad (_("bstr(ins|pick).[wd] require msbd >= lsbd"));
> }
> - else if (ip->insn->mask != 0 && (ip->insn_bin & 0xfe0003c0) == 0x04000000
> + else if (ip->insn->mask != 0
> + && (LARCH_INSN_CSRXCHG(ip->insn_bin)
> + || LARCH_INSN_GCSRXCHG(ip->insn_bin))
> + && (LARCH_GET_RJ(ip->insn_bin) == 0
> + || LARCH_GET_RJ(ip->insn_bin) == 1)
> /* csrxchg rd, rj, csr_num */
> && (strcmp ("csrxchg", ip->name) == 0
> || strcmp ("gcsrxchg", ip->name) == 0))
> @@ -2204,7 +2202,7 @@ loongarch_convert_frag_branch (fragS *fragp)
> case RELAX_BRANCH_26:
> insn = bfd_getl32 (buf);
> /* Invert the branch condition. */
> - if (LARCH_FLOAT_BRANCH == (insn & LARCH_BRANCH_OPCODE_MASK))
> + if (LARCH_INSN_FLOAT_BRANCH(insn))
> insn ^= LARCH_FLOAT_BRANCH_INVERT_BIT;
> else
> insn ^= LARCH_BRANCH_INVERT_BIT;
> diff --git a/include/opcode/loongarch.h b/include/opcode/loongarch.h
> index 965a164307f..10c58335606 100644
> --- a/include/opcode/loongarch.h
> +++ b/include/opcode/loongarch.h
> @@ -31,22 +31,67 @@ extern "C"
> #define LARCH_NOP 0x03400000
> #define LARCH_B 0x50000000
> /* BCEQZ/BCNEZ. */
> - #define LARCH_FLOAT_BRANCH 0x48000000
> - #define LARCH_BRANCH_OPCODE_MASK 0xfc000000
> #define LARCH_BRANCH_INVERT_BIT 0x04000000
> #define LARCH_FLOAT_BRANCH_INVERT_BIT 0x00000100
>
> + #define LARCH_MK_ADDI_D 0xffc00000
> + #define LARCH_OP_ADDI_D 0x02c00000
> + #define LARCH_MK_ORI 0xffc00000
> + #define LARCH_OP_ORI 0x03800000
> + #define LARCH_MK_LU12I_W 0xfe000000
> + #define LARCH_OP_LU12I_W 0x14000000
> + #define LARCH_MK_LD_D 0xffc00000
> + #define LARCH_OP_LD_D 0x28c00000
> + #define LARCH_MK_JIRL 0xfc000000
> + #define LARCH_OP_JIRL 0x4c000000
> + #define LARCH_MK_BCEQZ 0xfc000300
> + #define LARCH_OP_BCEQZ 0x4c000000
> + #define LARCH_MK_BCNEZ 0xfc000300
> + #define LARCH_OP_BCNEZ 0x4c000100
The opcodes for bceqz/bcnez are wrong. They are 0x48000000 and
0x48000100 respectively.
> + #define LARCH_MK_ATOMIC_MEM 0xffff8000
> + #define LARCH_MK_BSTRINS_W 0xffe08000
> + #define LARCH_OP_BSTRINS_W 0x00600000
> + #define LARCH_MK_BSTRINS_D 0xffc00000
> + #define LARCH_OP_BSTRINS_D 0x00800000
> + #define LARCH_MK_CSRRD 0xff0003e0
> + #define LARCH_OP_CSRRD 0x04000000
> + #define LARCH_MK_CSRWR 0xff0003e0
> + #define LARCH_OP_CSRWR 0x04000020
> + #define LARCH_MK_CSRXCHG 0xff000000
> + #define LARCH_OP_CSRXCHG 0x04000000
> + #define LARCH_MK_GCSRXCHG 0xff000000
> + #define LARCH_OP_GCSRXCHG 0x05000000
> +
> + #define LARCH_INSN_OPS(insn, op) ((insn & LARCH_MK_##op) == LARCH_OP_##op)
> + #define LARCH_INSN_ADDI_D(insn) LARCH_INSN_OPS((insn), ADDI_D)
> + #define LARCH_INSN_ORI(insn) LARCH_INSN_OPS((insn), ORI)
> + #define LARCH_INSN_LU12I_W(insn) LARCH_INSN_OPS((insn), LU12I_W)
> + #define LARCH_INSN_LD_D(insn) LARCH_INSN_OPS((insn), LD_D)
> + #define LARCH_INSN_JIRL(insn) LARCH_INSN_OPS((insn), JIRL)
> + #define LARCH_INSN_BCEQZ(insn) LARCH_INSN_OPS((insn), BCEQZ)
> + #define LARCH_INSN_BCNEZ(insn) LARCH_INSN_OPS((insn), BCNEZ)
> + #define LARCH_INSN_FLOAT_BRANCH(insn) (LARCH_INSN_BCEQZ(insn) || LARCH_INSN_BCNEZ(insn))
> + #define LARCH_INSN_BSTRINS_W(insn) LARCH_INSN_OPS((insn), BSTRINS_W)
> + #define LARCH_INSN_BSTRINS_D(insn) LARCH_INSN_OPS((insn), BSTRINS_D)
> + #define LARCH_INSN_CSRXCHG(insn) LARCH_INSN_OPS((insn), CSRXCHG)
> + #define LARCH_INSN_GCSRXCHG(insn) LARCH_INSN_OPS((insn), GCSRXCHG)
> +
> + #define LARCH_INSN_ATOMIC_MEM(insn) \
> + ((ip->insn_bin & 0xfff80000) == 0x38580000 \
> + || (ip->insn_bin & 0xfff00000) == 0x38600000 \
> + || (ip->insn_bin & 0xffff0000) == 0x38700000 \
> + || (ip->insn_bin & 0xffff0000) == 0x38710000)
> +
It is "insn" instead of "ip->insn_bin".
> #define ENCODE_BRANCH16_IMM(x) (((x) >> 2) << 10)
>
> #define OUT_OF_RANGE(value, bits, align) \
> ((value) < (-(1 << ((bits) - 1) << align)) \
> || (value) > ((((1 << ((bits) - 1)) - 1) << align)))
>
> - #define LARCH_LU12I_W 0x14000000
> - #define LARCH_ORI 0x03800000
> - #define LARCH_LD_D 0x28c00000
> #define LARCH_RD_A0 0x04
> #define LARCH_RD_RJ_A0 0x084
> + #define LARCH_GET_RD(insn) (insn & 0x1f)
> + #define LARCH_GET_RJ(insn) ((insn >> 5) & 0x1f)
>
> typedef uint32_t insn_t;
>
More information about the Binutils
mailing list