Re: [PATCH, rtl-optimization]: Fix PR46829, ICE in spill_failure with -fschedule-insns [was: Fixing instability of -fschedule-insns for x86]

[Oleg Endo <oleg dot endo at t-online dot de>]

Hi,

On Fri, 2012-08-17 at 20:34 +0200, Uros Bizjak wrote:
> On Fri, Aug 17, 2012 at 6:36 PM, Uros Bizjak <ubizjak@gmail.com> wrote:
> 
> > 2012-08-17  Uros Bizjak  <ubizjak@gmail.com>
> >
> >         PR rtl-optimization/46829
> >         * combine.c (recog_for_combine): Check operand constraints
> >         in case hard registers were propagater into insn pattern.
> >
> > testsuite/ChangeLog:
> >
> > 2012-08-17  Uros Bizjak  <ubizjak@gmail.com>
> >
> >         PR rtl-optimization/46829
> >         * gcc.target/i386/pr46829.c: New test.
> >
> > Patch was bootstrapped and regression tested on x86_64-pc-linux-gnu {,-m32}.
> 
> Oh ...
> 
> This part:
> 
> +                     && rtx_equal_p (recog_data.operand[j],
> +                                     recog_data.operand[op_alt[j].matches]))
> 
> should read:
> 
> +                     && rtx_equal_p (recog_data.operand[i],
> +                                     recog_data.operand[op_alt[j].matches]))
> 
> Note the "j" vs "i" array index difference in the first line.
> 
> Correct patch attached, bootstrapped and re-tested on
> x86_64-pc-linux-gnu {,-m32}.
> 

I've briefly checked your patch against rev 190459 for the SH target.
The CSiBE result-size for '-m4 -ml -O2 -mpretend-cmove' shows quite some
movements and improvements.  However, the additional restrictions you've
added effectively disable some of the combine patterns I've been adding
recently to improve code quality on SH and I see some SH specific
failures (haven't run the full test suite though):
FAIL: gcc.target/sh/pr49263.c scan-assembler-not and
FAIL: gcc.target/sh/pr51244-1.c scan-assembler-not movt|tst|negc|extu
FAIL: gcc.target/sh/pr51244-10.c scan-assembler-not shll|subc|and
FAIL: gcc.target/sh/pr51244-7.c scan-assembler-not cmp/hi
FAIL: gcc.target/sh/pr51244-7.c scan-assembler-not mov\t#0
FAIL: gcc.target/sh/pr51244-8.c scan-assembler-not shad|neg
FAIL: gcc.target/sh/pr51244-9.c scan-assembler-not mov\t#0
FAIL: gcc.target/sh/pr52933-1.c scan-assembler-times div0s 25
FAIL: gcc.target/sh/pr52933-2.c scan-assembler-times div0s 25
FAIL: gcc.target/sh/pr54236-1.c scan-assembler-times addc 3
FAIL: gcc.target/sh/pr54236-1.c scan-assembler-times subc 3
FAIL: gcc.target/sh/pr54236-1.c scan-assembler-times sett 4
FAIL: gcc.target/sh/pr54236-1.c scan-assembler-not movt

I've not checked every single case, but the first two seem to be the
core issues.

1) FAIL: gcc.target/sh/pr49263.c scan-assembler-not and

Test function:

int test (uint8_t x)
{
  return x & 15 ? -40 : -9;
}

Pattern in sh.md that would originally match:

(define_insn "tstsi_t_zero_extract_eq"
  [(set (reg:SI T_REG)
	(eq:SI (zero_extract:SI (match_operand 0 "logical_operand" "z")
		(match_operand:SI 1 "const_int_operand")
		(match_operand:SI 2 "const_int_operand"))
         (const_int 0)))]
  ...)

("z" is a constraint for the r0 register on SH)

Combine now says:

Trying 11 -> 12:
Successfully matched this instruction:
(set (reg:SI 147 t)
    (eq:SI (zero_extract:SI (reg:SI 4 r4 [ x ])
            (const_int 4 [0x4])
            (const_int 0 [0]))
        (const_int 0 [0])))
Operand failed to match constraints:
(reg:SI 4 r4 [ x ])

The problem in this case is that on SH (and other targets, too) function
args are passed in regs.  In this case it's the hard reg r4, which is
sort of pre-allocated already.  Before your patch, the pattern would
match and reload would stuff in a 'mov r4,r0' to satisfy the "z"
constraint.

2) FAIL: gcc.target/sh/pr51244-1.c scan-assembler-not movt|tst|negc|extu

Test function:

int
testfunc_01 (int a, int b, int c, int d)
{
  return (a == b || a == d) ? b : c;
}

Pattern in sh.md that would originally match:

(define_insn_and_split "nott"
  [(set (reg:SI T_REG)
	(xor:SI (match_operand:SI 0 "t_reg_operand" "") (const_int 1)))]
  "TARGET_SH1"
  ...)

Combine now says:

Trying 14 -> 15:
Successfully matched this instruction:
(set (reg:SI 147 t)
    (xor:SI (reg:SI 147 t)
        (const_int 1 [0x1])))
Operand failed to match constraints:
(reg:SI 147 t)

In this case the T_REG matching is already in the predicate (similar to
the thing that's being done on x86 you've mentioned before).  I've
started using this kind of matching pattern a while ago to get simpler
and better matching combine patterns around SH's T_REG.
(T_REG is a fixed 1 bit hard reg on SH, treated as SImode throughout the
MD.  It is used to hold comparison results, carry/borrow bits etc).

I'm not sure what would be the possible solutions for those cases.
For 1) maybe pre-allocated regs of args should be moved to pseudos
before combine?
For 2) I could probably try to come up with some matching that would
satisfy the new restrictions in combine and fix up the affected patterns
throughout the MD.

In the worst case, would it be an option to make this restriction in
combine optional (target hook or something)?

Cheers,
Oleg