This is the mail archive of the gcc-patches@gcc.gnu.org mailing list for the GCC project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

Re: [PATCH] combine: Allow combining two insns to two insns


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  <segher@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


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]