Re: [PATCH 07/10] accel/tcg: Return TCGTBCPUState from cpu_get_tb_cpu_state

[Pierrick Bouvier <pierrick.bouvier@linaro.org>]

On 4/29/25 11:55 AM, Richard Henderson wrote:
> On 4/28/25 15:37, Richard Henderson wrote:
>> On 4/28/25 15:26, Pierrick Bouvier wrote:
>>> On 4/28/25 3:07 PM, Richard Henderson wrote:
>>>> On 4/28/25 15:00, Pierrick Bouvier wrote:
>>>>> On 4/28/25 1:10 PM, Richard Henderson wrote:
>>>>>> diff --git a/include/accel/tcg/cpu-ops.h b/include/accel/tcg/cpu-ops.h
>>>>>> index f5e5746976..8dbe79ea7c 100644
>>>>>> --- a/include/accel/tcg/cpu-ops.h
>>>>>> +++ b/include/accel/tcg/cpu-ops.h
>>>>>> @@ -18,8 +18,13 @@
>>>>>>     #include "exec/vaddr.h"
>>>>>>     #include "tcg/tcg-mo.h"
>>>>>> -void cpu_get_tb_cpu_state(CPUArchState *env, vaddr *pc,
>>>>>> -                          uint64_t *cs_base, uint32_t *flags);
>>>>>> +struct TCGTBCPUState {
>>>>>> +    vaddr pc;
>>>>>> +    uint32_t flags;
>>>>>> +    uint64_t flags2;
>>>>>
>>>>> Could it be named cs_base instead?
>>>>> flags2 is a little bit generic.
>>>>
>>>> Of course flags2 is generic -- it's only cs_base for x86.
>>>>
>>>
>>> It seems to be named the same for other architectures as well,
>>> so it's wrong for all other arch too?
>>>
>>> My point is that:
>>> tb_gen_code(cpu, s.pc, s.flags2, s.flags, cflags);
>>>
>>> is error prone, and it's easy to swap flags and flags2 by mistake.
>>> I don't mind the name, but something more distinct would help.
>>
>> I fully intended to go on and pass in TCGTBCPUState, rather than 3 separate args.  I guess
>> I stopped early with the meson.build change for build twice.
> 
> You do have a point that it's still named "cs_base" within TranslationBlock.  I'll keep
> the cs_base name for now and rename it all at once at a later date.
>

Sounds good.

> 
> r~