Re: [PATCH 1/3] target/riscv: Remove the redundant initialization of env->misa_mxl

[Bin Meng]

On Thu, Jun 16, 2022 at 10:34 AM Alistair Francis <alistair23@gmail.com> wrote:
>
> On Mon, Jun 13, 2022 at 10:30 PM Bin Meng <bmeng.cn@gmail.com> wrote:
> >
> > On Mon, Jun 13, 2022 at 8:33 AM Alistair Francis <alistair23@gmail.com> 
> > wrote:
> > >
> > > On Thu, Jun 9, 2022 at 11:08 AM Bin Meng <bmeng.cn@gmail.com> wrote:
> > > >
> > > > env->misa_mxl was already set in the RISC-V cpu init routine, and
> > > > validated at the beginning of riscv_cpu_realize(). There is no need
> > > > to do a redundant initialization later.
> > > >
> > > > Signed-off-by: Bin Meng <bmeng.cn@gmail.com>
> > > > ---
> > > >
> > > >  target/riscv/cpu.c | 2 +-
> > > >  1 file changed, 1 insertion(+), 1 deletion(-)
> > > >
> > > > diff --git a/target/riscv/cpu.c b/target/riscv/cpu.c
> > > > index a91253d4bd..61d1737741 100644
> > > > --- a/target/riscv/cpu.c
> > > > +++ b/target/riscv/cpu.c
> > > > @@ -752,7 +752,7 @@ static void riscv_cpu_realize(DeviceState *dev, 
> > > > Error **errp)
> > > >              ext |= RVJ;
> > > >          }
> > > >
> > > > -        set_misa(env, env->misa_mxl, ext);
> > > > +        env->misa_ext_mask = env->misa_ext = ext;
> > >
> > > You're right that we don't need to set `misa_mxl`, but isn't it
> > > cleaner calling the helper function here instead of manually assigning
> > > it?
> > >
> >
> > There is no helper for assigning misa_ext only. Do you want a new
> > helper for that?
>
> No, I don't think we need a new helper. I mean, is there any harm in
> just calling `set_misa()` even if that means we are performing a
> redundant operation?
>

No there is no harm to perform a redundant initialization.

Feel free to drop this patch then.

Regards,
Bin