On Fri, Jan 10, 2014 at 02:36:47PM -0700, Jeff Law wrote:
> As mentioned in the PR. We have a memory load and two extensions.
>
> The first extension requires a copy because its source and
> destination are not the same hard register.
>
> The second extension does not require a copy and has a wider mode
> than the first extension.
>
> In this case we have to make sure to widen the copy we emit to
> eliminate the first extension. Otherwise upper bits aren't going to
> have their expected values. Thankfully the copy isn't emitted until
> the end of ree and we have the modified memory reference to peek at
> to get the proper mode.
>
> Bootstrapped and regression tested on x86_64-unknown-linux-gnu. OK
> for the trunk?
> --- a/gcc/ree.c
> +++ b/gcc/ree.c
> @@ -1003,11 +1003,20 @@ find_and_remove_re (void)
> for (unsigned int i = 0; i < reinsn_copy_list.length (); i += 2)
> {
> rtx curr_insn = reinsn_copy_list[i];
> + rtx def_insn = reinsn_copy_list[i + 1];
> +
> + /* Use the mode of the destination of the defining insn
> + for the mode of the copy. This is necessary if the
> + defining insn was used to eliminate a second extension
> + that was wider than the first. */
> + rtx sub_rtx = *get_sub_rtx (def_insn);
> rtx pat = PATTERN (curr_insn);
> - rtx new_reg = gen_rtx_REG (GET_MODE (SET_DEST (pat)),
> + rtx new_dst = gen_rtx_REG (GET_MODE (SET_DEST (sub_rtx)),
> REGNO (XEXP (SET_SRC (pat), 0)));
> - rtx set = gen_rtx_SET (VOIDmode, new_reg, SET_DEST (pat));
> - emit_insn_after (set, reinsn_copy_list[i + 1]);
> + rtx new_src = gen_rtx_REG (GET_MODE (SET_DEST (sub_rtx)),
> + REGNO (SET_DEST (pat)));
> + rtx set = gen_rtx_SET (VOIDmode, new_dst, new_src);
> + emit_insn_after (set, def_insn);
> }
There is one thing I still worry about, if some target has
an insn to say sign extend or zero extend a short memory load
into HARD_REGNO_NREGS () > 1 register, but the other involved
register has the only one (or fewer) hard registers available to it.
Consider registers SImode hard registers 0, 1, 2, 3:
(set (reg:SI 3) (something:SI))
(set (reg:HI 0) (expression:HI))
(set (reg:SI 2) (sign_extend:SI (reg:HI 0)))
(set (reg:DI 0) (sign_extend:DI (reg:HI 0)))
(use (reg:SI 3))
we transform this into:
(set (reg:SI 3) (something:SI))
(set (reg:SI 2) (sign_extend:SI (expression:HI)))
(set (reg:SI 0) (reg:HI 2))
(set (reg:DI 0) (sign_extend:DI (reg:HI 0)))
(use (reg:SI 3))
first (well, the middle is then pending in copy list), and next:
(set (reg:SI 3) (something))
(set (reg:DI 2) (sign_extend:DI (expression:HI)))
(set (reg:DI 0) (reg:DI 2))
(use (reg:SI 3))
but that looks wrong, because the second instruction would now clobber
(reg:SI 3). Dunno if we have such an target and thus if it is possible
to construct a testcase.
So, I'd say the handling of the second extend should notice that
it is actually extending load into a different register and bail out
if it would need more hard registers than it needed previously, or
something similar.
> --- /dev/null
> +++ b/gcc/testsuite/gcc.c-torture/execute/pr59747.c
> @@ -0,0 +1,28 @@
> +extern void abort (void) __attribute__ ((__noreturn__));
> +extern void exit (int) __attribute__ ((__noreturn__));
> +extern int printf (const char *, ...);
The printf prototype isn't needed, and the noreturn attributes
aren't needed either, as they are builtins, the compiler will
add those automatically.
Jakub
