Re: [PATCH 06/10] accel/tcg: Remove cpu_unwind_state_data() unused CPUState argument

[Peter Maydell]

On Fri, 15 Nov 2024 at 17:24, Richard Henderson
<richard.henderson@linaro.org> wrote:
>
> On 11/15/24 07:20, Philippe Mathieu-Daudé wrote:
> > Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
> > ---
> >   include/exec/translate-all.h | 3 +--
> >   accel/tcg/translate-all.c    | 2 +-
> >   target/i386/helper.c         | 3 ++-
> >   target/openrisc/sys_helper.c | 7 +++----
> >   4 files changed, 7 insertions(+), 8 deletions(-)
> >
> > diff --git a/include/exec/translate-all.h b/include/exec/translate-all.h
> > index f06cfedd52..9303318953 100644
> > --- a/include/exec/translate-all.h
> > +++ b/include/exec/translate-all.h
> > @@ -23,7 +23,6 @@
> >
> >   /**
> >    * cpu_unwind_state_data:
> > - * @cpu: the cpu context
> >    * @host_pc: the host pc within the translation
> >    * @data: output data
> >    *
> > @@ -32,7 +31,7 @@
> >    * function returns false; otherwise @data is loaded.
> >    * This is the same unwind info as given to restore_state_to_opc.
> >    */
> > -bool cpu_unwind_state_data(CPUState *cpu, uintptr_t host_pc, uint64_t 
> > *data);
> > +bool cpu_unwind_state_data(uintptr_t host_pc, uint64_t *data);
>
> Hmm.  I wonder if it should be called "cpu_*" at all then?
> Worth renaming to "tcg_*" or something?

Yes, it's odd, isn't it?

What's the plan for this function in a multi-target
emulation world? At the moment it (or functions it
calls) uses TARGET_INSN_START_WORDS which is a
target-CPU-type-specific value. If in the future we're
going to want it instead to look that up as e.g. some
property of the CPU class then maybe we should keep
passing it the CPU pointer? Or would we instead say
that we'll define TARGET_INSN_START_WORDS as the worst
case for any target, since it's always between 1 and 3,
so it doesn't waste that much space if we have a couple
of extra sleb128 zero values for targets that don't need
all 3 words?

thanks
-- PMM