On Sat, May 23, 2015 at 1:33 AM, Alan Modra <amo...@gmail.com> wrote:
> This stops combine messing with parameter and return value copies
> from/to hard registers. Bootstrapped and regression tested
> powerpc64le-linux, powerpc64-linux and x86_64-linux. In looking at a
> number of different powerpc64le gcc/*.o files, I noticed a few code
> generation improvements. There were cases where a register copy was
> no longer needed, cmp used in place of mr., and rlwinm instead of
> rldicl. x86_64 gcc/*.o showed no changes (apart from combine.o of
> course), but should see a small improvement in compile time with this
> change.
I noticed this a while back but I never got around to fixing this.
This should also improve other RISC targets like AARCH64 and MIPS
where I had saw issues like this too.
Thanks,
Andrew
>
> The "clear next_use when !can_combine_p" change is to fix a non-bug.
> Given
>
> 1) insn defining rn
> ...
> 2) insn defining rn but !can_combine_p
> ...
> 3) insn using rn
>
> then create_log_links might create a link from (3) to (1), I thought.
> However, can_combine_p doesn't currently allow this to happen.
> Obviously, any can_combine_p result depending on regno shouldn't give
> a different result at (1) and (2), but there is also at test of
> DF_REF_PRE_POST_MODIFY that can. The saving grace is that pre/post
> modify insns also use the register, which means next_use[rn] will
> point at (2), not (3), when (1) is processed.
>
> I came across this because at one stage I considered modifying
> can_combine_p. Someone who does so in the future might trigger the
> above problem, so I thought it worth posting the change.
>
> OK for mainline?
>
> * combine.c (set_return_regs): New function.
> (twiddle_first_block, twiddle_last_block): New functions.
> (create_log_links): Exclude instructions copying parameter
> values from hard regs to pseudos, and instructions copying
> return value pseudos to hard regs. Clear next_use when
> !can_combine_p.
>
> Index: gcc/combine.c
> ===================================================================
> --- gcc/combine.c (revision 223463)
> +++ gcc/combine.c (working copy)
> @@ -1048,9 +1048,79 @@ can_combine_use_p (df_ref use)
> return true;
> }
>
> -/* Fill in log links field for all insns. */
> +/* Used to build set of return value regs. Add X to the set. */
>
> static void
> +set_return_regs (rtx x, void *arg)
> +{
> + HARD_REG_SET *regs = (HARD_REG_SET *) arg;
> +
> + add_to_hard_reg_set (regs, GET_MODE (x), REGNO (x));
> +}
> +
> +/* Twiddle BLOCK_FOR_INSN to TO for instructions in the first block BB
> + that we don't want to combine with other instructions. */
> +
> +static void
> +twiddle_first_block (basic_block bb, basic_block to)
> +{
> + rtx_insn *insn;
> +
> + FOR_BB_INSNS (bb, insn)
> + {
> + if (NOTE_P (insn) && NOTE_KIND (insn) == NOTE_INSN_FUNCTION_BEG)
> + break;
> + if (!NONDEBUG_INSN_P (insn))
> + continue;
> +
> + /* reg,reg copies from parameter hard regs. */
> + rtx set = single_set (insn);
> + if (set
> + && REG_P (SET_DEST (set))
> + && REG_P (SET_SRC (set))
> + && HARD_REGISTER_P (SET_SRC (set)))
> + set_block_for_insn (insn, to);
> + }
> +}
> +
> +/* Twiddle BLOCK_FOR_INSN to TO for instructions in the last block BB
> + that we don't want to combine with other instructions. */
> +
> +static void
> +twiddle_last_block (basic_block bb, basic_block to, HARD_REG_SET return_regs)
> +{
> + rtx_insn *insn;
> +
> + FOR_BB_INSNS_REVERSE (bb, insn)
> + {
> + if (CALL_P (insn))
> + break;
> + if (!NONDEBUG_INSN_P (insn))
> + continue;
> +
> + rtx reg = NULL_RTX;
> + /* use_return_regster added USEs. */
> + if (GET_CODE (PATTERN (insn)) == USE)
> + reg = XEXP (PATTERN (insn), 0);
> + else
> + {
> + /* reg,reg copies that set return value hard regs. */
> + rtx set = single_set (insn);
> + if (set && REG_P (SET_SRC (set)))
> + reg = SET_DEST (set);
> + }
> + if (reg
> + && REG_P (reg)
> + && HARD_REGISTER_P (reg)
> + && overlaps_hard_reg_set_p (return_regs,
> + GET_MODE (reg), REGNO (reg)))
> + set_block_for_insn (insn, to);
> + }
> +}
> +
> +/* Fill in log links field for all insns that we wish to combine. */
> +
> +static void
> create_log_links (void)
> {
> basic_block bb;
> @@ -1057,9 +1127,28 @@ create_log_links (void)
> rtx_insn **next_use;
> rtx_insn *insn;
> df_ref def, use;
> + HARD_REG_SET return_regs;
>
> next_use = XCNEWVEC (rtx_insn *, max_reg_num ());
>
> + /* Don't combine instructions copying parameter values from hard
> + regs to pseudos. Exclude such instructions from LOG_LINKS by
> + temporarily zapping BLOCK_FOR_INSN. */
> +
> + bb = ENTRY_BLOCK_PTR_FOR_FN (cfun)->next_bb;
> + twiddle_first_block (bb, 0);
> +
> + /* Similarly, don't combine instructions copying return values
> + from pseudos to hard regs. */
> +
> + CLEAR_HARD_REG_SET (return_regs);
> + diddle_return_value (set_return_regs, &return_regs);
> + if (!hard_reg_set_empty_p (return_regs))
> + {
> + bb = EXIT_BLOCK_PTR_FOR_FN (cfun)->prev_bb;
> + twiddle_last_block (bb, 0, return_regs);
> + }
> +
> /* Pass through each block from the end, recording the uses of each
> register and establishing log links when def is encountered.
> Note that we do not clear next_use array in order to save time,
> @@ -1087,12 +1176,12 @@ create_log_links (void)
> if (!next_use[regno])
> continue;
>
> + use_insn = next_use[regno];
> + next_use[regno] = NULL;
> +
> if (!can_combine_def_p (def))
> continue;
>
> - use_insn = next_use[regno];
> - next_use[regno] = NULL;
> -
> if (BLOCK_FOR_INSN (use_insn) != bb)
> continue;
>
> @@ -1103,7 +1192,7 @@ create_log_links (void)
> we might wind up changing the semantics of the insn,
> even if reload can make what appear to be valid
> assignments later. */
> - if (regno < FIRST_PSEUDO_REGISTER
> + if (HARD_REGISTER_NUM_P (regno)
> && asm_noperands (PATTERN (use_insn)) >= 0)
> continue;
>
> @@ -1124,6 +1213,16 @@ create_log_links (void)
> }
> }
>
> + /* Repair BLOCK_FOR_INSN. */
> +
> + bb = ENTRY_BLOCK_PTR_FOR_FN (cfun)->next_bb;
> + twiddle_first_block (bb, bb);
> + if (!hard_reg_set_empty_p (return_regs))
> + {
> + bb = EXIT_BLOCK_PTR_FOR_FN (cfun)->prev_bb;
> + twiddle_last_block (bb, bb, return_regs);
> + }
> +
> free (next_use);
> }
>
>
> --
> Alan Modra
> Australia Development Lab, IBM
