Re: [PATCH] target/riscv: fix inverted checks for ext_zb[abcs]

[Vineet Gupta]

Hi Alistair,

On 2/3/22 16:59, Alistair Francis wrote:
> On Fri, Feb 4, 2022 at 1:42 AM Philipp Tomsich <philipp.tomsich@vrull.eu> wrote:
> > While changing to the use of cfg_ptr, the conditions for REQUIRE_ZB[ABCS]
> > inadvertently became inverted and slipped through the initial testing (which
> > used RV64GC_XVentanaCondOps as a target).
> > This fixes the regression.
> >
> > Tested against SPEC2017 w/ GCC 12 (prerelease) for RV64GC_zba_zbb_zbc_zbs.
> >
> > Fixes: 718143c126 ("target/riscv: add a MAINTAINERS entry for XVentanaCondOps")
> >
> > Signed-off-by: Philipp Tomsich <philipp.tomsich@vrull.eu>
>
> Reviewed-by: Alistair Francis <alistair.francis@wdc.com>
>
> > ---
> > We may want to squash this onto the affected commit, if it hasn't made
> > it beyond the next-tree, yet.
>
> Yeah, agreed. I'll squash it in
>
> Alistair

Has this already been committed upstream. I was running into weird issue 
related to bitmanip and seems this was missing in my local tree.

Also the "Fixes: " entry in changelog doesn't seem OK; the issue seems 
to have been introduced in f2a32bec8f0da99 ("target/riscv: access cfg 
structure through DisasContext")

Thx,
-Vineet

> >   target/riscv/insn_trans/trans_rvb.c.inc | 8 ++++----
> >   1 file changed, 4 insertions(+), 4 deletions(-)
> >
> > diff --git a/target/riscv/insn_trans/trans_rvb.c.inc 
> > b/target/riscv/insn_trans/trans_rvb.c.inc
> > index f9bd3b7ec4..e3c6b459d6 100644
> > --- a/target/riscv/insn_trans/trans_rvb.c.inc
> > +++ b/target/riscv/insn_trans/trans_rvb.c.inc
> > @@ -19,25 +19,25 @@
> >    */
> >
> >   #define REQUIRE_ZBA(ctx) do {                    \
> > -    if (ctx->cfg_ptr->ext_zba) {                 \
> > +    if (!ctx->cfg_ptr->ext_zba) {                 \
> >           return false;                            \
> >       }                                            \
> >   } while (0)
> >
> >   #define REQUIRE_ZBB(ctx) do {                    \
> > -    if (ctx->cfg_ptr->ext_zbb) {                 \
> > +    if (!ctx->cfg_ptr->ext_zbb) {                 \
> >           return false;                            \
> >       }                                            \
> >   } while (0)
> >
> >   #define REQUIRE_ZBC(ctx) do {                    \
> > -    if (ctx->cfg_ptr->ext_zbc) {                 \
> > +    if (!ctx->cfg_ptr->ext_zbc) {                 \
> >           return false;                            \
> >       }                                            \
> >   } while (0)
> >
> >   #define REQUIRE_ZBS(ctx) do {                    \
> > -    if (ctx->cfg_ptr->ext_zbs) {                 \
> > +    if (!ctx->cfg_ptr->ext_zbs) {                 \
> >           return false;                            \
> >       }                                            \
> >   } while (0)
> > --
> > 2.34.1