qemu-devel.nongnu.org archive mirror
 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 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).