Patchwork PATCH: PR middle-end/47449: =?utf-8?B?W3gz?= =?utf-8?Q?2=5D_can=E2=80=99t_find_a_register_in_class_=E2=80=98DIREG?= =?utf-8?B?4oCZIHdoaWxlIHJlbG9hZGluZyDigJhhc23igJk=?=

login
register
mail settings
Submitter H.J. Lu
Date Jan. 25, 2011, 2:16 a.m.
Message ID <20110125021652.GA21397@intel.com>
Download mbox | patch
Permalink /patch/80302/
State New
Headers show

Comments

H.J. Lu - Jan. 25, 2011, 2:16 a.m.
Hi,

fwprop propagates zero/sign extended hard registers while not propagating 
hard registers in propagate_rtx. This patch avoids propagating zero/sign
extended hard registers.  OK for 4.7?

Thanks.


H.J.
---
commit 9fe3f4242e370b5728223e4198eb34b82eaa7e33
Author: H.J. Lu <hjl.tools@gmail.com>
Date:   Mon Jan 24 18:11:35 2011 -0800

    Don't propagate zero/sign extended hard register.
Paolo Bonzini - Jan. 25, 2011, 3:02 a.m.
On 01/25/2011 03:16 AM, H.J. Lu wrote:
> Hi,
>
> fwprop propagates zero/sign extended hard registers while not propagating
> hard registers in propagate_rtx. This patch avoids propagating zero/sign
> extended hard registers.  OK for 4.7?

Is this a regression from say 3.4 or 4.1?  The patch surely fixes a 
thinko, and the same ought to be done in forward_propagate_subreg too.

fwprop isn't meant to deal with hard registers, especially on 
SMALL_REGISTER_CLASSES machine its transforms are the exact opposite of 
what you ought to do...

Paolo
H.J. Lu - Jan. 25, 2011, 3:50 a.m.
On Mon, Jan 24, 2011 at 7:02 PM, Paolo Bonzini <bonzini@gnu.org> wrote:
> On 01/25/2011 03:16 AM, H.J. Lu wrote:
>>
>> Hi,
>>
>> fwprop propagates zero/sign extended hard registers while not propagating
>> hard registers in propagate_rtx. This patch avoids propagating zero/sign
>> extended hard registers.  OK for 4.7?
>
> Is this a regression from say 3.4 or 4.1?  The patch surely fixes a thinko,
> and the same ought to be done in forward_propagate_subreg too.

This is a regression from gcc 4.4 since propagating zero/sign extended hard
registers was added to gcc 4.5:

http://gcc.gnu.org/ml/gcc-cvs/2009-08/msg00704.html

> fwprop isn't meant to deal with hard registers, especially on
> SMALL_REGISTER_CLASSES machine its transforms are the exact opposite of what
> you ought to do...
>

There are also:

  /* If this is a paradoxical SUBREG...  */
  if (GET_MODE_SIZE (use_mode)
      > GET_MODE_SIZE (GET_MODE (SUBREG_REG (use_reg))))
    {
      /* If this is a paradoxical SUBREG, we have no idea what value the
         extra bits would have.  However, if the operand is equivalent to
         a SUBREG whose operand is the same as our mode, and all the modes
         are within a word, we can just use the inner operand because
         these SUBREGs just say how to treat the register.  */
      use_insn = DF_REF_INSN (use);
      src = SET_SRC (def_set);
      if (GET_CODE (src) == SUBREG
          && REG_P (SUBREG_REG (src))
          && GET_MODE (SUBREG_REG (src)) == use_mode
          && subreg_lowpart_p (src)
          && all_uses_available_at (def_insn, use_insn))
        return try_fwprop_subst (use, DF_REF_LOC (use), SUBREG_REG (src),
                                 def_insn, false);
    }

Should it check hard registers?
Paolo Bonzini - Jan. 25, 2011, 3:54 a.m.
On 01/25/2011 04:50 AM, H.J. Lu wrote:
>    /* If this is a paradoxical SUBREG...  */
>    if (GET_MODE_SIZE (use_mode)
>        >  GET_MODE_SIZE (GET_MODE (SUBREG_REG (use_reg))))
>      {
>        /* If this is a paradoxical SUBREG, we have no idea what value the
>           extra bits would have.  However, if the operand is equivalent to
>           a SUBREG whose operand is the same as our mode, and all the modes
>           are within a word, we can just use the inner operand because
>           these SUBREGs just say how to treat the register.  */
>        use_insn = DF_REF_INSN (use);
>        src = SET_SRC (def_set);
>        if (GET_CODE (src) == SUBREG
>            &&  REG_P (SUBREG_REG (src))
>            &&  GET_MODE (SUBREG_REG (src)) == use_mode
>            &&  subreg_lowpart_p (src)
>            &&  all_uses_available_at (def_insn, use_insn))
>          return try_fwprop_subst (use, DF_REF_LOC (use), SUBREG_REG (src),
>                                   def_insn, false);
>      }
>
> Should it check hard registers?

Yes.

Paolo

Patch

diff --git a/gcc/ChangeLog.x32 b/gcc/ChangeLog.x32
index ebe1d13..761799f 100644
--- a/gcc/ChangeLog.x32
+++ b/gcc/ChangeLog.x32
@@ -1,5 +1,11 @@ 
 2011-01-24  H.J. Lu  <hongjiu.lu@intel.com>
 
+	PR middle-end/47449
+	* fwprop.c (forward_propagate_subreg): Don't propagate zero/sign
+	extended hard register.
+
+2011-01-24  H.J. Lu  <hongjiu.lu@intel.com>
+
 	PR target/47446
 	* config/i386/i386.c (ix86_output_addr_vec_elt): Check
 	TARGET_LP64 instead of TARGET_64BIT for ASM_QUAD.
diff --git a/gcc/fwprop.c b/gcc/fwprop.c
index 7ff5135..866cbe3 100644
--- a/gcc/fwprop.c
+++ b/gcc/fwprop.c
@@ -1119,6 +1119,7 @@  forward_propagate_subreg (df_ref use, rtx def_insn, rtx def_set)
       if ((GET_CODE (src) == ZERO_EXTEND
 	   || GET_CODE (src) == SIGN_EXTEND)
 	  && REG_P (XEXP (src, 0))
+	  && REGNO (XEXP (src, 0)) >= FIRST_PSEUDO_REGISTER
 	  && GET_MODE (XEXP (src, 0)) == use_mode
 	  && !free_load_extend (src, def_insn)
 	  && all_uses_available_at (def_insn, use_insn))
diff --git a/gcc/testsuite/ChangeLog.x32 b/gcc/testsuite/ChangeLog.x32
index 506585d..271a2ae 100644
--- a/gcc/testsuite/ChangeLog.x32
+++ b/gcc/testsuite/ChangeLog.x32
@@ -1,5 +1,10 @@ 
 2011-01-24  H.J. Lu  <hongjiu.lu@intel.com>
 
+	PR middle-end/47449
+	* gcc.target/i386/pr47449.c: New.
+
+2011-01-24  H.J. Lu  <hongjiu.lu@intel.com>
+
 	PR target/47446
 	* gcc.target/i386/pr47446-1.c: New.
 
diff --git a/gcc/testsuite/gcc.target/i386/pr47449.c b/gcc/testsuite/gcc.target/i386/pr47449.c
new file mode 100644
index 0000000..99ef32f
--- /dev/null
+++ b/gcc/testsuite/gcc.target/i386/pr47449.c
@@ -0,0 +1,12 @@ 
+/* { dg-do compile } */
+/* { dg-options "-O2" } */
+
+void bar (void *, void *);
+int
+foo (void *p1, void *p2)
+{
+  int ret1, ret2;
+  __asm ("" : "=D" (ret1), "=S" (ret2));
+  bar (p1, p2);
+  return ret1 + ret2;
+}