Re: [PATCH v2 2/5] RISC-V: Cache management instructions

[Tsukasa OI <research_trasio@irq.a4lg.com>]

(I forgot to add Cc:binutils@sourceware.org to my response to Chris)

On 2022/02/09 10:24, Christoph Müllner wrote:
> 
> 
> On Tue, Jan 11, 2022 at 11:26 AM Tsukasa OI via Binutils <binutils@sourceware.org <mailto:binutils@sourceware.org>> wrote:
> 
>     This commit adds 'Zicbom' / 'Zicboz' instructions.
> 
>     bfd/ChangeLog:
> 
>             * elfxx-riscv.c (riscv_multi_subset_supports): Add handling for
>             new instruction classes.
> 
>     include/ChangeLog:
> 
>             * opcode/riscv-opc.h (MATCH_CBO_CLEAN, MASK_CBO_CLEAN,
>             MATCH_CBO_FLUSH, MASK_CBO_FLUSH, MATCH_CBO_INVAL,
>             MASK_CBO_INVAL, MATCH_CBO_ZERO, MASK_CBO_ZERO): New macros.
>             * opcode/riscv.h (enum riscv_insn_class): Add new instruction
>             classes INSN_CLASS_ZICBOM and INSN_CLASS_ZICBOZ.
> 
>     opcodes/ChangeLog:
> 
>             * riscv-opc.c (riscv_opcodes): Add cache-block management
>             instructions.
>     ---
>      bfd/elfxx-riscv.c          | 4 ++++
>      include/opcode/riscv-opc.h | 9 +++++++++
>      include/opcode/riscv.h     | 2 ++
>      opcodes/riscv-opc.c        | 6 ++++++
>      4 files changed, 21 insertions(+)
> 
>     diff --git a/bfd/elfxx-riscv.c b/bfd/elfxx-riscv.c
>     index 3761811d4fd..a7cd9320655 100644
>     --- a/bfd/elfxx-riscv.c
>     +++ b/bfd/elfxx-riscv.c
>     @@ -2334,6 +2334,10 @@ riscv_multi_subset_supports (riscv_parse_subset_t *rps,
>          {
>          case INSN_CLASS_I:
>            return riscv_subset_supports (rps, "i");
>     +    case INSN_CLASS_ZICBOM:
>     +      return riscv_subset_supports (rps, "zicbom");
>     +    case INSN_CLASS_ZICBOZ:
>     +      return riscv_subset_supports (rps, "zicboz");
>          case INSN_CLASS_ZICSR:
>            return riscv_subset_supports (rps, "zicsr");
>          case INSN_CLASS_ZIFENCEI:
>     diff --git a/include/opcode/riscv-opc.h b/include/opcode/riscv-opc.h
>     index 0b8cc6c7ddb..b24e8a47c87 100644
>     --- a/include/opcode/riscv-opc.h
>     +++ b/include/opcode/riscv-opc.h
>     @@ -2029,6 +2029,15 @@
>      #define MASK_HSV_W 0xfe007fff
>      #define MATCH_HSV_D 0x6e004073
>      #define MASK_HSV_D 0xfe007fff
>     +/* Zicbom/Zicboz instructions. */
>     +#define MATCH_CBO_CLEAN 0x10200f
>     +#define MASK_CBO_CLEAN 0xfff07fff
>     +#define MATCH_CBO_FLUSH 0x20200f
>     +#define MASK_CBO_FLUSH 0xfff07fff
>     +#define MATCH_CBO_INVAL 0x200f
>     +#define MASK_CBO_INVAL 0xfff07fff
>     +#define MATCH_CBO_ZERO 0x40200f
>     +#define MASK_CBO_ZERO 0xfff07fff
>      /* Privileged CSR addresses.  */
>      #define CSR_USTATUS 0x0
>      #define CSR_UIE 0x4
>     diff --git a/include/opcode/riscv.h b/include/opcode/riscv.h
>     index 048ab0a5d68..1d74f0e521a 100644
>     --- a/include/opcode/riscv.h
>     +++ b/include/opcode/riscv.h
>     @@ -388,6 +388,8 @@ enum riscv_insn_class
>        INSN_CLASS_V,
>        INSN_CLASS_ZVEF,
>        INSN_CLASS_SVINVAL,
>     +  INSN_CLASS_ZICBOM,
>     +  INSN_CLASS_ZICBOZ,
>      };
> 
>      /* This structure holds information for a particular instruction.  */
>     diff --git a/opcodes/riscv-opc.c b/opcodes/riscv-opc.c
>     index 2da0f7cf0a4..07835ddd071 100644
>     --- a/opcodes/riscv-opc.c
>     +++ b/opcodes/riscv-opc.c
>     @@ -849,6 +849,12 @@ const struct riscv_opcode riscv_opcodes[] =
>      {"sfence.vma", 0, INSN_CLASS_I,    "s,t",      MATCH_SFENCE_VMA, MASK_SFENCE_VMA, match_opcode, 0 },
>      {"wfi",        0, INSN_CLASS_I,    "",         MATCH_WFI, MASK_WFI, match_opcode, 0 },
> 
>     +/* Zicbom and Zicboz instructions.  */
>     +{"cbo.clean",  0, INSN_CLASS_ZICBOM, "s", MATCH_CBO_CLEAN, MASK_CBO_CLEAN, match_opcode, 0 },
>     +{"cbo.flush",  0, INSN_CLASS_ZICBOM, "s", MATCH_CBO_FLUSH, MASK_CBO_FLUSH, match_opcode, 0 },
>     +{"cbo.inval",  0, INSN_CLASS_ZICBOM, "s", MATCH_CBO_INVAL, MASK_CBO_INVAL, match_opcode, 0 },
>     +{"cbo.zero",   0, INSN_CLASS_ZICBOZ, "s", MATCH_CBO_ZERO, MASK_CBO_ZERO, match_opcode, 0 },
>     +
> 
> 
> Hi Tsukasa,
> 
> There has been a discussion ([1]) about using "0(a0)"/"(a0)" vs "a0" as operand format for these instructions.
> Following that discussion, I'd suggest using the AMO (zero-)displacement format ("0(s)") here (and adjusting the tests accordingly).
> 
> BR
> Christoph

I'm not sure that `cbo.zero 0(a0)' (for example) is right (I will just
follow the conclusion of the discussion).  Still, it's not bad to have
an option.

So, I made one (mutually exclusive to old patchset):
<https://sourceware.org/pipermail/binutils/2022-February/119708.html>

Thanks,
Tsukasa

> 
> [1] https://github.com/riscv/riscv-CMOs/issues/47 <https://github.com/riscv/riscv-CMOs/issues/47>
> 
>  
> 
>      /* Zbb or zbkb instructions.  */
>      {"clz",        0, INSN_CLASS_ZBB,  "d,s",   MATCH_CLZ, MASK_CLZ, match_opcode, 0 },
>      {"ctz",        0, INSN_CLASS_ZBB,  "d,s",   MATCH_CTZ, MASK_CTZ, match_opcode, 0 },
>     -- 
>     2.32.0
>