On Thu, Dec 19, 2013 at 09:57:36PM -0700, Jeff Law wrote:
> * ree.c (combine_set_extension): Handle case where source
> and destination registers in an extension insn are different.
> (combine_reaching_defs): Allow source and destination
> registers in extension to be different under limited
> circumstances.
> (add_removable_extension): Remove restriction that the
> source and destination registers in the extension are the
> same.
> (find_and_remove_re): Emit a copy from the extension's
> destination to its source after the defining insn if
> the source and destination registers are different.
>
>
> testsuite/
>
> * gcc.target/i386/pr53623.c: New test.
Thanks for working on this, the only thing I'd worry about are
HARD_REGNO_NREGS > 1 registers if the two hard regs might overlap.
Perhaps it is fine as is and dunno how many targets actually allow partial
overlap in between the multi-register REGs. If you aren't sure this
would be handled properly, perhaps the check could go to
add_removable_extension below the REG_P check add
&& !reg_overlap_mentioned_p (dest, XEXP (src, 0))
> diff --git a/gcc/ree.c b/gcc/ree.c
> index 9938e98..63ad86c 100644
> --- a/gcc/ree.c
> +++ b/gcc/ree.c
> @@ -282,9 +282,21 @@ static bool
> combine_set_extension (ext_cand *cand, rtx curr_insn, rtx *orig_set)
> {
> rtx orig_src = SET_SRC (*orig_set);
> - rtx new_reg = gen_rtx_REG (cand->mode, REGNO (SET_DEST (*orig_set)));
> rtx new_set;
>
> + /* If the extension's source/destination registers are not the same
> + then we need to change the original load to reference the destination
> + of the extension. Then we need to emit a copy from that destination
> + to the original destination of the load. */
> + rtx new_reg;
> + bool copy_needed
> + = REGNO (SET_DEST (PATTERN (cand->insn)))
> + != REGNO (XEXP (SET_SRC (PATTERN (cand->insn)), 0));
Perhaps the right formatting here would be
bool copy_needed
= (REGNO (SET_DEST (PATTERN (cand->insn)))
!= REGNO (XEXP (SET_SRC (PATTERN (cand->insn)), 0)));
? ()s for emacs, and aligning != under REGNO.
> + if (copy_needed)
> + new_reg = gen_rtx_REG (cand->mode, REGNO (SET_DEST (PATTERN
> (cand->insn))));
Too long line.
Looks good to me otherwise.
Jakub
