Re: [PATCH 2/6] target/riscv: Use env_archcpu() in [check_]nanbox()

[LIU Zhiwei]

On 2023/10/11 13:31, Philippe Mathieu-Daudé wrote:
> On 11/10/23 05:25, LIU Zhiwei wrote:
> > On 2023/10/11 1:04, Richard Henderson wrote:
> > > On 10/9/23 05:42, LIU Zhiwei wrote:
> > > > On 2023/10/9 19:02, Philippe Mathieu-Daudé wrote:
> > > > > When CPUArchState* is available (here CPURISCVState*), we
> > > > > can use the fast env_archcpu() macro to get ArchCPU* (here
> > > > > RISCVCPU*). The QOM cast RISCV_CPU() macro will be slower
> > > > > when building with --enable-qom-cast-debug.
>
>
> > > > If so, maybe we have to do this qom cast somewhere.
> > >
> > > No, I don't think so.  Or at least not in these places.
> >
> > Yes.  Perhaps, we should remove all RISCV_CPU macros using after the 
> > qom objects realized.
> >
> > Do you think we should remove the RISCV_CPU using in 
> > riscv_cpu_exec_interrupt? Although it  is not so hot. I think there 
> > is no reason to use it there.
>
> I have some note in my TODO to check replacing CPUState by ArchCPU in
> TCGCPUOps (like the cpu_exec_interrupt handler you mentioned). 

IMHO, this will make it harder for heterogeneous SOC support. ArchCPU is 
not a target agnostic struct.

I must miss something.

> However
> I'm running out of time, so feel free to try it.
I'd like to have a try if it will not block the heterogeneous SOC support.
> Using ArchCPU avoids the cast in target code.
Yes
> > Except this, there are many other places in hw/ and target/riscv 
> > using the RISCV_CPU macro.
>
> If a method is exposed as API, we need to check the type. After that
> for internal calls this is pointless.

Make sense. Thanks.

Zhiwei

> > If we know whether we should remove the RISCV_CPU macro use in these 
> > places, we can do it in another patch.
> >
> > Thanks,
> > Zhiwei
> >
> > > r~