This is the mail archive of the
gcc-patches@gcc.gnu.org
mailing list for the GCC project.
Re: [RFA] [PATCH][PR tree-optimization/59749] Fix recently introduced ree bug
- From: Jakub Jelinek <jakub at redhat dot com>
- To: Jeff Law <law at redhat dot com>
- Cc: gcc-patches <gcc-patches at gcc dot gnu dot org>
- Date: Mon, 13 Jan 2014 08:34:30 +0100
- Subject: Re: [RFA] [PATCH][PR tree-optimization/59749] Fix recently introduced ree bug
- Authentication-results: sourceware.org; auth=none
- References: <52D067EF dot 7020005 at redhat dot com> <20140110215227 dot GJ892 at tucnak dot redhat dot com> <52D38607 dot 4090700 at redhat dot com>
- Reply-to: Jakub Jelinek <jakub at redhat dot com>
On Sun, Jan 12, 2014 at 11:21:59PM -0700, Jeff Law wrote:
> --- a/gcc/ree.c
> +++ b/gcc/ree.c
> @@ -297,6 +297,13 @@ combine_set_extension (ext_cand *cand, rtx curr_insn, rtx *orig_set)
> else
> new_reg = gen_rtx_REG (cand->mode, REGNO (SET_DEST (*orig_set)));
>
> + /* We're going to be widening the result of DEF_INSN, ensure that doing so
> + doesn't change the number of hard registers needed for the result. */
> + if (HARD_REGNO_NREGS (REGNO (new_reg), cand->mode)
> + != HARD_REGNO_NREGS (REGNO (SET_SRC (*orig_set)),
Note you can use orig_src instead of SET_SRC (*orig_set) here.
> + GET_MODE (SET_DEST (*orig_set))))
> + return false;
> +
> /* Merge constants by directly moving the constant into the register under
> some conditions. Recall that RTL constants are sign-extended. */
> if (GET_CODE (orig_src) == CONST_INT
Are you sure the above is needed even for the
REGNO (new_reg) == REGNO (SET_DEST (*orig_set))
&& REGNO (new_reg) == REGNO (orig_src) case?
I mean in that case no copy insn is going to be scheduled right now, nor has
been previously scheduled, so we are back to what the code did before
r206418. I can imagine it can be a problem, but doesn't have to be.
(set (reg:SI 3) (something:SI))
(set (reg:SI 2) (expression:SI)) // def_insn
(use (reg:SI 3))
(set (reg:DI 3) (sign_extend:DI (reg:SI 2)))
So, perhaps if we wanted to handle the HARD_REGNO_NREGS != HARD_REGNO_NREGS
case when all 3 REGNOs are the same, we'd need to limit it to the case where
cand->insn and curr_insn are in the same bb, DF_INSN_LUID of curr_insn
is smaller than DF_INSN_LUID of cand->insn and the extra hard regs aren't
used between the two. Perhaps not worth it?
BTW, I'm surprised to hear that it triggers in the testsuite already (for
the 3 REGNOs the same case, or different?), is that on x86_64 or i?86?
Do you have an example? I'm surprised that we'd have post reload a pattern
that extends into multiple hard registers.
Jakub