Re: [PATCH v2 04/15] target/ppc: Move DISAS_NORETURN setting into gen_exception*

[Richard Henderson]

On 4/29/21 10:07 AM, Luis Fernando Fujita Pires wrote:
> > > -static inline void gen_stop_exception(DisasContext *ctx)
> > > +static inline void gen_end_tb_exception(DisasContext *ctx, uint32_t
> > > +excp)
> > >    {
> > > -    gen_update_nip(ctx, ctx->base.pc_next);
> > > -    ctx->exception = POWERPC_EXCP_STOP;
> > > +    /* No need to update nip for SYNC/BRANCH, as execution flow will change
> > */
> > > +    if ((excp != POWERPC_EXCP_SYNC) &&
> > > +        (excp != POWERPC_EXCP_BRANCH))
> > > +    {
> > > +        gen_update_nip(ctx, ctx->base.pc_next);
> > > +    }
> > > +    ctx->exception = excp;
> > > +    ctx->base.is_jmp = DISAS_NORETURN;
> > >    }
> >
> > Hmm.  You didn't actually raise the exception, so you can't set
> > DISAS_NORETURN that way.  It looks like you should be using
> > gen_exception_nip().
>
> This is reproducing the behavior that was implemented before the DISAS_NORETURN 
> changes, that caused check-tcg to fail with an assertion otherwise.
>
> IIUC, POWERPC_EXCP_{STOP,SYNC,BRANCH} are not really exceptions and, in these cases, 
> ctx->exception is being used just to cause  ppc_tr_translate_insn() to end the 
> translation block. If so, we should not be using ctx->exception for that, but I 
> believe fixing that to not use ctx->exception belongs in a separate stand-alone patch.

Hum.  Well, you can set is_jmp = DISAS_TOO_MANY to force exit from the 
translation loop.

I believe a proper fix would be to turn all of these "fake" exceptions into 
DISAS_FOO constants, to be assigned to is_jmp. There are a bunch of 
DISAS_TARGET_N enumerators which should be renamed like in target/arm/translate.h:

#define DISAS_WFI       DISAS_TARGET_2
#define DISAS_SWI       DISAS_TARGET_3

etc.  Then most of the code that is special casing these constants should get 
moved to ppc_tr_tb_stop.


r~