Re: [PATCH IRA] update_equiv_regs fails to set EQUIV reg-note for pseudo with more than one definition

[Felix Yang <fei.yang0953@gmail.com>]

[-- Attachment #1: Type: text/plain, Size: 5085 bytes --]

Hi,

    Ignore the previous message.
    Attached please find the updated patch.
    Bootstrapped on x86_64-suse-linux. Please apply this patch if OK for trunk.

Index: gcc/ChangeLog
===================================================================
--- gcc/ChangeLog    (revision 215500)
+++ gcc/ChangeLog    (working copy)
@@ -1,3 +1,8 @@
+2014-09-23  Felix Yang  <felix.yang@huawei.com>
+
+    * ira.c (update_equiv_regs): Check all definitions for a multiple-set
+    register to make sure that the RHS have the same value.
+
 2014-09-23  Ilya Enkovich  <ilya.enkovich@intel.com>

     * cfgcleanup.c (try_optimize_cfg): Do not remove label
Index: gcc/ira.c
===================================================================
--- gcc/ira.c    (revision 215500)
+++ gcc/ira.c    (working copy)
@@ -3467,16 +3467,43 @@ update_equiv_regs (void)
       if (note && GET_CODE (XEXP (note, 0)) == EXPR_LIST)
         note = NULL_RTX;

-      if (DF_REG_DEF_COUNT (regno) != 1
-          && (! note
+      if (DF_REG_DEF_COUNT (regno) != 1)
+        {
+          rtx list;
+          bool equal_p = true;
+
+          if (! note
           || rtx_varies_p (XEXP (note, 0), 0)
           || (reg_equiv[regno].replacement
               && ! rtx_equal_p (XEXP (note, 0),
-                    reg_equiv[regno].replacement))))
-        {
-          no_equiv (dest, set, NULL);
-          continue;
+                    reg_equiv[regno].replacement)))
+        {
+          no_equiv (dest, set, NULL);
+          continue;
+        }
+
+          list = reg_equiv[regno].init_insns;
+          for (; list; list = XEXP (list, 1))
+        {
+          rtx note_tmp, insn_tmp;
+          insn_tmp = XEXP (list, 0);
+          note_tmp = find_reg_note (insn_tmp, REG_EQUAL, NULL_RTX);
+
+          if (note_tmp == 0
+              || ! rtx_equal_p (XEXP (note, 0), XEXP (note_tmp, 0)))
+            {
+              equal_p = false;
+              break;
+            }
+        }
+
+          if (! equal_p)
+        {
+          no_equiv (dest, set, NULL);
+          continue;
+        }
         }
+
       /* Record this insn as initializing this register.  */
       reg_equiv[regno].init_insns
         = gen_rtx_INSN_LIST (VOIDmode, insn, reg_equiv[regno].init_insns);
Cheers,
Felix


On Tue, Sep 23, 2014 at 6:45 PM, Felix Yang <fei.yang0953@gmail.com> wrote:
> Hi,
>     Attached please fined the updated patch.
>
> Cheers,
> Felix
>
>
> On Tue, Sep 23, 2014 at 10:48 AM, Yangfei (Felix) <felix.yang@huawei.com> wrote:
>>> On 09/22/14 08:40, Felix Yang wrote:
>>> > Hi,
>>> >
>>> >      I find that update_equiv_regs in ira.c sets the wrong EQUIV
>>> > reg-note for pseudo with more than one definiton in certain situation.
>>> >      Here is a simplified RTL snippet after this function finishs handling:
>>> >
>>> >   (insn 77 37 78 2 (set (reg:SI 171)
>>> >           (const_int 0 [0])) ticket151.c:33 52 {movsi_internal_dsp}
>>> >        (expr_list:REG_EQUAL (const_int 0 [0])
>>> >           (nil)))
>>> >
>>> > ......
>>> >
>>> > (insn 79 50 53 2 (set (mem/c:SI (reg/f:SI 136) [2 g_728+0 S4 A64])
>>> >           (reg:SI 171)) ticket151.c:33 52 {movsi_internal_dsp}
>>> >        (expr_list:REG_DEAD (reg:SI 171)
>>> >           (nil)))
>>> > (insn 53 79 54 2 (set (mem/c:SI (reg/f:SI 162) [4 g_163+0 S4 A32])
>>> >           (reg:SI 163)) 52 {movsi_internal_dsp}
>>> >        (expr_list:REG_DEAD (reg:SI 163)
>>> >           (expr_list:REG_DEAD (reg/f:SI 162)
>>> >               (nil))))
>>> > (insn 54 53 14 2 (set (reg:SI 171)
>>> >           (mem/u/c:SI (symbol_ref/u:SI ("*.LC8") [flags 0x2]) [4  S4
>>> > A32])) ticket151.c:49 52 {movsi_internal_dsp}
>>> >        (expr_list:REG_EQUIV (mem/u/c:SI (symbol_ref/u:SI ("*.LC8")
>>> > [flags 0x2]) [4  S4 A32])
>>> >           (expr_list:REG_EQUAL (mem/u/c:SI (symbol_ref/u:SI ("*.LC8")
>>> > [flags 0x2]) [4  S4 A32])
>>> >               (nil))))
>>> >
>>> >
>>> >      The REG_EQUIV of insn 54 is not correct as pseudo 171 is defined
>>> > in insn 77 with a differerent value.
>>> >      This may causes reload replacing pseudo 171 with mem/u/c:SI
>>> > (symbol_ref/u:SI ("*.LC8"), which is wrong.
>>> >      A proposed patch for this issue, please comment:
>>> Is it possible it's the conditional just above this one that is incorrect?
>>>
>>>           if (DF_REG_DEF_COUNT (regno) != 1
>>>                && (! note
>>>                    || rtx_varies_p (XEXP (note, 0), 0)
>>>                    || (reg_equiv[regno].replacement
>>>                        && ! rtx_equal_p (XEXP (note, 0),
>>>
>>> reg_equiv[regno].replacement))))
>>>              {
>>>                no_equiv (dest, set, NULL);
>>>                continue;
>>>              }
>>>
>>>
>>> ISTM the only time a multiply-set register can have a valid REG_EQUIV note is if
>>> each and every assignment to that pseudo has the same RHS.
>>>
>>> Jeff
>>
>>
>> Thanks Jeff. Yes, I agree that this is a better place to consider. I am thinking of a better way to fix this.
>>
>> Vladimir, can you shed light on this and give me some suggestions? Thank you.
>>

[-- Attachment #2: ira-update-equiv-regs.diff --]
[-- Type: text/plain, Size: 1867 bytes --]

Index: gcc/ChangeLog
===================================================================
--- gcc/ChangeLog	(revision 215500)
+++ gcc/ChangeLog	(working copy)
@@ -1,3 +1,8 @@
+2014-09-23  Felix Yang  <felix.yang@huawei.com>
+
+	* ira.c (update_equiv_regs): Check all definitions for a multiple-set
+	register to make sure that the RHS have the same value.
+
 2014-09-23  Ilya Enkovich  <ilya.enkovich@intel.com>
 
 	* cfgcleanup.c (try_optimize_cfg): Do not remove label
Index: gcc/ira.c
===================================================================
--- gcc/ira.c	(revision 215500)
+++ gcc/ira.c	(working copy)
@@ -3467,16 +3467,43 @@ update_equiv_regs (void)
 	  if (note && GET_CODE (XEXP (note, 0)) == EXPR_LIST)
 	    note = NULL_RTX;
 
-	  if (DF_REG_DEF_COUNT (regno) != 1
-	      && (! note
+	  if (DF_REG_DEF_COUNT (regno) != 1)
+	    {
+	      rtx list;
+	      bool equal_p = true;
+
+	      if (! note
 		  || rtx_varies_p (XEXP (note, 0), 0)
 		  || (reg_equiv[regno].replacement
 		      && ! rtx_equal_p (XEXP (note, 0),
-					reg_equiv[regno].replacement))))
-	    {
-	      no_equiv (dest, set, NULL);
-	      continue;
+					reg_equiv[regno].replacement)))
+		{
+		  no_equiv (dest, set, NULL);
+		  continue;
+		}
+
+	      list = reg_equiv[regno].init_insns;
+	      for (; list; list = XEXP (list, 1))
+		{
+		  rtx note_tmp, insn_tmp;
+		  insn_tmp = XEXP (list, 0);
+		  note_tmp = find_reg_note (insn_tmp, REG_EQUAL, NULL_RTX);
+
+		  if (note_tmp == 0
+		      || ! rtx_equal_p (XEXP (note, 0), XEXP (note_tmp, 0)))
+		    {
+		      equal_p = false;
+		      break;
+		    }
+		}
+
+	      if (! equal_p)
+		{
+		  no_equiv (dest, set, NULL);
+		  continue;
+		}
 	    }
+
 	  /* Record this insn as initializing this register.  */
 	  reg_equiv[regno].init_insns
 	    = gen_rtx_INSN_LIST (VOIDmode, insn, reg_equiv[regno].init_insns);