Re: [Patch, rtl] PR middle-end/78016, keep REG_NOTE order during insn copy

[Jiong Wang]

On 21/10/16 11:13, Bernd Schmidt wrote:
On 10/21/2016 09:43 AM, Eric Botcazou wrote:
I disagree: there are currently n ways of copying NOTEs in the RTL 
middle-end,
with different properties each time.  We need only one primitive in 
rtlanal.c.

I feel the fact that they have different properties means we shouldn't 
try to unify them: we'll just end up with a long list of boolean 
parameters, with no way of quickly telling what a given function call 
is doing. A copy loop is short enough that it can be implemented 
in-place and people can quickly tell what is going on by looking at it.

Maybe the inner if statement could be a small helper function 
(append_copy_of_reg_note).


Bernd

Hi Bernd, Eric,

  How does the attached patch looks to you?  x86_64 bootstrap & regression OK.

  I borrowed Bernd' code to write the tail pointer directly.


2016-10-21  Bernd Schmidt  <bschm...@redhat.com>
            Jiong Wang  <jiong.w...@arm.com>
  
gcc/

        PR middle-end/78016
        * emit-rtl.c (emit_copy_of_insn_after): Copy REG_NOTES in order instead
        of in reverse order.
        * sel-sched-ir.c (create_copy_of_insn_rtx): Likewise.


diff --git a/gcc/emit-rtl.c b/gcc/emit-rtl.c
index 2d6d1eb6c1311871f15dbed13d7c084ed3845a86..4d849ca6e64273bedc5bf8b9a62a5cc5d4606129 100644
--- a/gcc/emit-rtl.c
+++ b/gcc/emit-rtl.c
@@ -6168,17 +6168,31 @@ emit_copy_of_insn_after (rtx_insn *insn, rtx_insn *after)
      which may be duplicated by the basic block reordering code.  */
   RTX_FRAME_RELATED_P (new_rtx) = RTX_FRAME_RELATED_P (insn);
 
+  /* Locate the end of existing REG_NOTES in NEW_RTX.  */
+  rtx *ptail = &REG_NOTES (new_rtx);
+  while (*ptail != NULL_RTX)
+    ptail = &XEXP (*ptail, 1);
+
   /* Copy all REG_NOTES except REG_LABEL_OPERAND since mark_jump_label
      will make them.  REG_LABEL_TARGETs are created there too, but are
      supposed to be sticky, so we copy them.  */
   for (link = REG_NOTES (insn); link; link = XEXP (link, 1))
     if (REG_NOTE_KIND (link) != REG_LABEL_OPERAND)
       {
-	if (GET_CODE (link) == EXPR_LIST)
-	  add_reg_note (new_rtx, REG_NOTE_KIND (link),
-			copy_insn_1 (XEXP (link, 0)));
+	rtx new_node;
+
+	if (GET_CODE (link) == INT_LIST)
+	  new_node = gen_rtx_INT_LIST ((machine_mode) REG_NOTE_KIND (link),
+				       XINT (link, 0), NULL_RTX);
 	else
-	  add_shallow_copy_of_reg_note (new_rtx, link);
+	  new_node = alloc_reg_note (REG_NOTE_KIND (link),
+				     (GET_CODE (link) == EXPR_LIST
+				      ? copy_insn_1 (XEXP (link, 0))
+				      : XEXP (link ,0)),
+				     NULL_RTX);
+
+	*ptail = new_node;
+	ptail = &XEXP (new_node, 1);
       }
 
   INSN_CODE (new_rtx) = INSN_CODE (insn);
diff --git a/gcc/sel-sched-ir.c b/gcc/sel-sched-ir.c
index 210b1e4edfb359a161cda4826704005ae9ab5a24..324ae8cf05209757a3a3f3dee97c9274876c7ed7 100644
--- a/gcc/sel-sched-ir.c
+++ b/gcc/sel-sched-ir.c
@@ -5761,6 +5761,11 @@ create_copy_of_insn_rtx (rtx insn_rtx)
   res = create_insn_rtx_from_pattern (copy_rtx (PATTERN (insn_rtx)),
                                       NULL_RTX);
 
+  /* Locate the end of existing REG_NOTES in RES.  */
+  rtx *ptail = &REG_NOTES (res);
+  while (*ptail != NULL_RTX)
+    ptail = &XEXP (*ptail, 1);
+
   /* Copy all REG_NOTES except REG_EQUAL/REG_EQUIV and REG_LABEL_OPERAND
      since mark_jump_label will make them.  REG_LABEL_TARGETs are created
      there too, but are supposed to be sticky, so we copy them.  */
@@ -5769,11 +5774,12 @@ create_copy_of_insn_rtx (rtx insn_rtx)
 	&& REG_NOTE_KIND (link) != REG_EQUAL
 	&& REG_NOTE_KIND (link) != REG_EQUIV)
       {
-	if (GET_CODE (link) == EXPR_LIST)
-	  add_reg_note (res, REG_NOTE_KIND (link),
-			copy_insn_1 (XEXP (link, 0)));
-	else
-	  add_reg_note (res, REG_NOTE_KIND (link), XEXP (link, 0));
+	rtx new_node = alloc_reg_note (REG_NOTE_KIND (link),
+				       (GET_CODE (link) == EXPR_LIST
+					? copy_insn_1 (XEXP (link, 0))
+					: XEXP (link ,0)), NULL_RTX);
+	*ptail = new_node;
+	ptail = &XEXP (new_node, 1);
       }
 
   return res;