On Tue, Nov 06, 2007 at 03:30:04PM +0100, Ulrich Weigand wrote:
> Mark Mitchell wrote:
> > H.J. Lu wrote:
> >
> > > http://gcc.gnu.org/ml/gcc-patches/2007-08/msg01865.html
> > >
> > > which involves reload.
> >
> > I'm not going to try to wade into reload. Ulrich, Eric, Ian -- would
> > one of you please review this patch?
>
> @@ -1821,6 +1835,18 @@ find_reg (struct insn_chain *chain, int
> this_cost--;
> if (rl->out && REG_P (rl->out) && REGNO (rl->out) == regno)
> this_cost--;
> +#ifdef SECONDARY_MEMORY_NEEDED
> + /* If a memory location is needed for rl->in and dest_reg
> + is usable, we will favor it. */
> + else if (dest_reg == regno
> + && rl->in
> + && REG_P (rl->in)
> + && REGNO (rl->in) < FIRST_PSEUDO_REGISTER
> + && SECONDARY_MEMORY_NEEDED (REGNO_REG_CLASS (REGNO (rl->in)),
> + rl->class,
> + rl->mode))
> + this_cost = 0;
> +#endif
>
>
> Hmm, this isn't really related to secondary memory. In general,
> if we have a simple move with input reload, the destination of
> the move should be the preferred reload register. In fact, there
> already is code in find_reloads that is supposed to address this
> problem:
>
> /* Special case a simple move with an input reload and a
> destination of a hard reg, if the hard reg is ok, use it. */
> for (i = 0; i < n_reloads; i++)
> if (rld[i].when_needed == RELOAD_FOR_INPUT
> && GET_CODE (PATTERN (insn)) == SET
> && REG_P (SET_DEST (PATTERN (insn)))
> && SET_SRC (PATTERN (insn)) == rld[i].in
> && !elimination_target_reg_p (SET_DEST (PATTERN (insn))))
> {
>
> This does not trigger in the given test case because the SUBREG
> interferes:
>
> Reloads for insn # 6
> Reload 0: reload_in (DF) = (reg:DF 5 di)
> SSE_REGS, RELOAD_FOR_INPUT (opnum = 1), can't combine
> reload_in_reg: (subreg:DF (reg/v:DI 5 di [orig:59 in ] [59]) 0)
>
> (insn:HI 6 3 10 2 xxx.i:4
> (set (reg:DF 21 xmm0 [orig:58 <result> ] [58])
> (subreg:DF (reg/v:DI 5 di [orig:59 in ] [59]) 0)) 102
> {*movdf_integer_rex64} (expr_list:REG_DEAD (reg/v:DI 5 di [orig:59 in ] [59])
> (nil)))
>
> Note how reload_in is not equal to the SET_SRC, but reload_in_reg is.
> In that case, the same special case should apply.
>
> The following patch fixes the test case for me:
>
> Index: gcc/reload.c
> ===================================================================
> --- gcc/reload.c (revision 129925)
> +++ gcc/reload.c (working copy)
> @@ -4462,7 +4462,8 @@
> if (rld[i].when_needed == RELOAD_FOR_INPUT
> && GET_CODE (PATTERN (insn)) == SET
> && REG_P (SET_DEST (PATTERN (insn)))
> - && SET_SRC (PATTERN (insn)) == rld[i].in
> + && (SET_SRC (PATTERN (insn)) == rld[i].in
> + || SET_SRC (PATTERN (insn)) == rld[i].in_reg)
> && !elimination_target_reg_p (SET_DEST (PATTERN (insn))))
> {
> rtx dest = SET_DEST (PATTERN (insn));
>
>
> H.J., could you verify that this solves your problem?
>
Yes, it works for me. I tested it on Linux/ia32, Linux/intel64
and linux/ia64. There are no regressions.
Thanks.
H.J.
----
gcc/
2007-11-06 Ulrich Weigand <[EMAIL PROTECTED]>
PR target/30961
* reload1.c (find_reloads): Also check in_reg when handling a
simple move with an input reload and a destination of a hard
register.
gcc/testsuite/
2007-11-06 H.J. Lu <[EMAIL PROTECTED]>
PR target/30961
* gcc.target/i386/pr30961-1.c: New.
--- gcc/reload.c.second 2007-09-08 09:50:55.000000000 -0700
+++ gcc/reload.c 2007-11-06 07:43:52.000000000 -0800
@@ -4464,7 +4464,8 @@ find_reloads (rtx insn, int replace, int
if (rld[i].when_needed == RELOAD_FOR_INPUT
&& GET_CODE (PATTERN (insn)) == SET
&& REG_P (SET_DEST (PATTERN (insn)))
- && SET_SRC (PATTERN (insn)) == rld[i].in)
+ && (SET_SRC (PATTERN (insn)) == rld[i].in
+ || SET_SRC (PATTERN (insn)) == rld[i].in_reg))
{
rtx dest = SET_DEST (PATTERN (insn));
unsigned int regno = REGNO (dest);
--- gcc/testsuite/gcc.target/i386/pr30961-1.c.second 2007-11-06
07:41:26.000000000 -0800
+++ gcc/testsuite/gcc.target/i386/pr30961-1.c 2007-11-06 07:41:26.000000000
-0800
@@ -0,0 +1,13 @@
+/* { dg-do compile } */
+/* { dg-require-effective-target lp64 } */
+/* { dg-options "-O2" } */
+
+double
+convert (long long in)
+{
+ double f;
+ __builtin_memcpy( &f, &in, sizeof( in ) );
+ return f;
+}
+
+/* { dg-final { scan-assembler-not "movapd" } } */
