From f14bed3fd4a17e1255e2ed6db1dfe50e654e601d Mon Sep 17 00:00:00 2001 From: Richard Henderson Date: Mon, 2 Nov 2020 19:36:20 -0800 Subject: [PATCH 1/2] tcg: Remove assert from set_jmp_reset_offset MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Since 6e6c4efed99, there has been a more appropriate range check done later at the end of tcg_gen_code. There, a failing range check results in a returned error code, which causes the TB to be restarted at half the size. Reported-by: Sai Pavan Boddu Tested-by: Sai Pavan Boddu Reviewed-by: Philippe Mathieu-Daudé Signed-off-by: Richard Henderson --- tcg/tcg.c | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/tcg/tcg.c b/tcg/tcg.c index f49f1a7f35..43c6cf8f52 100644 --- a/tcg/tcg.c +++ b/tcg/tcg.c @@ -335,10 +335,11 @@ static bool tcg_resolve_relocs(TCGContext *s) static void set_jmp_reset_offset(TCGContext *s, int which) { - size_t off = tcg_current_code_size(s); - s->tb_jmp_reset_offset[which] = off; - /* Make sure that we didn't overflow the stored offset. */ - assert(s->tb_jmp_reset_offset[which] == off); + /* + * We will check for overflow at the end of the opcode loop in + * tcg_gen_code, where we bound tcg_current_code_size to UINT16_MAX. + */ + s->tb_jmp_reset_offset[which] = tcg_current_code_size(s); } #include "tcg-target.c.inc" From c56caea3b2a4ef5d760266f554df0d92c5a45f87 Mon Sep 17 00:00:00 2001 From: Richard Henderson Date: Tue, 3 Nov 2020 13:20:21 -0800 Subject: [PATCH 2/2] tcg: Revert "tcg/optimize: Flush data at labels not TCG_OPF_BB_END" This reverts commit cd0372c515c4732d8bd3777cdd995c139c7ed7ea. The patch is incorrect in that it retains copies between globals and non-local temps, and non-local temps still die at the end of the BB. Failing test case for hppa: .globl _start _start: cmpiclr,= 0x24,%r19,%r0 cmpiclr,<> 0x2f,%r19,%r19 ---- 00010057 0001005b movi_i32 tmp0,$0x24 sub_i32 tmp1,tmp0,r19 mov_i32 tmp2,tmp0 mov_i32 tmp3,r19 movi_i32 tmp1,$0x0 ---- 0001005b 0001005f brcond_i32 tmp2,tmp3,eq,$L1 movi_i32 tmp0,$0x2f sub_i32 tmp1,tmp0,r19 mov_i32 tmp2,tmp0 mov_i32 tmp3,r19 movi_i32 tmp1,$0x0 mov_i32 r19,tmp1 setcond_i32 psw_n,tmp2,tmp3,ne set_label $L1 In this case, both copies of "mov_i32 tmp3,r19" are removed. The second because opt thought it was redundant. The first is removed later by liveness because tmp3 is known to be dead. This leaves the setcond_i32 with an uninitialized input. Revert the entire patch for 5.2, and a proper optimization across the branch may be considered for the next development cycle. Reported-by: qemu@igor2.repo.hu Reviewed-by: Peter Maydell Signed-off-by: Richard Henderson --- tcg/optimize.c | 35 +++++++++++++++++------------------ 1 file changed, 17 insertions(+), 18 deletions(-) diff --git a/tcg/optimize.c b/tcg/optimize.c index 9952c28bdc..220f4601d5 100644 --- a/tcg/optimize.c +++ b/tcg/optimize.c @@ -1484,30 +1484,29 @@ void tcg_optimize(TCGContext *s) } } } - /* fall through */ + goto do_reset_output; default: do_default: - /* - * Default case: we know nothing about operation (or were unable - * to compute the operation result) so no propagation is done. - */ - for (i = 0; i < nb_oargs; i++) { - reset_temp(op->args[i]); - /* - * Save the corresponding known-zero bits mask for the - * first output argument (only one supported so far). - */ - if (i == 0) { - arg_info(op->args[i])->mask = mask; + /* Default case: we know nothing about operation (or were unable + to compute the operation result) so no propagation is done. + We trash everything if the operation is the end of a basic + block, otherwise we only trash the output args. "mask" is + the non-zero bits mask for the first output arg. */ + if (def->flags & TCG_OPF_BB_END) { + bitmap_zero(temps_used.l, nb_temps); + } else { + do_reset_output: + for (i = 0; i < nb_oargs; i++) { + reset_temp(op->args[i]); + /* Save the corresponding known-zero bits mask for the + first output argument (only one supported so far). */ + if (i == 0) { + arg_info(op->args[i])->mask = mask; + } } } break; - - case INDEX_op_set_label: - /* Trash everything at the start of a new extended bb. */ - bitmap_zero(temps_used.l, nb_temps); - break; } /* Eliminate duplicate and redundant fence instructions. */