qemu-devel.nongnu.org archive mirror
 help / color / mirror / Atom feed
* [PULL for-5.2 0/2] tcg patch queue
@ 2020-11-05  0:29 Richard Henderson
  2020-11-05  0:29 ` [PULL for-5.2 1/2] tcg: Remove assert from set_jmp_reset_offset Richard Henderson
                   ` (2 more replies)
  0 siblings, 3 replies; 4+ messages in thread
From: Richard Henderson @ 2020-11-05  0:29 UTC (permalink / raw)
  To: qemu-devel; +Cc: peter.maydell

The following changes since commit 3c8c36c9087da957f580a9bb5ebf7814a753d1c6:

  Merge remote-tracking branch 'remotes/kraxel/tags/ui-20201104-pull-request' into staging (2020-11-04 16:52:17 +0000)

are available in the Git repository at:

  https://github.com/rth7680/qemu.git tags/pull-tcg-20201104

for you to fetch changes up to c56caea3b2a4ef5d760266f554df0d92c5a45f87:

  tcg: Revert "tcg/optimize: Flush data at labels not TCG_OPF_BB_END" (2020-11-04 10:35:40 -0800)

----------------------------------------------------------------
Fix assert in set_jmp_reset_offset
Revert cross-branch optimization in tcg/optimize.c.

----------------------------------------------------------------
Richard Henderson (2):
      tcg: Remove assert from set_jmp_reset_offset
      tcg: Revert "tcg/optimize: Flush data at labels not TCG_OPF_BB_END"

 tcg/optimize.c | 35 +++++++++++++++++------------------
 tcg/tcg.c      |  9 +++++----
 2 files changed, 22 insertions(+), 22 deletions(-)


^ permalink raw reply	[flat|nested] 4+ messages in thread

* [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

* Re: [PULL for-5.2 0/2] tcg patch queue
  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 ` [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 ` Peter Maydell
  2 siblings, 0 replies; 4+ messages in thread
From: Peter Maydell @ 2020-11-05 17:24 UTC (permalink / raw)
  To: Richard Henderson; +Cc: QEMU Developers

On Thu, 5 Nov 2020 at 00:29, Richard Henderson
<richard.henderson@linaro.org> wrote:
>
> The following changes since commit 3c8c36c9087da957f580a9bb5ebf7814a753d1c6:
>
>   Merge remote-tracking branch 'remotes/kraxel/tags/ui-20201104-pull-request' into staging (2020-11-04 16:52:17 +0000)
>
> are available in the Git repository at:
>
>   https://github.com/rth7680/qemu.git tags/pull-tcg-20201104
>
> for you to fetch changes up to c56caea3b2a4ef5d760266f554df0d92c5a45f87:
>
>   tcg: Revert "tcg/optimize: Flush data at labels not TCG_OPF_BB_END" (2020-11-04 10:35:40 -0800)
>
> ----------------------------------------------------------------
> Fix assert in set_jmp_reset_offset
> Revert cross-branch optimization in tcg/optimize.c.
>
> ----------------------------------------------------------------
> Richard Henderson (2):
>       tcg: Remove assert from set_jmp_reset_offset
>       tcg: Revert "tcg/optimize: Flush data at labels not TCG_OPF_BB_END"


Applied, thanks.

Please update the changelog at https://wiki.qemu.org/ChangeLog/5.2
for any user-visible changes.

-- PMM


^ permalink raw reply	[flat|nested] 4+ messages in thread

end of thread, other threads:[~2020-11-05 17:28 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
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 ` [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

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).