Re: [PATCH-for-10.1 10/12] tcg: Unify tcg_gen_insn_start() to handle 1 or 2 arguments

[Richard Henderson <richard.henderson@linaro.org>]

On 3/19/25 06:45, Philippe Mathieu-Daudé wrote:
> Merge the tcg_gen_insn_start() definition using 1 extra word
> with the definition using 2, using a2=0 in callers.
> 
> Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
> ---
>   include/tcg/tcg-op.h          | 16 ++++------------
>   target/i386/tcg/translate.c   |  2 +-
>   target/m68k/translate.c       |  2 +-
>   target/microblaze/translate.c |  2 +-
>   target/openrisc/translate.c   |  2 +-
>   target/sh4/translate.c        |  2 +-
>   target/sparc/translate.c      |  2 +-
>   7 files changed, 10 insertions(+), 18 deletions(-)

Nack.

> @@ -50,7 +40,9 @@ static inline void tcg_gen_insn_start(uint64_t pc, uint64_t a1, uint64_t a2)
>   
>       tcg_set_insn_start_param(op, 0, pc);
>       tcg_set_insn_start_param(op, 1, a1);
> -    tcg_set_insn_start_param(op, 2, a2);
> +    if (insn_start_words > 2) {
> +        tcg_set_insn_start_param(op, 2, a2);
> +    }

Not this, where you're passing unused arguments but for some reason are not storing them.

If you want to unify all callers on 3 arguments, that's one thing.
But conditionally not storing them?  That's just weird.

If you *do* want to unify all callers on 3 arguments, then you can dispense with 
TARGET_INSN_START_EXTRA_WORDS entirely.  Just define TCG_INSN_START_WORDS == 3 generically 
and drop everything else.


r~