Re: [PATCH 1/1] disas/riscv: Guard dec->cfg dereference for host disassemble

[LIU Zhiwei]

On 2024/12/6 21:36, Richard Henderson wrote:
> On 12/5/24 22:39, LIU Zhiwei wrote:
> > Both zcmt and zcmp are not compatible with Zcd, as they reuse some 
> > encodings from c.fsdsp.
>
> Ok, fair.  A comment about conflicts at that point may help.
Ok.
> > Zimop or Zcmop also overlap with other isa extensions, as they are 
> > maybe-ops. Other extensions
> > such as zicfiss will reuse their encodings.
> >
> > I think we had better disassemble them to zicifss if it has been 
> > implemented on the target cpu. Otherwise
> > we disassemble them to maybe-ops.
>
> My point is that they are only belong to zimop until they are 
> assigned, like zicifss.

No, they always belong to zimop unless they are assigned to other 
extensions. Applications built with zicfiss can also
run on cpu with zimop. In this case, the instructions of zicfiss should 
be disassemble as zimop maybe-ops which has it's default behavior 
different with the behavior of zicfiss.

 Zimop belongs to mandate extension in RVB23 profile. There may be a 
lot of cpus implement zimop but doesn't implement overlapping zicfiss.  
So disassemble the applications to zimop is appropriate for these cpus.

> At that point they *have* a defined meaning in the standard isa.
>
> So, yes, disassemble as zicifss, but always, not "if it has been 
> implemented in the target cpu".
>
> > > > +        if (((i == 0) || cfg) && guard_func(cfg)) {
> > >
> > > This should be i == 0 || (cfg && guard_func(cfg)).
> >
> > OK. Although I think they are both right.
>
>  i = 0
>  cfg = NULL
>
>    (0 == 0 || NULL) && guard_func(NULL)
> -> (true || false) && guard_func(NULL)
> -> true && guard_func(NULL)
> -> guard_func(NULL)
> -> boom.
>
> Or are you saying it won't go boom because we happen to know the 0th 
> guard_func only returns true
Yes.
> ? There's still no reason to call it...

Agree. Will use this way.

Thanks,
Zhiwei

> r~