* [PULL for-5.2 1/2] tcg: Remove assert from set_jmp_reset_offset
2020-11-05 0:29 [PULL for-5.2 0/2] tcg patch queue Richard Henderson
@ 2020-11-05 0:29 ` Richard Henderson
2020-11-05 0:29 ` [PULL for-5.2 2/2] tcg: Revert "tcg/optimize: Flush data at labels not TCG_OPF_BB_END" Richard Henderson
2020-11-05 17:24 ` [PULL for-5.2 0/2] tcg patch queue Peter Maydell
2 siblings, 0 replies; 4+ messages in thread
From: Richard Henderson @ 2020-11-05 0:29 UTC (permalink / raw)
To: qemu-devel
Cc: Sai Pavan Boddu, peter.maydell, Philippe Mathieu-Daudé,
Sai Pavan Boddu
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 <saipava@xilinx.com>
Tested-by: Sai Pavan Boddu <sai.pavan.boddu@xilinx.com>
Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
---
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"
--
2.25.1
^ permalink raw reply related [flat|nested] 4+ messages in thread* [PULL for-5.2 2/2] tcg: Revert "tcg/optimize: Flush data at labels not TCG_OPF_BB_END"
2020-11-05 0:29 [PULL for-5.2 0/2] tcg patch queue Richard Henderson
2020-11-05 0:29 ` [PULL for-5.2 1/2] tcg: Remove assert from set_jmp_reset_offset Richard Henderson
@ 2020-11-05 0:29 ` Richard Henderson
2020-11-05 17:24 ` [PULL for-5.2 0/2] tcg patch queue Peter Maydell
2 siblings, 0 replies; 4+ messages in thread
From: Richard Henderson @ 2020-11-05 0:29 UTC (permalink / raw)
To: qemu-devel; +Cc: peter.maydell, qemu
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 <peter.maydell@linaro.org>
Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
---
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. */
--
2.25.1
^ permalink raw reply related [flat|nested] 4+ messages in thread