Re: [PATCH] Avoid emitting useless notes in GCSE

Re: [PATCH] Avoid emitting useless notes in GCSE

[Joern RENNECKE]

 > Hello,
 >
 > While working on an unrelated problem, it occured to me that GCSE can 
emit
 > seemingly useless REG_EQUAL notes, that is to say REQ_EQUAL notes that
 > contain the copy of the source of the SET they are attached to.
 >
 > Is there any benefit in doing so?  If no, I propose the attached 
patch for
 > mainline (I'll properly test it if it is okayed on principle).

If a later pass, like RTL LICM, hoists out a constant, the REG_EQUAL note
becomes relevant.  reload needs such notes to put the constant back if the
pseudo created by LICM failed to get a hard register.

Re: [PATCH] Avoid emitting useless notes in GCSE

[Eric Botcazou]

> If a later pass, like RTL LICM, hoists out a constant, the REG_EQUAL note
> becomes relevant.  reload needs such notes to put the constant back if the
> pseudo created by LICM failed to get a hard register.

Do you mean the pass that modifies the insn is not responsible for emitting 
the note?

-- 
Eric Botcazou

Re: [PATCH] Avoid emitting useless notes in GCSE

[Joern RENNECKE]

Eric Botcazou wrote:

>>If a later pass, like RTL LICM, hoists out a constant, the REG_EQUAL note
>>becomes relevant.  reload needs such notes to put the constant back if the
>>pseudo created by LICM failed to get a hard register.
>>    
>>
>
>Do you mean the pass that modifies the insn is not responsible for emitting 
>the note?
>
IIRC that responsibility used to be considered to lie with the code that 
emitted the instruction
in the first place.  However, that was never perfectly implemented, 
particularily since there
are unclear areas which code is responsible when tree->rtl generation 
calls nested expanders,
and after more than five years with this code in gcse, this note-adding 
functionality in passes
before gcse is likely to be atrophied.  It appears saner to make the 
later passes add the notes
when they change the source, but if some fail to do that, there is a 
benefit to having the notes
placed there by gcse.  And you asked if there is a benefit to this, not 
if there was a better way
to get that benefit...

Re: [PATCH] Avoid emitting useless notes in GCSE

[Eric Botcazou]

> It appears saner to make the later passes add the notes
> when they change the source, but if some fail to do that, there is a
> benefit to having the notes placed there by gcse.

This kind of reasoning would justify adding a REG_EQUAL note containing the 
source for every single SET emitted in the RTL stream!

> And you asked if there is a benefit to this, not if there was a better way
> to get that benefit...

Yes, because I've had my share of bogus REG_EQUAL notes confusing subsequent 
passes.  Lean and mean is my motto when it comes to notes. :-)

-- 
Eric Botcazou

Re: [PATCH] Avoid emitting useless notes in GCSE

[Roger Sayle]

On Wed, 30 Mar 2005, Eric Botcazou wrote:
> 2005-03-30  Eric Botcazou  <ebotcazou@adacore.com>
>
> 	* gcse.c (try_replace_reg): Do not emit a REG_EQUAL note if the source
> 	of the SET has not been modified.

This is OK if properly tested.  I acknowledge Joern's concern that there
could theoretically be places where these "useless" notes may potentially
help.  However, this requires (i) GCSE to add the duplicate REG_EQUAL
note, (ii) Some later pass to eliminate/replace the constant with
something else (perhaps reload) and (iii) some even later pass to benefit
from having the REG_EQUAL note there.  Possible, but unlikely.

Instead, not only do we use more memory tracking these notes, but they
also affect compile-time when used in cselib, GCSE and particularly
"combine_insns" in combine.c.  There's an argument to increase the use
of REG_EQUAL notes in combine.c, which is already a performance hot-spot
and each duplicate note would double the time taken to process each
combinable insn.

As Joern suggested/agreed, if we ever find a case where these notes
improve(d) things, we should fix the (ii) pass, that eliminated the
constant operand without (re)placing a REG_EQUAL note.  To confirm that
your change typically doesn't affect generated code, you might want
to double check that "cc1" (or some similar large corpus) generates
identically the same code after as before.

Roger
--

[PATCH] Avoid emitting useless notes in GCSE

[Eric Botcazou]

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

Hello,

While working on an unrelated problem, it occured to me that GCSE can emit 
seemingly useless REG_EQUAL notes, that is to say REQ_EQUAL notes that 
contain the copy of the source of the SET they are attached to.

Is there any benefit in doing so?  If no, I propose the attached patch for 
mainline (I'll properly test it if it is okayed on principle).


2005-03-30  Eric Botcazou  <ebotcazou@adacore.com>

	* gcse.c (try_replace_reg): Do not emit a REG_EQUAL note if the source
	of the SET has not been modified.


-- 
Eric Botcazou

[-- Attachment #2: gcse_useless_notes.diff --]
[-- Type: text/x-diff, Size: 1411 bytes --]

Index: gcse.c
===================================================================
RCS file: /cvs/gcc/gcc/gcc/gcse.c,v
retrieving revision 1.337
diff -u -p -r1.337 gcse.c
--- gcse.c	11 Mar 2005 09:04:58 -0000	1.337
+++ gcse.c	30 Mar 2005 12:09:47 -0000
@@ -2656,16 +2656,18 @@ try_replace_reg (rtx from, rtx to, rtx i
 	 SETs, but it probably won't buy us anything.  */
       src = simplify_replace_rtx (SET_SRC (set), from, to);
 
-      if (!rtx_equal_p (src, SET_SRC (set))
-	  && validate_change (insn, &SET_SRC (set), src, 0))
-	success = 1;
-
-      /* If we've failed to do replacement, have a single SET, don't already
-	 have a note, and have no special SET, add a REG_EQUAL note to not
-	 lose information.  */
-      if (!success && note == 0 && set != 0
-	  && GET_CODE (SET_DEST (set)) != ZERO_EXTRACT)
-	note = set_unique_reg_note (insn, REG_EQUAL, copy_rtx (src));
+      if (!rtx_equal_p (src, SET_SRC (set)))
+	{
+	  if (validate_change (insn, &SET_SRC (set), src, 0))
+	    success = 1;
+
+	  /* If we've failed to do replacement, have a single SET, don't
+	     already have a note, and have no special SET, add a REG_EQUAL
+	     note to not lose information.  */
+	  if (!success && note == 0 && set != 0
+	      && GET_CODE (SET_DEST (set)) != ZERO_EXTRACT)
+	    note = set_unique_reg_note (insn, REG_EQUAL, copy_rtx (src));
+	}
     }
 
   /* REG_EQUAL may get simplified into register.