| Message ID | 20250415192515.232910-154-richard.henderson@linaro.org |
|---|---|
| State | New |
| Headers | show |
| Series | tcg: Convert to TCGOutOp structures | expand |
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 <richard.henderson@linaro.org> > --- > 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 <pierrick.bouvier@linaro.org>
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 <richard.henderson@linaro.org> > --- > 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;
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~
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;
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 <richard.henderson@linaro.org> --- tcg/tcg.c | 3 +++ 1 file changed, 3 insertions(+)