Re: [PATCH v2 13/36] tcg: Use tcg_constant_{i32,i64} with tcg int expanders

[Alex Bennée]

Richard Henderson <address@hidden> writes:

> On 4/22/20 1:04 PM, Alex Bennée wrote:
>> 
>> Richard Henderson <address@hidden> writes:
>> 
>>> Signed-off-by: Richard Henderson <address@hidden>
>> 
>> 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 <address@hidden>
>>   Date:   Tue Apr 21 18:16:59 2020 -0700
>> 
>>       tcg: Use tcg_constant_{i32,i64} with tcg int expanders
>> 
>>       Signed-off-by: Richard Henderson <address@hidden>
>>       Message-Id: <address@hidden>
>
> 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