Re: [SH] ICE compiling pr34330 testcase for sh-linux-gnu

Re: [SH] ICE compiling pr34330 testcase for sh-linux-gnu

[Joern Rennecke]

> This is then transformed by subst_reloads to the final broken form:
>
> (insn 171 170 172 5 .../pr34330.c:17 (set (reg:SI 9 r9)
>          (plus:SI (reg:SI 8 r8)
>              (reg:SI 0 r0 [orig:243 ivtmp.11 ] [243]))) -1 (nil))
>
> This is logically correct as r9 genuinely does contain the result of the
> substituted expression, but it does not satisfy the constraints.
>
> And here's where I get stuck. I don't know where in the code it's
> supposed to check that the code will be correct after the substitution.
>
> The (plus (plus ...) ...) rtl is generated by
> find_reloads_subreg_address using make_memloc and plus_constant, and it
> seems correct in itself.

This used to be 'handled' by reload register allocation which would
use the same reload register for RELOAD_FOR_INPUT_ADDRESS as for
RELOAD_FOR_INPADDR_ADDREESS if both are for the same operand.
It is strange that you have two RELOAD_FOR_INPUT_ADDRESS reloads
instead.

Although, in terms of ensuring correctness, it would be saner if
gen_reload would make sure it generates valid code even for unusual
reload register allocations.
Unfortunately, that'll require it to make a guess if a two-address add
is required; you can scan the insn pattern for matching constraints, but
that is somewhat hit-and-miss (obviously you can't use constrain_operands
there unless you want to save / restore all the extraction data); you'll
end up generating some unneeded register-register moves on some
architectures.  But you could make sure they get removed in postreload.

Re: [SH] ICE compiling pr34330 testcase for sh-linux-gnu

[Andrew Stubbs]

On 09/07/09 19:11, Ian Lance Taylor wrote:
> Andrew Stubbs<ams@codesourcery.com>  writes:
>
>> The problem insn is created by gen_reload when it is given the
>> following rtl as input:
>>
>> (plus:SI (plus:SI (reg/v/f:SI 4 r4 [orig:192 a ] [192])
>>          (const_int 2 [0x2]))
>>      (reg:SI 0 r0 [orig:188 ivtmp.24 ] [188]))
>
> You need to backtrack before that point to see why find_reloads let that
> go through.

OK, I've gone through it all trying to understand it, but this is fairly 
complex code and I don't claim to get it.

Here's my analysis of the problem.

The problem instruction in pr34330.c.181r.sched1 (the state before 
register allocation/reload) is:

(insn 97 103 111 5 .../pr34330.c:17 (set (mem/s:HI (reg/f:SI 239 [ 
D.1960 ]) [6 D.1960_8->s1+0 S2 A16])
         (subreg:HI (reg:SI 257) 2)) 187 {movhi_i} (expr_list:REG_DEAD 
(reg:SI 257)
         (nil)))

The reloads for this instruction are:

Reloads for insn # 97
Reload 0: reload_in (SI) = (plus:SI (reg/v/f:SI 4 r4 [orig:250 a ] [250])
                                                     (const_int 2 [0x2]))
         GENERAL_REGS, RELOAD_FOR_INPUT_ADDRESS (opnum = 1)
         reload_in_reg: (plus:SI (reg/v/f:SI 4 r4 [orig:250 a ] [250])
                                                     (const_int 2 [0x2]))
          reload_reg_rtx: (reg:SI 8 r8)
Reload 1: reload_in (SI) = (plus:SI (plus:SI (reg/v/f:SI 4 r4 [orig:250 
a ] [250])
                                                         (const_int 2 
[0x2]))
                                                     (reg:SI 0 r0 
[orig:243 ivtmp.11 ] [243]))
         GENERAL_REGS, RELOAD_FOR_INPUT_ADDRESS (opnum = 1)
         reload_in_reg: (plus:SI (plus:SI (reg/v/f:SI 4 r4 [orig:250 a ] 
[250])
                                                         (const_int 2 
[0x2]))
                                                     (reg:SI 0 r0 
[orig:243 ivtmp.11 ] [243]))
         reload_reg_rtx: (reg:SI 9 r9)
Reload 2: reload_out (HI) = (mem/s:HI (reg/f:SI 2 r2 [orig:239 D.1960 ] 
[239]) [6 D.1960_8->s1+0 S2 A16])
         NO_REGS, RELOAD_FOR_OUTPUT (opnum = 0), optional
         reload_out_reg: (mem/s:HI (reg/f:SI 2 r2 [orig:239 D.1960 ] 
[239]) [6 D.1960_8->s1+0 S2 A16])
Reload 3: reload_in (HI) = (mem:HI (plus:SI (plus:SI (reg/v/f:SI 4 r4 
[orig:250 a ] [250])
                                                             (const_int 
2 [0x2]))
                                                         (reg:SI 0 r0 
[orig:243 ivtmp.11 ] [243])) [3 S4 A32])
         GENERAL_REGS, RELOAD_FOR_INPUT (opnum = 1), can't combine
         reload_in_reg: (subreg:HI (reg:SI 257) 2)
         reload_reg_rtx: (reg:HI 8 r8)


Which results in this instruction sequence in pr34330.c.183r.ira:

(insn 169 103 170 4 .../pr34330.c:17 (set (reg:SI 8 r8)
         (const_int 2 [0x2])) 175 {movsi_ie} (nil))

(insn 170 169 171 4 .../pr34330.c:17 (set (reg:SI 8 r8)
         (plus:SI (reg:SI 8 r8)
             (reg/v/f:SI 4 r4 [orig:250 a ] [250]))) 35 
{*addsi3_compact} (expr_list:REG_EQUIV (plus:SI (reg/v/f:SI 4 r4 
[orig:250 a ] [250])
             (const_int 2 [0x2]))
         (nil)))

(insn 171 170 172 4 .../pr34330.c:17 (set (reg:SI 9 r9)
         (plus:SI (reg:SI 8 r8)
             (reg:SI 0 r0 [orig:243 ivtmp.11 ] [243]))) 35 
{*addsi3_compact} (nil))

(insn 172 171 97 4 .../pr34330.c:17 (set (reg:HI 8 r8)
         (mem:HI (reg:SI 9 r9) [3 S4 A32])) 187 {movhi_i} (nil))

(insn 97 172 111 4 .../pr34330.c:17 (set (mem/s:HI (reg/f:SI 2 r2 
[orig:239 D.1960 ] [239]) [6 D.1960_8->s1+0 S2 A16])
         (reg:HI 8 r8)) 187 {movhi_i} (nil))


The problem is in insn 171 where r9 != r8 and therefore cannot match an 
SH 2-operand add instruction.

Looking back at how this happens, insn 171 is created by gen_reload from 
reload #1 and initially looks like this:

(insn 171 170 172 5 .../pr34330.c:17 (set (reg:SI 9 r9)
         (plus:SI (plus:SI (reg/v/f:SI 4 r4 [orig:250 a ] [250])
                 (const_int 2 [0x2]))
             (reg:SI 0 r0 [orig:243 ivtmp.11 ] [243]))) -1 (nil))

This is then transformed by subst_reloads to the final broken form:

(insn 171 170 172 5 .../pr34330.c:17 (set (reg:SI 9 r9)
         (plus:SI (reg:SI 8 r8)
             (reg:SI 0 r0 [orig:243 ivtmp.11 ] [243]))) -1 (nil))

This is logically correct as r9 genuinely does contain the result of the 
substituted expression, but it does not satisfy the constraints.

And here's where I get stuck. I don't know where in the code it's 
supposed to check that the code will be correct after the substitution.

The (plus (plus ...) ...) rtl is generated by 
find_reloads_subreg_address using make_memloc and plus_constant, and it 
seems correct in itself.

Any help would be appreciated.

Thanks

Andrew

Re: [SH] ICE compiling pr34330 testcase for sh-linux-gnu

[Maxim Kuvyrkov]

Andrew Stubbs wrote:
> I'm having trouble with an ICE, and I'm hoping somebody can enlighten me.
> 
> Given the following command:
> 
> cc1 -fpreprocessed ../pr34330.i -quiet -dumpbase pr34330.c -da -mb 
> -auxbase-strip pr34330.c -Os -version -ftree-parallelize-loops=4 
> -ftree-vectorize -o pr34330.s -fschedule-insns
> 
> I get an internal compiler error:
> 
> GNU C (GCC) version 4.5.0 20090702 (experimental) (sh-linux-gnu)
>         compiled by GNU C version 4.3.2, GMP version 4.3.1, MPFR version 
> 2.4.1-p5, MPC version 0.6
> GGC heuristics: --param ggc-min-expand=30 --param ggc-min-heapsize=4096
> GNU C (GCC) version 4.5.0 20090702 (experimental) (sh-linux-gnu)
>         compiled by GNU C version 4.3.2, GMP version 4.3.1, MPFR version 
> 2.4.1-p5, MPC version 0.6
> GGC heuristics: --param ggc-min-expand=30 --param ggc-min-heapsize=4096
> Compiler executable checksum: c91a929a0209c0670a3ae8b8067b9f9a
> /scratch/ams/4.4-sh-linux-gnu-lite/src/gcc-trunk-4.4/gcc/testsuite/gcc.dg/torture/pr34330.c: 
> In function 'foo':
> /scratch/ams/4.4-sh-linux-gnu-lite/src/gcc-trunk-4.4/gcc/testsuite/gcc.dg/torture/pr34330.c:22:1: 
> error: insn does not satisfy its constraints:
> (insn 171 170 172 4 
> /scratch/ams/4.4-sh-linux-gnu-lite/src/gcc-trunk-4.4/gcc/testsuite/gcc.dg/torture/pr34330.c:17 
> (set (reg:SI 9 r9)
>         (plus:SI (reg:SI 8 r8)
>             (reg:SI 0 r0 [orig:243 ivtmp.11 ] [243]))) 35 
> {*addsi3_compact} (nil))
> /scratch/ams/4.4-sh-linux-gnu-lite/src/gcc-trunk-4.4/gcc/testsuite/gcc.dg/torture/pr34330.c:22:1: 
> internal compiler error: in reload_cse_simplify_operands, at 
> postreload.c:396

This looks much like PR37053 on m68k/ColdFire; the easiest way to check 
if this ICE was caused by the same error is to revert hunk in rtlanal.c: 
commutative_operand_precedence() -- see in the PR.

As to the fix, there are several patches being discussed here 
(http://gcc.gnu.org/ml/gcc-patches/2009-07/msg00816.html) and here 
(http://gcc.gnu.org/ml/gcc-patches/2009-07/msg00823.html).

My $0.02.

--
Maxim

Re: [SH] ICE compiling pr34330 testcase for sh-linux-gnu

[Ian Lance Taylor]

Andrew Stubbs <ams@codesourcery.com> writes:

> The problem insn is created by gen_reload when it is given the
> following rtl as input:
>
> (plus:SI (plus:SI (reg/v/f:SI 4 r4 [orig:192 a ] [192])
>         (const_int 2 [0x2]))
>     (reg:SI 0 r0 [orig:188 ivtmp.24 ] [188]))

You need to backtrack before that point to see why find_reloads let that
go through.

Ian

[SH] ICE compiling pr34330 testcase for sh-linux-gnu

[Andrew Stubbs]

I'm having trouble with an ICE, and I'm hoping somebody can enlighten me.

Given the following command:

cc1 -fpreprocessed ../pr34330.i -quiet -dumpbase pr34330.c -da -mb 
-auxbase-strip pr34330.c -Os -version -ftree-parallelize-loops=4 
-ftree-vectorize -o pr34330.s -fschedule-insns

I get an internal compiler error:

GNU C (GCC) version 4.5.0 20090702 (experimental) (sh-linux-gnu)
         compiled by GNU C version 4.3.2, GMP version 4.3.1, MPFR 
version 2.4.1-p5, MPC version 0.6
GGC heuristics: --param ggc-min-expand=30 --param ggc-min-heapsize=4096
GNU C (GCC) version 4.5.0 20090702 (experimental) (sh-linux-gnu)
         compiled by GNU C version 4.3.2, GMP version 4.3.1, MPFR 
version 2.4.1-p5, MPC version 0.6
GGC heuristics: --param ggc-min-expand=30 --param ggc-min-heapsize=4096
Compiler executable checksum: c91a929a0209c0670a3ae8b8067b9f9a
/scratch/ams/4.4-sh-linux-gnu-lite/src/gcc-trunk-4.4/gcc/testsuite/gcc.dg/torture/pr34330.c: 
In function 'foo':
/scratch/ams/4.4-sh-linux-gnu-lite/src/gcc-trunk-4.4/gcc/testsuite/gcc.dg/torture/pr34330.c:22:1: 
error: insn does not satisfy its constraints:
(insn 171 170 172 4 
/scratch/ams/4.4-sh-linux-gnu-lite/src/gcc-trunk-4.4/gcc/testsuite/gcc.dg/torture/pr34330.c:17 
(set (reg:SI 9 r9)
         (plus:SI (reg:SI 8 r8)
             (reg:SI 0 r0 [orig:243 ivtmp.11 ] [243]))) 35 
{*addsi3_compact} (nil))
/scratch/ams/4.4-sh-linux-gnu-lite/src/gcc-trunk-4.4/gcc/testsuite/gcc.dg/torture/pr34330.c:22:1: 
internal compiler error: in reload_cse_simplify_operands, at 
postreload.c:396
Please submit a full bug report,
with preprocessed source if appropriate.
See <http://gcc.gnu.org/bugs.html> for instructions.

The problem is that r8 != r9 but SH requires that it is.

The problem insn is created by gen_reload when it is given the following 
rtl as input:

(plus:SI (plus:SI (reg/v/f:SI 4 r4 [orig:192 a ] [192])
         (const_int 2 [0x2]))
     (reg:SI 0 r0 [orig:188 ivtmp.24 ] [188]))

The problem appears to be that the nested plus does not match any of the 
patterns it recognizes so it falls through to the final else clause:

   /* Otherwise, just write (set OUT IN) and hope for the best.  */
   else
     emit_insn (gen_rtx_SET (VOIDmode, out, in));

... which doesn't even attempt to check the constraints.

Is this an unexpected corner case for reload? Or is the input RTL 
mal-formed somehow?

This case fails in both GCC 4.4 and SVN trunk (although the latter has 
disabled -fschedule-insns by default so it needs to be re-enabled 
explicitly).

Thanks for an help.

Andrew