On Tue, 2018-07-24 at 17:18 +0000, Segher Boessenkool wrote:
> This patch allows combine to combine two insns into two. This helps
> in many cases, by reducing instruction path length, and also allowing
> further combinations to happen. PR85160 is a typical example of code
> that it can improve.
>
> This patch does not allow such combinations if either of the original
> instructions was a simple move instruction. In those cases combining
> the two instructions increases register pressure without improving
> the
> code. With this move test register pressure does no longer increase
> noticably as far as I can tell.
>
> (At first I also didn't allow either of the resulting insns to be a
> move instruction. But that is actually a very good thing to have, as
> should have been obvious).
>
> Tested for many months; tested on about 30 targets.
>
> I'll commit this later this week if there are no objections.
>
>
> Segher
>
>
> 2018-07-24 Segher Boessenkool <seg...@kernel.crashing.org>
>
> PR rtl-optimization/85160
> * combine.c (is_just_move): New function.
> (try_combine): Allow combining two instructions into two if
> neither of
> the original instructions was a move.
>
> ---
> gcc/combine.c | 22 ++++++++++++++++++++--
> 1 file changed, 20 insertions(+), 2 deletions(-)
>
> diff --git a/gcc/combine.c b/gcc/combine.c
> index cfe0f19..d64e84d 100644
> --- a/gcc/combine.c
> +++ b/gcc/combine.c
> @@ -2604,6 +2604,17 @@ can_split_parallel_of_n_reg_sets (rtx_insn
> *insn, int n)
> return true;
> }
>
> +/* Return whether X is just a single set, with the source
> + a general_operand. */
> +static bool
> +is_just_move (rtx x)
> +{
> + if (INSN_P (x))
> + x = PATTERN (x);
> +
> + return (GET_CODE (x) == SET && general_operand (SET_SRC (x),
> VOIDmode));
> +}
If I'm reading it right, the patch only calls this function on i2 and
i3, which are known to be rtx_insn *, rather than just rtx.
Hence the only way in which GET_CODE (x) can be SET is if the INSN_P
pattern test sets x to PATTERN (x) immediately above: it can't be a SET
otherwise - but this isn't obvious from the code.
Can this function take an rtx_insn * instead? Maybe something like:
/* Return whether INSN's pattern is just a single set, with the source
a general_operand. */
static bool
is_just_move_p (rtx_insn *insn)
{
if (!INSN_P (insn))
return false;
rtx x = PATTERN (insn);
return (GET_CODE (x) == SET && general_operand (SET_SRC (x), VOIDmode));
}
or similar?
[...snip...]
Thanks; I hope this is constructive.
Dave
