Re: [PATCH v3 1/2] target/riscv: add Zicond as an experimental extension

[Philipp Tomsich]

Alistair,

On Mon, 6 Mar 2023 at 05:53, Alistair Francis <alistair23@gmail.com> wrote:
>
> On Wed, Feb 8, 2023 at 12:40 AM Philipp Tomsich
> <philipp.tomsich@vrull.eu> wrote:
> >
> > This implements the Zicond (conditional integer operations) extension,
> > as of version 1.0-draft-20230120 as an experimental extension in QEMU
> > ("x-zicond").
> >
> > The Zicond extension acts as a building block for branchless sequences
> > including conditional-{arithmetic,logic,select,move}.  Refer to the
> > specification for usage scenarios and application guidance.
> >
> > The following instructions constitute Zicond:
> >   - czero.eqz rd, rs1, rs2  =>  rd = (rs2 == 0) ? 0 : rs1
> >   - czero.nez rd, rs1, rs2  =>  rd = (rs2 != 0) ? 0 : rs1
> >
> > See
> >   
> > https://github.com/riscv/riscv-zicond/releases/download/v1.0-draft-20230120/riscv-zicond_1.0-draft-20230120.pdf
> > for the (current version of the) Zicond specification and usage details.
> >
> > Signed-off-by: Philipp Tomsich <philipp.tomsich@vrull.eu>
>
> Sorry about this.
>
> It looks like while I was out a different patch implementing this
> extension was applied. I think this patch just fell through the cracks
> as it was sent before I left and before Palmer took over.
>
> The second patch is still useful though, if you rebase it on the
> current master and resend it I can apply it.

Rebased onto master and resent as v4.
Given that the second patch depends on the first one (i.e., the entire
shared logic), I kept this as two separate changes (one to refactor
Zicond; one to reuse it in XVentanaCondOps).

Thanks,
Philipp.

>
>
> Alistair
>
> > ---
> >
> > Changes in v3:
> > - don't add this to MAINTAINERS, as it is an official extension
> >
> > Changes in v2:
> > - gates availability of the instructions through a REQUIRE_ZICOND
> >   macro (these were previously always enabled)
> >
> >  target/riscv/cpu.c                           |  4 ++
> >  target/riscv/cpu.h                           |  1 +
> >  target/riscv/insn32.decode                   |  4 ++
> >  target/riscv/insn_trans/trans_rvzicond.c.inc | 54 ++++++++++++++++++++
> >  target/riscv/translate.c                     |  1 +
> >  5 files changed, 64 insertions(+)
> >  create mode 100644 target/riscv/insn_trans/trans_rvzicond.c.inc
> >
> > diff --git a/target/riscv/cpu.c b/target/riscv/cpu.c
> > index 14a7027095..98177d8328 100644
> > --- a/target/riscv/cpu.c
> > +++ b/target/riscv/cpu.c
> > @@ -73,6 +73,7 @@ struct isa_ext_data {
> >  static const struct isa_ext_data isa_edata_arr[] = {
> >      ISA_EXT_DATA_ENTRY(h, false, PRIV_VERSION_1_12_0, ext_h),
> >      ISA_EXT_DATA_ENTRY(v, false, PRIV_VERSION_1_12_0, ext_v),
> > +    ISA_EXT_DATA_ENTRY(zicond, true, PRIV_VERSION_1_12_0, ext_zicond),
> >      ISA_EXT_DATA_ENTRY(zicsr, true, PRIV_VERSION_1_10_0, ext_icsr),
> >      ISA_EXT_DATA_ENTRY(zifencei, true, PRIV_VERSION_1_10_0, ext_ifencei),
> >      ISA_EXT_DATA_ENTRY(zihintpause, true, PRIV_VERSION_1_10_0, 
> > ext_zihintpause),
> > @@ -1097,6 +1098,9 @@ static Property riscv_cpu_extensions[] = {
> >      DEFINE_PROP_BOOL("x-smaia", RISCVCPU, cfg.ext_smaia, false),
> >      DEFINE_PROP_BOOL("x-ssaia", RISCVCPU, cfg.ext_ssaia, false),
> >
> > +    /* Zicond 1.0-draft-20230120 */
> > +    DEFINE_PROP_BOOL("x-zicond", RISCVCPU, cfg.ext_zicond, false),
> > +
> >      DEFINE_PROP_END_OF_LIST(),
> >  };
> >
> > diff --git a/target/riscv/cpu.h b/target/riscv/cpu.h
> > index bcf0826753..aaf3acb753 100644
> > --- a/target/riscv/cpu.h
> > +++ b/target/riscv/cpu.h
> > @@ -446,6 +446,7 @@ struct RISCVCPUConfig {
> >      bool ext_zkt;
> >      bool ext_ifencei;
> >      bool ext_icsr;
> > +    bool ext_zicond;
> >      bool ext_zihintpause;
> >      bool ext_smstateen;
> >      bool ext_sstc;
> > diff --git a/target/riscv/insn32.decode b/target/riscv/insn32.decode
> > index b7e7613ea2..ca812c2f7a 100644
> > --- a/target/riscv/insn32.decode
> > +++ b/target/riscv/insn32.decode
> > @@ -890,3 +890,7 @@ sm3p1       00 01000 01001 ..... 001 ..... 0010011 @r2
> >  # *** RV32 Zksed Standard Extension ***
> >  sm4ed       .. 11000 ..... ..... 000 ..... 0110011 @k_aes
> >  sm4ks       .. 11010 ..... ..... 000 ..... 0110011 @k_aes
> > +
> > +# *** Zicond Standard Extension ***
> > +czero_eqz   0000111 ..... ..... 101 ..... 0110011 @r
> > +czero_nez   0000111 ..... ..... 111 ..... 0110011 @r
> > diff --git a/target/riscv/insn_trans/trans_rvzicond.c.inc 
> > b/target/riscv/insn_trans/trans_rvzicond.c.inc
> > new file mode 100644
> > index 0000000000..20e9694a2c
> > --- /dev/null
> > +++ b/target/riscv/insn_trans/trans_rvzicond.c.inc
> > @@ -0,0 +1,54 @@
> > +/*
> > + * RISC-V translation routines for the XVentanaCondOps extension.
> > + *
> > + * Copyright (c) 2022 VRULL GmbH.
> > + *
> > + * This program is free software; you can redistribute it and/or modify it
> > + * under the terms and conditions of the GNU General Public License,
> > + * version 2 or later, as published by the Free Software Foundation.
> > + *
> > + * This program is distributed in the hope it will be useful, but WITHOUT
> > + * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
> > + * FITNESS FOR A PARTICULAR PURPOSE.  See the GNU General Public License 
> > for
> > + * more details.
> > + *
> > + * You should have received a copy of the GNU General Public License along 
> > with
> > + * this program.  If not, see <http://www.gnu.org/licenses/>.
> > + */
> > +
> > +#define REQUIRE_ZICOND(ctx) do {                 \
> > +    if (!ctx->cfg_ptr->ext_zicond) {             \
> > +        return false;                            \
> > +    }                                            \
> > +} while (0)
> > +
> > +/* Emits "$rd = ($rs2 <cond> $zero) ? $zero : $rs1" */
> > +static inline void gen_czero(TCGv dest, TCGv src1, TCGv src2, TCGCond cond)
> > +{
> > +    TCGv zero = tcg_constant_tl(0);
> > +    tcg_gen_movcond_tl(cond, dest, src2, zero, zero, src1);
> > +}
> > +
> > +static inline void gen_czero_eqz(TCGv dest, TCGv src1, TCGv src2)
> > +{
> > +    gen_czero(dest, src1, src2, TCG_COND_EQ);
> > +}
> > +
> > +static inline void gen_czero_nez(TCGv dest, TCGv src1, TCGv src2)
> > +{
> > +    gen_czero(dest, src1, src2, TCG_COND_NE);
> > +}
> > +
> > +static bool trans_czero_eqz(DisasContext *ctx, arg_r *a)
> > +{
> > +    REQUIRE_ZICOND(ctx);
> > +
> > +    return gen_logic(ctx, a, gen_czero_eqz);
> > +}
> > +
> > +static bool trans_czero_nez(DisasContext *ctx, arg_r *a)
> > +{
> > +    REQUIRE_ZICOND(ctx);
> > +
> > +    return gen_logic(ctx, a, gen_czero_nez);
> > +}
> > diff --git a/target/riscv/translate.c b/target/riscv/translate.c
> > index 01cc30a365..93850938ae 100644
> > --- a/target/riscv/translate.c
> > +++ b/target/riscv/translate.c
> > @@ -1076,6 +1076,7 @@ static uint32_t opcode_at(DisasContextBase *dcbase, 
> > target_ulong pc)
> >  #include "insn_trans/trans_rvv.c.inc"
> >  #include "insn_trans/trans_rvb.c.inc"
> >  #include "insn_trans/trans_rvzawrs.c.inc"
> > +#include "insn_trans/trans_rvzicond.c.inc"
> >  #include "insn_trans/trans_rvzfh.c.inc"
> >  #include "insn_trans/trans_rvk.c.inc"
> >  #include "insn_trans/trans_privileged.c.inc"
> > --
> > 2.34.1
> >
> >