Don't combine across likely spilled hard reg setters (PR rtl-optimization/59477)

Hi!

As discussed in the PR, when combine combines something across a setter
of a likely spilled non-fixed hard register, we may get RA ICE, as the
combined insn might need for reload a hard reg that is live at the point
where combiner has combined the insn.

The following patch attempts to fix that by not creating LOG_LINKS in
certain cases.  There are two cases which the patch handles separately:
1) CALL_INSN and preceeding load_register_parameters sequence
   (from the call insn up to the earliest insn in the sequence that
   sets some likely spilled non-fixed hard register
2) other setters of likely spilled non-fixed hard register
   (loading of return value of a function into the return register(s),
   explicit register vars, ...)

1) is handled specially, by disallowing LOG_LINKS from before the first
   insn into the sequence, because in between that first insn and
   the CALL_INSN a likely spilled hard register is live.  LOG_LINKS from
   before the sequence to after the CALL_INSN or from within the sequence
   to after the call or from within the sequence to within the sequence
   are allowed
2) other setters are handled just like a basic block boundary for LOG_LINKS,
   no LOG_LINKS are allowed to cross that boundary

Bootstrapped/regtested on x86_64-linux and i686-linux.

For i686-linux stage3 + target libraries, just 8 *.o files are affected by
the patch, for x86_64-linux approx. 8% of *.o files are affected
by the patch, though haven't checked in detail how much it affects each
file, so far only looked at x86_64-linux builtins.o, where the patch
pessimized the code in a single spot:
-       movq    (%rax,%rdx), %rdi
+       addq    %rax, %rdx
+       movq    (%rdx), %rdi
        call    _ZL12validate_argPK9tree_node9tree_code
which comes from insn 112 not being combined with 114, because both si
and di are on x86_64 likely spilled non-fixed hard registers and thus we
disallowed LOG_LINKS from before insn 113 into the 113..114 range, because
of the fear that the combined pattern might during reload need the hard
register that is live across it:
(insn 112 111 113 17 (parallel [
            (set (reg/f:DI 134)
                (plus:DI (reg:DI 132)
                    (reg/v:DI 113 [ off ])))
            (clobber (reg:CC 17 flags))
        ]) ../../gcc/gimple.h:2066 266 {*adddi_1}
     (expr_list:REG_DEAD (reg:DI 132)
        (expr_list:REG_DEAD (reg/v:DI 113 [ off ])
            (expr_list:REG_UNUSED (reg:CC 17 flags)
                (nil)))))
(insn 113 112 114 17 (set (reg:SI 4 si)
        (reg:SI 94 [ D.97472 ])) ../../gcc/builtins.c:11417 90 {*movsi_internal}
     (expr_list:REG_DEAD (reg:SI 94 [ D.97472 ])
        (nil)))
(insn 114 113 115 17 (set (reg:DI 5 di)
        (mem/f:DI (reg/f:DI 134) [8 *_43+0 S8 A64])) ../../gcc/builtins.c:11417 89 {*movdi_internal}
     (expr_list:REG_DEAD (reg/f:DI 134)
        (nil)))
(call_insn/c/i 115 114 116 17 (set (reg:QI 0 ax)
        (call (mem:QI (symbol_ref:DI ("_ZL12validate_argPK9tree_node9tree_code") [flags 0x3]  <function_decl 0x7f4045caf200 validate_arg>) [0 validate_arg S1 A8])
            (const_int 0 [0]))) ../../gcc/builtins.c:11417 680 {*call_value}
     (expr_list:REG_DEAD (reg:DI 5 di)
        (expr_list:REG_DEAD (reg:SI 4 si)
            (expr_list:REG_EH_REGION (const_int 0 [0])
                (nil))))
    (expr_list:REG_FRAME_RELATED_EXPR (use (reg:DI 5 di))
        (expr_list:REG_BR_PRED (use (reg:SI 4 si))
            (nil))))

Thoughts on this?  I can surely investigate more tomorrow, look at more
random object files that differ and see how many changes it creates.
Unfortunately, because the patch works at LOG_LINKS creation time, it is
hard to gather statistics during compilation, because while lots of
LOG_LINKS will be affected, only much smaller actual combinations will not
be successfully performed because of the missing LOG_LINKS.
Typically the call argument load sequence contains just (set (reg:.. hard reg) (reg:.. pseudo))
insns and we wouldn't combine into that anyway.

The patch removes the likely_spilled_retval stuff Joern wrote a few years
ago because I believe this change should obsolete that.  But, have tried to
reproduce the ICE using sh-elf target and haven't managed to do so, 4.9 is
quite a different compiler from 2005-ish gcc.

2014-01-15  Jakub Jelinek  <jakub@redhat.com>

	PR rtl-optimization/59477
	* combine.c (create_log_links): For sequences of insns loading
	hard register parameters for a call starting with load of some
	non-fixed likely spilled hard register don't record LOG_LINKS
	from before the first load to any insn in between the first
	load and the call insn.  For setters of non-fixed likely
	spilled hard registers other than in calls and the above sequences
	disallow recoding of any LOG_LINKS crossing such insns.
	(struct likely_spilled_retval_info): Remove.
	(likely_spilled_retval_1, likely_spilled_retval): Remove.
	(try_combine): Don't call likely_spilled_retval.

	* g++.dg/torture/pr59477.C: New test.

	Jakub

Don't combine across likely spilled hard reg setters (PR rtl-optimization/59477)

Commit Message

Comments

Patch