All of lore.kernel.org
 help / color / mirror / Atom feed
From: "Alex Bennée" <alex.bennee@linaro.org>
To: Richard Henderson <richard.henderson@linaro.org>
Cc: qemu-devel@nongnu.org
Subject: Re: [PATCH v2 13/36] tcg: Use tcg_constant_{i32,i64} with tcg int expanders
Date: Fri, 24 Apr 2020 14:23:06 +0100	[thread overview]
Message-ID: <87d07x9fwl.fsf@linaro.org> (raw)
In-Reply-To: <df8292e5-5b27-31eb-1e4a-3e8f835481fc@linaro.org>


Richard Henderson <richard.henderson@linaro.org> writes:

> On 4/22/20 1:04 PM, Alex Bennée wrote:
>> 
>> Richard Henderson <richard.henderson@linaro.org> writes:
>> 
>>> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
>> 
>> We have a regression. Setting up a build dir with:
>> 
>>   ../../configure --disable-tools --disable-docs --target-list=sparc-softmmu,sparc64-softmmu
>>   make -j30 && make check-acceptance
>> 
>> And then running a bisect between HEAD and master:
>> 
>>   git bisect run /bin/sh -c "cd builds/bisect && make -j30 && ./tests/venv/bin/avocado run ./tests/acceptance/boot_linux_console.py:BootLinuxConsole.test_sparc_ss20"
>> 
>> Fingers:
>> 
>>   a4d42b76dd29818e4f393c4c3eb59601b0015b2f is the first bad commit
>>   commit a4d42b76dd29818e4f393c4c3eb59601b0015b2f
>>   Author: Richard Henderson <richard.henderson@linaro.org>
>>   Date:   Tue Apr 21 18:16:59 2020 -0700
>> 
>>       tcg: Use tcg_constant_{i32,i64} with tcg int expanders
>> 
>>       Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
>>       Message-Id: <20200422011722.13287-14-richard.henderson@linaro.org>
>
> Ho hum.  I can reproduce this, but after a day of debugging I'm no closer to
> figuring out what's wrong than when I started.
>
> I'm going to put this whole section of TEMP_CONST to the side for now.

From my own poking around I can say the hang occurs when you first
introduce just:

  void tcg_gen_movi_i32(TCGv_i32 ret, int32_t arg)
  {
      tcg_gen_mov_i32(ret, tcg_constant_i32(arg));
  }

and nothing else. Which indicates the problem has to be in the core
plumbing itself. This is odd because all the other architectures are
fine. I wonder if there is something special about sparc's constant
generation?

Eyeballing the numbers it does seem like sparc generates more negative
numbers than ARM does - although ARM does generate some. I thought I'd
just have a check to see what happens so I looked at the first
occurrence in the sparc test:

  0x00006224:  sethi  %hi(0xffdcf000), %g6
  0x00006228:  mov  %g6, %g6      ! 0xffdcf000
  0x0000622c:  sethi  %hi(0xffd00000), %g4
  0x00006230:  mov  %g4, %g4      ! 0xffd00000
  0x00006234:  sub  %g6, %g4, %g6
  0x00006238:  sub  %g1, %g6, %g3
  0x0000623c:  sethi  %hi(0x1000), %g5
  0x00006240:  sub  %g3, %g5, %g3
  0x00006244:  sub  %g3, %g5, %g3

Which seems to be translated into ops ok:

   ---- 00006224 00006228
   mov_i32 g6,$0xffdcf000

   ---- 00006228 0000622c

   ---- 0000622c 00006230
   mov_i32 g4,$0xffd00000

   ---- 00006230 00006234

   ---- 00006234 00006238
   sub_i32 tmp0,g6,g4
   mov_i32 g6,tmp0

   ---- 00006238 0000623c
   sub_i32 tmp0,g1,g6
   mov_i32 g3,tmp0

   ---- 0000623c 00006240
   mov_i32 g5,$0x1000

   ---- 00006240 00006244
   sub_i32 tmp0,g3,g5
   mov_i32 g3,tmp0

   ---- 00006244 00006248
   sub_i32 tmp0,g3,g5
   mov_i32 g3,tmp0

and looks like its doing the expected constant folding here.

   ---- 00006224 00006228

   ---- 00006228 0000622c

   ---- 0000622c 00006230

   ---- 00006230 00006234

   ---- 00006234 00006238
   movi_i32 tmp0,$0xcf000                   pref=0xffff
   mov_i32 g6,tmp0                          dead: 1  pref=0xffff

   ---- 00006238 0000623c
   sub_i32 tmp0,g1,g6                       dead: 1 2  pref=0xffff
   mov_i32 g3,tmp0                          dead: 1  pref=0xffff

   ---- 0000623c 00006240
   mov_i32 g5,$0x1000                       sync: 0  dead: 0  pref=0xffff

   ---- 00006240 00006244
   sub_i32 tmp0,g3,$0x1000                  dead: 1  pref=0xffff
   mov_i32 g3,tmp0                          dead: 1  pref=0xffff

   ---- 00006244 00006248
   sub_i32 tmp0,g3,$0x1000                  dead: 1  pref=0xffff
   mov_i32 g3,tmp0                          sync: 0  dead: 1  pref=0xf038


One other data point is it is certainly in the optimisation phase that
things go wrong because:

  //#define USE_TCG_OPTIMIZATIONS

means the test passes.


>
>
> r~


-- 
Alex Bennée


  reply	other threads:[~2020-04-24 13:24 UTC|newest]

Thread overview: 75+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-04-22  1:16 [PATCH v2 00/36] tcg 5.1 omnibus patch set Richard Henderson
2020-04-22  1:16 ` [PATCH v2 01/36] tcg: Add tcg_gen_gvec_dup_imm Richard Henderson
2020-04-22  1:16 ` [PATCH v2 02/36] target/s390x: Use tcg_gen_gvec_dup_imm Richard Henderson
2020-04-22  1:16 ` [PATCH v2 03/36] target/ppc: " Richard Henderson
2020-04-22  1:16 ` [PATCH v2 04/36] target/arm: " Richard Henderson
2020-04-22  1:16 ` [PATCH v2 05/36] tcg: Use tcg_gen_gvec_dup_imm in logical simplifications Richard Henderson
2020-04-22  1:16 ` [PATCH v2 06/36] tcg: Remove tcg_gen_gvec_dup{8,16,32,64}i Richard Henderson
2020-04-22  1:16 ` [PATCH v2 07/36] tcg: Add tcg_gen_gvec_dup_tl Richard Henderson
2020-04-22  1:16 ` [PATCH v2 08/36] tcg: Improve vector tail clearing Richard Henderson
2020-04-22  1:16 ` [PATCH v2 09/36] tcg: Consolidate 3 bits into enum TCGTempKind Richard Henderson
2020-04-22 11:25   ` Alex Bennée
2020-04-22 19:58   ` Aleksandar Markovic
2020-04-23  9:00     ` Philippe Mathieu-Daudé
2020-04-23 15:40       ` Richard Henderson
2020-04-23 17:24         ` Daniel P. Berrangé
2020-04-23 23:11           ` Richard Henderson
2020-04-24  9:08             ` Daniel P. Berrangé
2020-04-22  1:16 ` [PATCH v2 10/36] tcg: Add temp_readonly Richard Henderson
2020-04-22 11:26   ` Alex Bennée
2020-04-22  1:16 ` [PATCH v2 11/36] tcg: Introduce TYPE_CONST temporaries Richard Henderson
2020-04-22 15:17   ` Alex Bennée
2020-04-22 16:55     ` Richard Henderson
2020-04-22  1:16 ` [PATCH v2 12/36] tcg: Use tcg_constant_i32 with icount expander Richard Henderson
2020-04-22 15:40   ` Alex Bennée
2020-04-22  1:16 ` [PATCH v2 13/36] tcg: Use tcg_constant_{i32, i64} with tcg int expanders Richard Henderson
2020-04-22 16:18   ` [PATCH v2 13/36] tcg: Use tcg_constant_{i32,i64} " Alex Bennée
2020-04-22 17:02     ` Richard Henderson
2020-04-22 17:57       ` Alex Bennée
2020-04-22 20:04   ` Alex Bennée
2020-04-23 23:13     ` Richard Henderson
2020-04-24 13:23       ` Alex Bennée [this message]
2020-04-22  1:17 ` [PATCH v2 14/36] tcg: Use tcg_constant_{i32, vec} with tcg vec expanders Richard Henderson
2020-04-22 17:00   ` [PATCH v2 14/36] tcg: Use tcg_constant_{i32,vec} " Alex Bennée
2020-04-22  1:17 ` [PATCH v2 15/36] tcg: Use tcg_constant_{i32,i64} with tcg plugins Richard Henderson
2020-04-22 17:18   ` [PATCH v2 15/36] tcg: Use tcg_constant_{i32, i64} " Alex Bennée
2020-04-22  1:17 ` [PATCH v2 16/36] tcg: Rename struct tcg_temp_info to TempOptInfo Richard Henderson
2020-04-22 17:19   ` Alex Bennée
2020-04-22  1:17 ` [PATCH v2 17/36] tcg/optimize: Adjust TempOptInfo allocation Richard Henderson
2020-04-22 17:53   ` Alex Bennée
2020-04-22 18:28     ` Alex Bennée
2020-04-22  1:17 ` [PATCH v2 18/36] tcg/optimize: Use tcg_constant_internal with constant folding Richard Henderson
2020-04-22 18:28   ` Alex Bennée
2020-04-22  1:17 ` [PATCH v2 19/36] tcg/tci: Add special tci_movi_{i32,i64} opcodes Richard Henderson
2020-04-22 19:02   ` Alex Bennée
2020-04-22  1:17 ` [PATCH v2 20/36] tcg: Remove movi and dupi opcodes Richard Henderson
2020-04-22  9:12   ` Aleksandar Markovic
2020-04-22 19:03   ` Alex Bennée
2020-04-22  1:17 ` [PATCH v2 21/36] tcg: Use tcg_out_dupi_vec from temp_load Richard Henderson
2020-04-22 19:28   ` Alex Bennée
2020-04-22  1:17 ` [PATCH v2 22/36] tcg: Increase tcg_out_dupi_vec immediate to int64_t Richard Henderson
2020-04-22 19:33   ` Alex Bennée
2020-04-22  1:17 ` [PATCH v2 23/36] tcg: Add tcg_reg_alloc_dup2 Richard Henderson
2020-04-22 19:40   ` Alex Bennée
2020-04-22  1:17 ` [PATCH v2 24/36] tcg/i386: Use tcg_constant_vec with tcg vec expanders Richard Henderson
2020-04-22 19:43   ` Alex Bennée
2020-04-22  1:17 ` [PATCH v2 25/36] tcg: Remove tcg_gen_dup{8,16,32,64}i_vec Richard Henderson
2020-04-23  9:11   ` Alex Bennée
2020-04-22  1:17 ` [PATCH v2 26/36] tcg: Add load_dest parameter to GVecGen2 Richard Henderson
2020-04-23  9:37   ` Alex Bennée
2020-04-22  1:17 ` [PATCH v2 27/36] tcg: Fix integral argument type to tcg_gen_rot[rl]i_i{32, 64} Richard Henderson
2020-04-22 10:19   ` Philippe Mathieu-Daudé
2020-04-23  9:38   ` [PATCH v2 27/36] tcg: Fix integral argument type to tcg_gen_rot[rl]i_i{32,64} Alex Bennée
2020-04-22  1:17 ` [PATCH v2 28/36] tcg: Implement gvec support for rotate by immediate Richard Henderson
2020-04-23 13:28   ` Alex Bennée
2020-04-22  1:17 ` [PATCH v2 29/36] tcg: Implement gvec support for rotate by vector Richard Henderson
2020-04-23 13:41   ` Alex Bennée
2020-04-22  1:17 ` [PATCH v2 30/36] tcg: Remove expansion to shift by vector from do_shifts Richard Henderson
2020-04-22  1:17 ` [PATCH v2 31/36] tcg: Implement gvec support for rotate by scalar Richard Henderson
2020-04-23 13:46   ` Alex Bennée
2020-04-22  1:17 ` [PATCH v2 32/36] tcg/i386: Implement INDEX_op_rotl[is]_vec Richard Henderson
2020-04-22  1:17 ` [PATCH v2 33/36] tcg/aarch64: Implement INDEX_op_rotli_vec Richard Henderson
2020-04-22  1:17 ` [PATCH v2 34/36] tcg/ppc: Implement INDEX_op_rot[lr]v_vec Richard Henderson
2020-04-22  1:17 ` [PATCH v2 35/36] target/ppc: Use tcg_gen_gvec_rotlv Richard Henderson
2020-04-22  1:17 ` [PATCH v2 36/36] target/s390x: Use tcg_gen_gvec_rotl{i,s,v} Richard Henderson
2020-04-23 13:50 ` [PATCH v2 00/36] tcg 5.1 omnibus patch set Alex Bennée

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=87d07x9fwl.fsf@linaro.org \
    --to=alex.bennee@linaro.org \
    --cc=qemu-devel@nongnu.org \
    --cc=richard.henderson@linaro.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.