strengthen protection against REG_EQUIV/EQUAL on !REG set

strengthen protection against REG_EQUIV/EQUAL on !REG set

[Olivier Hainque]

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

Hello,

This is a followup on a suggestion made along the thread at

  http://gcc.gnu.org/ml/gcc-patches/2010-09/msg00967.html

where we were observing the middle-end setting invalid REG_EQUIV
notes on set(mem) insns. At the time we had fixed specific locations
where this was happening via explicit calls to set_unique_reg_note.

Steven suggested to add some code to set_unique_reg_note as well. This
patch implements this suggestion. We (AdaCore) have been using it without
problem for a while now, it turned out useful to prevent a real case in
the gcc-4.5 series and we never saw the original issue resurface again
since then.

Bootstrapped and regtested on x86_64-suse-linux

OK to commit ?

Thanks in advance,

With Kind Regards,

Olivier

2012-04-12  Olivier Hainque  <hainque@adacore.com>

 	* emit-rtl.c (set_unique_reg_note): Don't add REG_EQUAL or REG_EQUIV
	on a SET if the destination isn't a REG or a SUBREG of REG.


[-- Attachment #2: regequiv.dif --]
[-- Type: video/x-dv, Size: 982 bytes --]

Re: strengthen protection against REG_EQUIV/EQUAL on !REG set

[Olivier Hainque]

Clarifying:

On Apr 12, 2012, at 11:54 , Olivier Hainque wrote:
> At the time we had fixed specific locations
> where this was happening via explicit calls to set_unique_reg_note.

 We had fixed the problems observable at the time by preventing
 calls to set_unique_reg_notes when they would lead to invalid settings.

 We did that at specific locations where we had observed real
 problematic calls taking place.

> Steven suggested to add some code to set_unique_reg_note as well.
> This patch implements this suggestion.


 Olivier

Re: strengthen protection against REG_EQUIV/EQUAL on !REG set

[Richard Sandiford]

Olivier Hainque <hainque@adacore.com> writes:
> *** /tmp/rkQ7Ep_emit-rtl.c	2012-04-12 11:19:51.830104512 +0200
> --- gcc/emit-rtl.c	2012-04-11 22:39:11.323103686 +0200
> *************** set_unique_reg_note (rtx insn, enum reg_
> *** 4955,4960 ****
> --- 4955,4975 ----
>         if (GET_CODE (datum) == ASM_OPERANDS)
>   	return NULL_RTX;
>   
> +       /* Don't add REG_EQUAL/REG_EQUIV notes on a single_set destination which
> + 	 isn't a REG or a SUBREG of REG.  Such notes are invalid, could lead
> + 	 to bogus assumptions downstream (e.g. that a (set MEM) couldn't trap),
> + 	 and many callers just don't care checking.  Note that we might have
> + 	 single_set (insn) == 0 here, typically from reload attaching REG_EQUAL
> + 	 to USEs for inheritance processing purposes.  */
> +       {
> + 	rtx set  = single_set (insn);
> + 
> + 	if (set && ! (REG_P (SET_DEST (set))
> + 		      || (GET_CODE (SET_DEST (set)) == SUBREG
> + 			  && REG_P (SUBREG_REG (SET_DEST (set))))))
> + 	  return NULL_RTX;
> +       }
> + 

STRICT_LOW_PART is OK too.

I like the idea, but I think we're in danger of having too many
functions check the set.  Further up set_unique_reg_note we have:

      /* Don't add REG_EQUAL/REG_EQUIV notes if the insn
	 has multiple sets (some callers assume single_set
	 means the insn only has one set, when in fact it
	 means the insn only has one * useful * set).  */
      if (GET_CODE (PATTERN (insn)) == PARALLEL && multiple_sets (insn))
	{
	  gcc_assert (!note);
	  return NULL_RTX;
	}

And set_dst_reg_note has:

  rtx set = single_set (insn);

  if (set && SET_DEST (set) == dst)
    return set_unique_reg_note (insn, kind, datum);

Would be nice to use a single function that knows about the extra
contraints here.  Maybe something like the attached?

I'm deliberately requiring the SET to the first rtx in the PARALLEL.
I'm not entirely happy with the optabs.c code:

  if (! rtx_equal_p (SET_DEST (set), target)
      /* For a STRICT_LOW_PART, the REG_NOTE applies to what is inside it.  */
      && (GET_CODE (SET_DEST (set)) != STRICT_LOW_PART
	  || ! rtx_equal_p (XEXP (SET_DEST (set), 0), target)))
    return 1;

either; the note is always GET_MODE (target), so surely this
should be checking for STRICT_LOW_PART before applying rtx_equal_p?
Maybe set_for_reg_notes should return the reg too, and we'd just
apply rtx_equal_p to that.

Richard


gcc/
	* rtl.h (set_for_reg_notes): Declare.
	* emit-rtl.c (set_for_reg_notes): New function.
	(set_unique_reg_note): Use it.
	* optabs.c (add_equal_note): Likewise.

Index: gcc/rtl.h
===================================================================
--- gcc/rtl.h	2012-04-24 22:55:36.002967164 +0100
+++ gcc/rtl.h	2012-04-24 22:59:34.150966581 +0100
@@ -1879,6 +1879,7 @@ extern enum machine_mode choose_hard_reg
 					       bool);
 
 /* In emit-rtl.c  */
+extern rtx set_for_reg_notes (rtx);
 extern rtx set_unique_reg_note (rtx, enum reg_note, rtx);
 extern rtx set_dst_reg_note (rtx, enum reg_note, rtx, rtx);
 extern void set_insn_deleted (rtx);
Index: gcc/emit-rtl.c
===================================================================
--- gcc/emit-rtl.c	2012-04-24 22:55:36.002967164 +0100
+++ gcc/emit-rtl.c	2012-04-24 23:00:13.677966484 +0100
@@ -4944,6 +4944,45 @@ force_next_line_note (void)
   last_location = -1;
 }
 
+/* Notes like REG_EQUAL and REG_EQUIV refer to a set in an instruction.
+   Return the set in INSN that such notes describe, or NULL if the notes
+   have no meaning for INSN.  */
+
+rtx
+set_for_reg_notes (rtx insn)
+{
+  rtx pat, reg;
+
+  if (!INSN_P (insn))
+    return NULL_RTX;
+
+  pat = PATTERN (insn);
+  if (GET_CODE (pat) == PARALLEL)
+    {
+      /* We do not use single_set because that ignores SETs of unused
+	 registers.  REG_EQUAL and REG_EQUIV notes really do require the
+	 PARALLEL to have a single SET.  */
+      if (multiple_sets (insn))
+	return NULL_RTX;
+      pat = XVECEXP (pat, 0, 0);
+    }
+
+  if (GET_CODE (pat) != SET)
+    return NULL_RTX;
+
+  reg = SET_DEST (pat);
+
+  /* Notes apply to the contents of a STRICT_LOW_PART.  */
+  if (GET_CODE (reg) == STRICT_LOW_PART)
+    reg = XEXP (reg, 0);
+
+  /* Check that we have a register.  */
+  if (!(REG_P (reg) || GET_CODE (reg) == SUBREG))
+    return NULL_RTX;
+
+  return pat;
+}
+
 /* Place a note of KIND on insn INSN with DATUM as the datum. If a
    note of this type already exists, remove it first.  */
 
@@ -4956,39 +4995,26 @@ set_unique_reg_note (rtx insn, enum reg_
     {
     case REG_EQUAL:
     case REG_EQUIV:
-      /* Don't add REG_EQUAL/REG_EQUIV notes if the insn
-	 has multiple sets (some callers assume single_set
-	 means the insn only has one set, when in fact it
-	 means the insn only has one * useful * set).  */
-      if (GET_CODE (PATTERN (insn)) == PARALLEL && multiple_sets (insn))
-	{
-	  gcc_assert (!note);
-	  return NULL_RTX;
-	}
+      if (!set_for_reg_notes (insn))
+	return NULL_RTX;
 
       /* Don't add ASM_OPERAND REG_EQUAL/REG_EQUIV notes.
 	 It serves no useful purpose and breaks eliminate_regs.  */
       if (GET_CODE (datum) == ASM_OPERANDS)
 	return NULL_RTX;
-
-      if (note)
-	{
-	  XEXP (note, 0) = datum;
-	  df_notes_rescan (insn);
-	  return note;
-	}
       break;
 
     default:
-      if (note)
-	{
-	  XEXP (note, 0) = datum;
-	  return note;
-	}
       break;
     }
 
-  add_reg_note (insn, kind, datum);
+  if (note)
+    XEXP (note, 0) = datum;
+  else
+    {
+      add_reg_note (insn, kind, datum);
+      note = REG_NOTES (insn);
+    }
 
   switch (kind)
     {
@@ -5000,14 +5026,14 @@ set_unique_reg_note (rtx insn, enum reg_
       break;
     }
 
-  return REG_NOTES (insn);
+  return note;
 }
 
 /* Like set_unique_reg_note, but don't do anything unless INSN sets DST.  */
 rtx
 set_dst_reg_note (rtx insn, enum reg_note kind, rtx datum, rtx dst)
 {
-  rtx set = single_set (insn);
+  rtx set = set_for_reg_notes (insn);
 
   if (set && SET_DEST (set) == dst)
     return set_unique_reg_note (insn, kind, datum);
Index: gcc/optabs.c
===================================================================
--- gcc/optabs.c	2012-04-24 22:55:36.002967164 +0100
+++ gcc/optabs.c	2012-04-24 22:59:34.148966581 +0100
@@ -191,7 +191,7 @@ add_equal_note (rtx insns, rtx target, e
        last_insn = NEXT_INSN (last_insn))
     ;
 
-  set = single_set (last_insn);
+  set = set_for_reg_notes (last_insn);
   if (set == NULL_RTX)
     return 1;
 

Re: strengthen protection against REG_EQUIV/EQUAL on !REG set

[Olivier Hainque]

Hello Richard,

On Apr 25, 2012, at 00:06 , Richard Sandiford wrote:
> STRICT_LOW_PART is OK too.

 Ah, right.

> Would be nice to use a single function that knows about the extra
> contraints here.  Maybe something like the attached?
> 
> I'm deliberately requiring the SET to the first rtx in the PARALLEL.

 Looks cleaner indeed. Do you want me to test ?

 Thanks a lot for your feedback.

 Olivier

Re: strengthen protection against REG_EQUIV/EQUAL on !REG set

[Olivier Hainque]

Hello Richard,

Re $subject, at http://gcc.gnu.org/ml/gcc-patches/2012-04/msg01515.html

You suggested:
>> Would be nice to use a single function that knows about the extra
>> contraints here.  Maybe something like the attached?

<< 2012-04-24  ...

	* rtl.h (set_for_reg_notes): Declare.
	* emit-rtl.c (set_for_reg_notes): New function.
	(set_unique_reg_note): Use it.
	* optabs.c (add_equal_note): Likewise.
>>

I had answered:
> Looks cleaner indeed. Do you want me to test ?

I gave it a try. Your patch bootstraps and regtests fine on mainline for x86-linux.

May I commit ?

Thanks in advance for your feedback,

With Kind Regards,

Olivier

Re: strengthen protection against REG_EQUIV/EQUAL on !REG set

[Richard Sandiford]

Olivier Hainque <hainque@adacore.com> writes:
> Hello Richard,
>
> Re $subject, at http://gcc.gnu.org/ml/gcc-patches/2012-04/msg01515.html
>
> You suggested:
>>> Would be nice to use a single function that knows about the extra
>>> contraints here.  Maybe something like the attached?
>
> << 2012-04-24  ...
>
> 	* rtl.h (set_for_reg_notes): Declare.
> 	* emit-rtl.c (set_for_reg_notes): New function.
> 	(set_unique_reg_note): Use it.
> 	* optabs.c (add_equal_note): Likewise.
>>>
>
> I had answered:
>> Looks cleaner indeed. Do you want me to test ?
>
> I gave it a try. Your patch bootstraps and regtests fine on mainline for x86-linux.

Sorry, was going to test this earlier, but got distracted by
lower-subreg stuff.  I need to fix the subreg handling so that
we check whether the inner part of a SUBREG is a REG (done in
my copy at home).  I also wanted to make sure there were no
asm differences due to notes being wrongly dropped.

Hope to do that this weekend.

Richard

Re: strengthen protection against REG_EQUIV/EQUAL on !REG set

[Olivier Hainque]

On May 4, 2012, at 16:16 , Richard Sandiford wrote:

> Sorry, was going to test this earlier, but got distracted by
> lower-subreg stuff.

 No problem at all. I just happened to have had an opportunity to
 test as part of a series of miscellaneous other submissions.

>  I need to fix the subreg handling so that
> we check whether the inner part of a SUBREG is a REG (done in
> my copy at home).

 Indeed. That was in the original patch and I missed the
 difference in the alternate version.

>  I also wanted to make sure there were no
> asm differences due to notes being wrongly dropped.

 Ah, nice :)

 Thanks much for your feedback,

 Olivier

Re: strengthen protection against REG_EQUIV/EQUAL on !REG set

[Richard Sandiford]

Richard Sandiford <rdsandiford@googlemail.com> writes:
> Olivier Hainque <hainque@adacore.com> writes:
>> Hello Richard,
>>
>> Re $subject, at http://gcc.gnu.org/ml/gcc-patches/2012-04/msg01515.html
>>
>> You suggested:
>>>> Would be nice to use a single function that knows about the extra
>>>> contraints here.  Maybe something like the attached?
>>
>> << 2012-04-24  ...
>>
>> 	* rtl.h (set_for_reg_notes): Declare.
>> 	* emit-rtl.c (set_for_reg_notes): New function.
>> 	(set_unique_reg_note): Use it.
>> 	* optabs.c (add_equal_note): Likewise.
>>>>
>>
>> I had answered:
>>> Looks cleaner indeed. Do you want me to test ?
>>
>> I gave it a try. Your patch bootstraps and regtests fine on mainline for x86-linux.
>
> Sorry, was going to test this earlier, but got distracted by
> lower-subreg stuff.  I need to fix the subreg handling so that
> we check whether the inner part of a SUBREG is a REG (done in
> my copy at home).  I also wanted to make sure there were no
> asm differences due to notes being wrongly dropped.

So I tried compiling some recent cc1 .ii files on x86_64 at -O2.
The only differences were in fixed-value.ii.  In this case we
used to create:

---------------------------------------------------------------------------
(insn 899 898 900 68 (parallel [
            (set (reg/f:DI 597)
                (plus:DI (reg/f:DI 20 frame)
                    (const_int -32 [0xffffffffffffffe0])))
            (clobber (reg:CC 17 flags))
        ]) /home/richards/gcc/HEAD/gcc/gcc/fixed-value.c:568 252 {*adddi_1}
     (nil))

(insn 900 899 901 72 (parallel [
            (set (reg:DI 598)
                (plus:DI (reg:DI 597)
                    (const_int 8 [0x8])))
            (clobber (reg:CC 17 flags))
        ]) /home/richards/gcc/HEAD/gcc/gcc/fixed-value.c:568 -1
     (nil))

(insn 901 900 902 72 (set (mem/f:DI (plus:DI (reg/f:DI 56 virtual-outgoing-args)
                (const_int 24 [0x18])) [0 S8 A64])
        (reg:DI 598)) /home/richards/gcc/HEAD/gcc/gcc/fixed-value.c:568 -1
     (expr_list:REG_EQUAL (plus:DI (reg:DI 597)
            (const_int 8 [0x8]))
        (nil)))

[...other uses of 597...]

(insn 921 920 922 73 (parallel [
            (set (reg:DI 604)
                (plus:DI (reg:DI 603)
                    (const_int 8 [0x8])))
            (clobber (reg:CC 17 flags))
        ]) /home/richards/gcc/HEAD/gcc/gcc/fixed-value.c:580 -1
     (nil))
---------------------------------------------------------------------------

where 901 has a REG_EQUAL note against a MEM.  cse1 changes the note to:

---------------------------------------------------------------------------
(insn 901 900 902 68 (set (mem/f:DI (plus:DI (reg/f:DI 7 sp)
                (const_int 24 [0x18])) [0 S8 A64])
        (reg/f:DI 598)) /home/richards/gcc/HEAD/gcc/gcc/fixed-value.c:568 62 {*movdi_internal_rex64}
     (expr_list:REG_EQUAL (plus:DI (reg/f:DI 20 frame)
            (const_int -24 [0xffffffffffffffe8]))
        (nil)))
---------------------------------------------------------------------------

but doesn't touch 921 (probably worth finding out why).  cse2 then sees
this note and records it as having the same value as both the MEM and
reg 598.  So when it comes to insn 921 and replaces the source with reg 598,
it also adds a note:

---------------------------------------------------------------------------
(insn 921 1285 939 71 (set (reg/f:DI 698)
        (reg/f:DI 598)) /home/richards/gcc/HEAD/gcc/gcc/fixed-value.c:580 62 {*movdi_internal_rex64}
     (expr_list:REG_EQUAL (plus:DI (reg/f:DI 20 frame)
            (const_int -24 [0xffffffffffffffe8]))
        (expr_list:REG_UNUSED (reg:CC 17 flags)
            (nil))))
---------------------------------------------------------------------------

Reload is then able to use this information to drop the 698 and
rematerialise it where necessary.  After the patch we just get:

---------------------------------------------------------------------------
(insn 921 1285 939 71 (set (reg/f:DI 698)
        (reg/f:DI 598)) /home/richards/gcc/HEAD/gcc/gcc/fixed-value.c:580 62 {*movdi_internal_rex64}
     (expr_list:REG_UNUSED (reg:CC 17 flags)
        (nil)))
---------------------------------------------------------------------------

The problem here seems to be some inconsistency about what is considered
"constant" in cse.  The condition for table elements is:

  elt->is_const = (CONSTANT_P (x) || fixed_base_plus_p (x));

But the condition for values that been calculated through folding
(e.g. because no note exists) is:

      if (src_const == 0
	  && (CONSTANT_P (src_folded)
	      /* Consider (minus (label_ref L1) (label_ref L2)) as
		 "constant" here so we will record it. This allows us
		 to fold switch statements when an ADDR_DIFF_VEC is used.  */
	      || (GET_CODE (src_folded) == MINUS
		  && GET_CODE (XEXP (src_folded, 0)) == LABEL_REF
		  && GET_CODE (XEXP (src_folded, 1)) == LABEL_REF)))
	src_const = src_folded, src_const_elt = elt;

fixed_base_plus_p has some old code related to rtl inlining,
so these days I think the first condition should be equivalent to:

  elt->is_const = function_invariant_p (x);

I'm not sure why fixed_plus_base_p checks fixed_regs[ARG_POINTER_REGNUM]
though.  (function_invariant_p doesn't.)

If we change the second to also include fixed_base_plus_p/
function_invariant_p:

      if (src_const == 0
	  && (function_invariant_p (src_folded)
	      /* Consider (minus (label_ref L1) (label_ref L2)) as
		 "constant" here so we will record it. This allows us
		 to fold switch statements when an ADDR_DIFF_VEC is used.  */
	      || (GET_CODE (src_folded) == MINUS
		  && GET_CODE (XEXP (src_folded, 0)) == LABEL_REF
		  && GET_CODE (XEXP (src_folded, 1)) == LABEL_REF)))
	src_const = src_folded, src_const_elt = elt;

then we get the note back.  That in itself isn't a complete patch,
but it makes a surprising difference.  I've filed 53260 to record this.

Even though the REG_EQUAL note above is invalid (and documented as
being invalid), I'm guessing people wouldn't want the patch to be
applied until this is sorted out.

Richard

Re: strengthen protection against REG_EQUIV/EQUAL on !REG set

[Olivier Hainque]

>> 	* rtl.h (set_for_reg_notes): Declare.
>> 	* emit-rtl.c (set_for_reg_notes): New function.
>> 	(set_unique_reg_note): Use it.
>> 	* optabs.c (add_equal_note): Likewise.
> This is fine.

 checked-in as revision 210998.

 Thanks Jeff :-)

 Olivier

Re: strengthen protection against REG_EQUIV/EQUAL on !REG set

[Jeff Law]

On 05/26/14 06:28, Olivier Hainque wrote:
> Hello,
>
> This is a follow up on a thread started long ago there:
>
>    http://gcc.gnu.org/ml/gcc-patches/2010-09/msg00967.html
>
> With a first followup and a patch proposal there:
>
>    http://gcc.gnu.org/ml/gcc-patches/2012-04/msg00731.html
>
> Then a refinement suggested by Richard Sandiford here:
>
>    http://gcc.gnu.org/ml/gcc-patches/2012-04/msg01515.html
>
> We have been running with the proposed patch for years now,
> it still applies on mainline and bootstraps + regtests fine
> on x86_64-linux for languages=all,ada.
>
> Ok to commit ?
>
> Thanks in advance,
>
> With Kind Regards,
>
> Olivier
>
> 2014-05-26  Richard Sandiford  <rdsandiford@googlemail.com>
>
> 	* rtl.h (set_for_reg_notes): Declare.
> 	* emit-rtl.c (set_for_reg_notes): New function.
> 	(set_unique_reg_note): Use it.
> 	* optabs.c (add_equal_note): Likewise.
This is fine.


Thanks,
Jeff

strengthen protection against REG_EQUIV/EQUAL on !REG set

[Olivier Hainque]

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

Hello,

This is a follow up on a thread started long ago there:

  http://gcc.gnu.org/ml/gcc-patches/2010-09/msg00967.html

With a first followup and a patch proposal there:

  http://gcc.gnu.org/ml/gcc-patches/2012-04/msg00731.html

Then a refinement suggested by Richard Sandiford here:

  http://gcc.gnu.org/ml/gcc-patches/2012-04/msg01515.html

We have been running with the proposed patch for years now,
it still applies on mainline and bootstraps + regtests fine
on x86_64-linux for languages=all,ada.

Ok to commit ?

Thanks in advance,

With Kind Regards,

Olivier

2014-05-26  Richard Sandiford  <rdsandiford@googlemail.com>

	* rtl.h (set_for_reg_notes): Declare.
	* emit-rtl.c (set_for_reg_notes): New function.
	(set_unique_reg_note): Use it.
	* optabs.c (add_equal_note): Likewise.


[-- Attachment #2: ehregnote.diff --]
[-- Type: application/octet-stream, Size: 3651 bytes --]

diff --git gcc/emit-rtl.c gcc/emit-rtl.c
index 4736f8d..b066111 100644
--- gcc/emit-rtl.c
+++ gcc/emit-rtl.c
@@ -4956,6 +4956,45 @@ gen_use (rtx x)
   return seq;
 }
 
+/* Notes like REG_EQUAL and REG_EQUIV refer to a set in an instruction.
+   Return the set in INSN that such notes describe, or NULL if the notes
+   have no meaning for INSN.  */
+
+rtx
+set_for_reg_notes (rtx insn)
+{
+  rtx pat, reg;
+
+  if (!INSN_P (insn))
+    return NULL_RTX;
+
+  pat = PATTERN (insn);
+  if (GET_CODE (pat) == PARALLEL)
+    {
+      /* We do not use single_set because that ignores SETs of unused
+	 registers.  REG_EQUAL and REG_EQUIV notes really do require the
+	 PARALLEL to have a single SET.  */
+      if (multiple_sets (insn))
+	return NULL_RTX;
+      pat = XVECEXP (pat, 0, 0);
+    }
+
+  if (GET_CODE (pat) != SET)
+    return NULL_RTX;
+
+  reg = SET_DEST (pat);
+
+  /* Notes apply to the contents of a STRICT_LOW_PART.  */
+  if (GET_CODE (reg) == STRICT_LOW_PART)
+    reg = XEXP (reg, 0);
+
+  /* Check that we have a register.  */
+  if (!(REG_P (reg) || GET_CODE (reg) == SUBREG))
+    return NULL_RTX;
+
+  return pat;
+}
+
 /* Place a note of KIND on insn INSN with DATUM as the datum. If a
    note of this type already exists, remove it first.  */
 
@@ -4968,39 +5007,26 @@ set_unique_reg_note (rtx insn, enum reg_note kind, rtx datum)
     {
     case REG_EQUAL:
     case REG_EQUIV:
-      /* Don't add REG_EQUAL/REG_EQUIV notes if the insn
-	 has multiple sets (some callers assume single_set
-	 means the insn only has one set, when in fact it
-	 means the insn only has one * useful * set).  */
-      if (GET_CODE (PATTERN (insn)) == PARALLEL && multiple_sets (insn))
-	{
-	  gcc_assert (!note);
-	  return NULL_RTX;
-	}
+      if (!set_for_reg_notes (insn))
+	return NULL_RTX;
 
       /* Don't add ASM_OPERAND REG_EQUAL/REG_EQUIV notes.
 	 It serves no useful purpose and breaks eliminate_regs.  */
       if (GET_CODE (datum) == ASM_OPERANDS)
 	return NULL_RTX;
-
-      if (note)
-	{
-	  XEXP (note, 0) = datum;
-	  df_notes_rescan (insn);
-	  return note;
-	}
       break;
 
     default:
-      if (note)
-	{
-	  XEXP (note, 0) = datum;
-	  return note;
-	}
       break;
     }
 
-  add_reg_note (insn, kind, datum);
+  if (note)
+    XEXP (note, 0) = datum;
+  else
+    {
+      add_reg_note (insn, kind, datum);
+      note = REG_NOTES (insn);
+    }
 
   switch (kind)
     {
@@ -5012,14 +5038,14 @@ set_unique_reg_note (rtx insn, enum reg_note kind, rtx datum)
       break;
     }
 
-  return REG_NOTES (insn);
+  return note;
 }
 
 /* Like set_unique_reg_note, but don't do anything unless INSN sets DST.  */
 rtx
 set_dst_reg_note (rtx insn, enum reg_note kind, rtx datum, rtx dst)
 {
-  rtx set = single_set (insn);
+  rtx set = set_for_reg_notes (insn);
 
   if (set && SET_DEST (set) == dst)
     return set_unique_reg_note (insn, kind, datum);
diff --git gcc/optabs.c gcc/optabs.c
index e16f055..8480627 100644
--- gcc/optabs.c
+++ gcc/optabs.c
@@ -228,7 +228,7 @@ add_equal_note (rtx insns, rtx target, enum rtx_code code, rtx op0, rtx op1)
       return 0;
     }
 
-  set = single_set (last_insn);
+  set = set_for_reg_notes (last_insn);
   if (set == NULL_RTX)
     return 1;
 
diff --git gcc/rtl.h gcc/rtl.h
index f1cda4c..445cb4b 100644
--- gcc/rtl.h
+++ gcc/rtl.h
@@ -1987,6 +1987,7 @@ extern enum machine_mode choose_hard_reg_mode (unsigned int, unsigned int,
 					       bool);
 
 /* In emit-rtl.c  */
+extern rtx set_for_reg_notes (rtx);
 extern rtx set_unique_reg_note (rtx, enum reg_note, rtx);
 extern rtx set_dst_reg_note (rtx, enum reg_note, rtx, rtx);
 extern void set_insn_deleted (rtx);