Re: [RFC PATCH 3/4] target/riscv: check smstateen fcsr flag

[Mayuresh Chitale]

On Tue, Apr 11, 2023 at 7:23 AM Richard Henderson
<richard.henderson@linaro.org> wrote:
>
> On 4/10/23 07:13, Mayuresh Chitale wrote:
> > +#ifndef CONFIG_USER_ONLY
> > +#define smstateen_fcsr_check(ctx) do { \
> > +    if (!ctx->smstateen_fcsr_ok) { \
> > +        if (ctx->virt_enabled) { \
> > +            generate_exception(ctx, RISCV_EXCP_VIRT_INSTRUCTION_FAULT); \
> > +        } else { \
> > +            generate_exception(ctx, RISCV_EXCP_ILLEGAL_INST); \
> > +        } \
> > +        return true; \
> > +    } \
> > +} while (0)
> > +#else
> > +#define smstateen_fcsr_check(ctx)
> > +#endif
> > +
> > +#define REQUIRE_ZFINX_OR_F(ctx) do { \
> > +    if (!has_ext(ctx, RVF)) { \
> > +        if (!ctx->cfg_ptr->ext_zfinx) { \
> > +            return false; \
> > +        } \
> > +        smstateen_fcsr_check(ctx); \
> >       } \
> >   } while (0)
>
> As a matter of style, I strongly object to a *nested* macro returning from 
> the calling
> function.  These should all be changed to normal functions of the form
>
>      if (!require_xyz(ctx) || !require_abc(ctx)) {
>          return something;
>      }
>
> etc.  insn_trans/trans_rvv.c.inc is much much cleaner in this respect.
Ok. I will change smstateen_fcsr_check to a function.
>
>
> r~