Re: [PATCH v4 153/163] tcg: Assign TCGOP_TYPE in liveness_pass_2

[Richard Henderson]

On 4/18/25 03:46, Nicholas Piggin wrote:

On Wed Apr 16, 2025 at 5:25 AM AEST, Richard Henderson wrote:

Here we cannot rely on the default copied from
tcg_op_insert_{after,before}, because the relevant
op could be typeless, such as INDEX_op_call.

Fixes: ...


Missing ^


Yeah, I filled in that blank just recently:
Fixes: fb744ece3a78 ("tcg: Copy TCGOP_TYPE in tcg_op_insert_{after,before}")


@@ -4408,6 +4408,7 @@ liveness_pass_2(TCGContext *s)
: INDEX_op_ld_i64);
  TCGOp *lop = tcg_op_insert_before(s, op, lopc, 3);
  
+TCGOP_TYPE(lop) = arg_ts->type;


tcg_op_insert_before/after I think are only called in these 3 places?


No, it's used quite a bit more in tcg/optimize.c.


So after this patch, the type assignment in those functions looks
redundant. Maybe you could pass in the type as an argument.


During development I had that, but it turned out to be unwieldy.  But perhaps that could 
be fixed with a local wrapper within optimize.c, taking the type from OptContext.


I'll experiment and get back to you.


r~

Re: [PATCH v4 153/163] tcg: Assign TCGOP_TYPE in liveness_pass_2

[Nicholas Piggin]

On Wed Apr 16, 2025 at 5:25 AM AEST, Richard Henderson wrote:
> Here we cannot rely on the default copied from
> tcg_op_insert_{after,before}, because the relevant
> op could be typeless, such as INDEX_op_call.
>
> Fixes: ...

Missing ^

> Signed-off-by: Richard Henderson 
> ---
>  tcg/tcg.c | 3 +++
>  1 file changed, 3 insertions(+)
>
> diff --git a/tcg/tcg.c b/tcg/tcg.c
> index 05604d122a..3c80ad086c 100644
> --- a/tcg/tcg.c
> +++ b/tcg/tcg.c
> @@ -4408,6 +4408,7 @@ liveness_pass_2(TCGContext *s)
>: INDEX_op_ld_i64);
>  TCGOp *lop = tcg_op_insert_before(s, op, lopc, 3);
>  
> +TCGOP_TYPE(lop) = arg_ts->type;

tcg_op_insert_before/after I think are only called in these 3 places?
So after this patch, the type assignment in those functions looks
redundant. Maybe you could pass in the type as an argument.

Thanks,
Nick

>  lop->args[0] = temp_arg(dir_ts);
>  lop->args[1] = temp_arg(arg_ts->mem_base);
>  lop->args[2] = arg_ts->mem_offset;
> @@ -4480,6 +4481,7 @@ liveness_pass_2(TCGContext *s)
>  arg_ts->state = TS_MEM;
>  }
>  
> +TCGOP_TYPE(sop) = arg_ts->type;
>  sop->args[0] = temp_arg(out_ts);
>  sop->args[1] = temp_arg(arg_ts->mem_base);
>  sop->args[2] = arg_ts->mem_offset;
> @@ -4507,6 +4509,7 @@ liveness_pass_2(TCGContext *s)
>: INDEX_op_st_i64);
>  TCGOp *sop = tcg_op_insert_after(s, op, sopc, 3);
>  
> +TCGOP_TYPE(sop) = arg_ts->type;
>  sop->args[0] = temp_arg(dir_ts);
>  sop->args[1] = temp_arg(arg_ts->mem_base);
>  sop->args[2] = arg_ts->mem_offset;

Re: [PATCH v4 153/163] tcg: Assign TCGOP_TYPE in liveness_pass_2

[Pierrick Bouvier]

On 4/15/25 12:25, Richard Henderson wrote:

Here we cannot rely on the default copied from
tcg_op_insert_{after,before}, because the relevant
op could be typeless, such as INDEX_op_call.

Fixes: ...
Signed-off-by: Richard Henderson 
---
  tcg/tcg.c | 3 +++
  1 file changed, 3 insertions(+)

diff --git a/tcg/tcg.c b/tcg/tcg.c
index 05604d122a..3c80ad086c 100644
--- a/tcg/tcg.c
+++ b/tcg/tcg.c
@@ -4408,6 +4408,7 @@ liveness_pass_2(TCGContext *s)
: INDEX_op_ld_i64);
  TCGOp *lop = tcg_op_insert_before(s, op, lopc, 3);
  
+TCGOP_TYPE(lop) = arg_ts->type;

  lop->args[0] = temp_arg(dir_ts);
  lop->args[1] = temp_arg(arg_ts->mem_base);
  lop->args[2] = arg_ts->mem_offset;
@@ -4480,6 +4481,7 @@ liveness_pass_2(TCGContext *s)
  arg_ts->state = TS_MEM;
  }
  
+TCGOP_TYPE(sop) = arg_ts->type;

  sop->args[0] = temp_arg(out_ts);
  sop->args[1] = temp_arg(arg_ts->mem_base);
  sop->args[2] = arg_ts->mem_offset;
@@ -4507,6 +4509,7 @@ liveness_pass_2(TCGContext *s)
: INDEX_op_st_i64);
  TCGOp *sop = tcg_op_insert_after(s, op, sopc, 3);
  
+TCGOP_TYPE(sop) = arg_ts->type;

  sop->args[0] = temp_arg(dir_ts);
  sop->args[1] = temp_arg(arg_ts->mem_base);
  sop->args[2] = arg_ts->mem_offset;


Reviewed-by: Pierrick Bouvier