public inbox for gcc-patches@gcc.gnu.org
 help / color / mirror / Atom feed
From: Steven Bosscher <stevenb.gcc@gmail.com>
To: Eric Botcazou <ebotcazou@adacore.com>
Cc: gcc-patches@gcc.gnu.org, Paolo Bonzini <bonzini@gnu.org>,
		Dominique Dhumieres <dominiq@lps.ens.fr>,
	hubicka@ucw.cz, Richard Henderson <rth@redhat.com>,
		Jeff Law <law@redhat.com>
Subject: Re: Fix twolf -funroll-loops -O3 miscompilation (a semi-latent web.c bug)
Date: Wed, 28 Nov 2012 23:54:00 -0000	[thread overview]
Message-ID: <CABu31nN0zpEpi4V4JJU=h+NZgUS89gYDrmhRozz3Z+LATwJ6bg@mail.gmail.com> (raw)
In-Reply-To: <1965689.goESbr3phy@polaris>

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

On Wed, Nov 28, 2012 at 11:10 PM, Eric Botcazou wrote:
>> Well, I'm not sure I agree that they are wrong. Consider:
>>
>> S0: r1 = ...
>> S1: r2 = r1 + 10
>> S2: r1 = r2 + 10 { REG_EQUAL r1 + 20 }
>> S3: r3 = r1 + 10
>>
>> Clearly it would be wrong to use the REG_EQUAL note on S2 to optimize S3 to:
>>
>> S0: r1 = ...
>> S1: r2 = r1 + 10
>> S2: r1 = r1 + 20
>> S3: r3 = r1 + 30
>
> But the note is wrong by itself.  The semantics is clear: the note means that
> r1 is equal to r1 + 20 right after S2, which doesn't make any sense.

Or maybe the semantics are not what the compiler is actually doing.
Because clearly several places in the compiler can create this kind of
self-referencing note right now, and the main consumer of the notes,
CSE, has apparently not had too much trouble handing them.

But with the documented semantics, you're right. And, to be honest,
I'm of the "the fewer notes, the better" camp :-)


>> 2: debug_rtx((rtx)0x7ffff4df5e58) = (insn 638 637 639 85 (parallel [
>>             (set (reg:SI 891)
>>                 (minus:SI (reg:SI 890)
>>                     (reg:SI 546 [ D.45472 ])))
>>             (clobber (reg:CC 17 flags))
>>         ]) ../../trunk/gcc/value-prof.c:928 283 {*subsi_1}
>>      (expr_list:REG_EQUIV (mem:SI (plus:DI (reg/v/f:DI 204 [ e12 ])
>>                 (const_int 52 [0x34])) [4 e12_223->probability+0 S4 A32])
>>         (expr_list:REG_UNUSED (reg:CC 17 flags)
>>             (expr_list:REG_EQUAL (minus:SI (const_int 10000 [0x2710])
>>                     (reg:SI 546 [ D.45472 ]))
>>                 (nil)))))
>> void
>> (gdb)
>>
>> Is that valid?
>
> Yes, the comment is wrong and should have been removed in r183719:
>   http://gcc.gnu.org/ml/gcc-patches/2012-01/msg01547.html

Ugh, that doesn't look like a very solid fix.

Attached is a different fix. It splits DF_REF_IN_NOTE in two: One flag
for each kind of note. This allows the dead note removal code to
distinguish the source note for the EQ_USES.  I needed to remove one
flag to keep the df_ref_flags 16-bit, but the DF_REF_SUBREG flag looks
completely unnecessary to me, and only ira.c uses it, so it wasn't to
hard to scavenge a single bit.


The patch also includes all places I've found so far where the
compiler could create self-referencing notes:

1. optabs.c: Not sure what it was trying to do, but now it just
refuses to add a note if TARGET is mentioned in one of the source
operands.

2. gcse.c: gcse_emit_move_after added notes, but none of them was very
useful as far as I could tell, and almost all of them turned
self-referencing after CPROP. So I propose we just never add notes in
this case.

3. cprop.c: It seems to me that the purpose here is to propagate
constants. If a reg could not be propagated, then the REG_EQUAL note
will not help much either. Propagating constants via REG_EQUAL notes
still helps folding comparisons sometimes, so I'm proposing we only
propagate those. As a bonus: less garbage RTL because a
cprop_constant_p can be shared.

4. fwprop.c: If the SET_DEST is a REG that is mentioned in the
SET_SRC, don' create a note. This one I'm not very happy with, because
it doesn't handle the case that the REG is somehow simplified out of
the SET_SRC, but being smarter about this would only complicate
things. I for one can't think of anything better for the moment,
anyway.


Finally, it makes sense to compute the NOTE problem for CSE.

Bootstrap&testing in progress at the older revision I've been using to
debug the darwin10 and sparc bugs. I'll test trunk head on x86_64 and
powerpc later. In the mean time, let me hear what you think of this
one :-)

Ciao!
Steven

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

	* ira.c (build_insn_chain): Look at the rtx code to determine if
	the use is actually a SUBREG.
	* df.h (enum df_ref_flags): Add new flags DF_REF_IN_EQUAL_NOTE and
	DF_REF_IN_EQUIV_NOTE, make DF_REF_IN_NOTE a combination of the two,
	and remove DF_REF_SUBREG to keep the number of flags at 16.  Renumber.
	* df-core.c (df_ref_dump): Print 'e' for a REG_EQUAL use,
	and 'q' for a REG_EQUIV use.
	* df-problems.c (df_chain_dump): Likewise.
	(df_remove_dead_eq_notes): Simplify condition.  Look only at refs
	in the kind of note we're processing.  Add sanity check to make
	sure that the SCAN problem is up-to-date, i.e. the EQ_USE reg is
	really mentioned in the note.
	* df-scan.c (df_uses_create): Adjust assert.
	(df_notes_rescan): Split handling of REG_EQUAL and REG_EQUIV notes.
	(df_insn_refs_collect): Likewise.
	(df_def_record_1): Do not set DF_REF_SUBREG.
	(df_uses_record): Likewise.
	* fwprop.c (update_df): Pass DF_REF_IN_EQUAL_NOTE.
	(forward_propagate_and_simplify): Only look in REG_EQUAL notes.
	Do not create self-referencing REG_EQUAL notes.
	(forward_propagate_into): Only look in REG_EQUAL notes.
	* gcse.c (gcse_emit_move_after): Remove.
	(pre_delete): Use emit_insn_after.
	(hoist_code): Likewise.
	* cprop.c (try_replace_reg): Only attach notes if the source was
	folded to a CPROP constant.
	(cprop_jump): Likewise.
	* optabs.c (add_equal_note): Do not create self-referencing REG_EQUAL
	notes.
	* cse.c (cse_main): Add the DF_NOTE problem.

Index: ira.c
===================================================================
--- ira.c	(revision 193394)
+++ ira.c	(working copy)
@@ -3669,7 +3669,7 @@ build_insn_chain (void)
 		       fabricated use. */
 		    if (DF_REF_FLAGS_IS_SET (use, DF_REF_READ_WRITE)
 			&& !DF_REF_FLAGS_IS_SET (use, DF_REF_ZERO_EXTRACT)
-			&& DF_REF_FLAGS_IS_SET (use, DF_REF_SUBREG))
+			&& GET_CODE (DF_REF_REG (use)) == SUBREG)
 		      continue;
 
 		    /* Add the last use of each var to dead_or_set.  */
Index: df.h
===================================================================
--- df.h	(revision 193394)
+++ df.h	(working copy)
@@ -87,30 +87,31 @@ enum df_ref_flags
        bottom of the block.  This is never set for regular refs.  */
     DF_REF_AT_TOP = 1 << 1,
 
-    /* This flag is set if the use is inside a REG_EQUAL or REG_EQUIV
-       note.  */
-    DF_REF_IN_NOTE = 1 << 2,
+    /* This flag is set if the use is inside a REG_EQUAL note,
+       or a REG_EQUIV note, or any one of the two types of note.  */
+    DF_REF_IN_EQUAL_NOTE = 1 << 2,
+    DF_REF_IN_EQUIV_NOTE = 1 << 3,
+    DF_REF_IN_NOTE = DF_REF_IN_EQUAL_NOTE | DF_REF_IN_EQUIV_NOTE,
 
     /* This bit is true if this ref can make regs_ever_live true for
        this regno.  */
-    DF_HARD_REG_LIVE = 1 << 3,
-
+    DF_HARD_REG_LIVE = 1 << 4,
 
     /* This flag is set if this ref is a partial use or def of the
        associated register.  */
-    DF_REF_PARTIAL = 1 << 4,
+    DF_REF_PARTIAL = 1 << 5,
 
     /* Read-modify-write refs generate both a use and a def and
        these are marked with this flag to show that they are not
        independent.  */
-    DF_REF_READ_WRITE = 1 << 5,
+    DF_REF_READ_WRITE = 1 << 6,
 
     /* This flag is set if this ref, generally a def, may clobber the
        referenced register.  This is generally only set for hard
        registers that cross a call site.  With better information
        about calls, some of these could be changed in the future to
        DF_REF_MUST_CLOBBER.  */
-    DF_REF_MAY_CLOBBER = 1 << 6,
+    DF_REF_MAY_CLOBBER = 1 << 7,
 
     /* This flag is set if this ref, generally a def, is a real
        clobber. This is not currently set for registers live across a
@@ -121,25 +122,20 @@ enum df_ref_flags
        clobber is to a subreg.  So in order to tell if the clobber
        wipes out the entire register, it is necessary to also check
        the DF_REF_PARTIAL flag.  */
-    DF_REF_MUST_CLOBBER = 1 << 7,
-
+    DF_REF_MUST_CLOBBER = 1 << 8,
 
     /* If the ref has one of the following two flags set, then the
        struct df_ref can be cast to struct df_ref_extract to access
        the width and offset fields.  */
 
     /* This flag is set if the ref contains a SIGN_EXTRACT.  */
-    DF_REF_SIGN_EXTRACT = 1 << 8,
+    DF_REF_SIGN_EXTRACT = 1 << 9,
 
     /* This flag is set if the ref contains a ZERO_EXTRACT.  */
-    DF_REF_ZERO_EXTRACT = 1 << 9,
+    DF_REF_ZERO_EXTRACT = 1 << 10,
 
     /* This flag is set if the ref contains a STRICT_LOW_PART.  */
-    DF_REF_STRICT_LOW_PART = 1 << 10,
-
-    /* This flag is set if the ref contains a SUBREG.  */
-    DF_REF_SUBREG = 1 << 11,
-
+    DF_REF_STRICT_LOW_PART = 1 << 11,
 
     /* This bit is true if this ref is part of a multiword hardreg.  */
     DF_REF_MW_HARDREG = 1 << 12,
Index: df-core.c
===================================================================
--- df-core.c	(revision 193394)
+++ df-core.c	(working copy)
@@ -2127,9 +2127,10 @@ static void
 df_ref_dump (df_ref ref, FILE *file)
 {
   fprintf (file, "%c%d(%d)",
-	   DF_REF_REG_DEF_P (ref)
-	   ? 'd'
-	   : (DF_REF_FLAGS (ref) & DF_REF_IN_NOTE) ? 'e' : 'u',
+	   DF_REF_REG_DEF_P (ref) ? 'd'
+	   : (DF_REF_FLAGS (ref) & DF_REF_IN_EQUAL_NOTE) ? 'e'
+	   : (DF_REF_FLAGS (ref) & DF_REF_IN_EQUIV_NOTE) ? 'q'
+	   : 'u',
 	   DF_REF_ID (ref),
 	   DF_REF_REGNO (ref));
 }
Index: df-problems.c
===================================================================
--- df-problems.c	(revision 193394)
+++ df-problems.c	(working copy)
@@ -73,9 +73,10 @@ df_chain_dump (struct df_link *link, FIL
   for (; link; link = link->next)
     {
       fprintf (file, "%c%d(bb %d insn %d) ",
-	       DF_REF_REG_DEF_P (link->ref)
-	       ? 'd'
-	       : (DF_REF_FLAGS (link->ref) & DF_REF_IN_NOTE) ? 'e' : 'u',
+	       DF_REF_REG_DEF_P (link->ref) ? 'd'
+	       : (DF_REF_FLAGS (link->ref) & DF_REF_IN_EQUAL_NOTE) ? 'e'
+	       : (DF_REF_FLAGS (link->ref) & DF_REF_IN_EQUIV_NOTE) ? 'q'
+	       : 'u',
 	       DF_REF_ID (link->ref),
 	       DF_REF_BBNO (link->ref),
 	       DF_REF_IS_ARTIFICIAL (link->ref)
@@ -2889,27 +2890,39 @@ df_remove_dead_eq_notes (rtx insn, bitma
 
   while (link)
     {
+      /* A mask for the kind of note we should be looking for.  Because
+	 there is always at most one REG_EQUAL and/or at most one REG_EQUIV
+	 note, and we visit the notes one at a time, we can tell with this
+	 flag whether the EQ_USE we're looking at is in the note we are
+	 processing.  */
+      int note_kind_flag = DF_REF_IN_EQUAL_NOTE;
       switch (REG_NOTE_KIND (link))
 	{
-	case REG_EQUAL:
 	case REG_EQUIV:
+	  note_kind_flag = DF_REF_IN_EQUIV_NOTE;
+	  /* fallthru */
+	case REG_EQUAL:
 	  {
-	    /* Remove the notes that refer to dead registers.  As we have at most
-	       one REG_EQUAL/EQUIV note, all of EQ_USES will refer to this note
-	       so we need to purge the complete EQ_USES vector when removing
-	       the note using df_notes_rescan.  */
 	    df_ref *use_rec;
 	    bool deleted = false;
 
 	    for (use_rec = DF_INSN_EQ_USES (insn); *use_rec; use_rec++)
 	      {
 		df_ref use = *use_rec;
-		if (DF_REF_REGNO (use) > FIRST_PSEUDO_REGISTER
-		    && DF_REF_LOC (use)
-		    && (DF_REF_FLAGS (use) & DF_REF_IN_NOTE)
-		    && ! bitmap_bit_p (live, DF_REF_REGNO (use))
-		    && loc_mentioned_in_p (DF_REF_LOC (use), XEXP (link, 0)))
+
+		if (! (DF_REF_FLAGS (use) & note_kind_flag))
+		  continue;
+
+		/* A REG_EQUAL/REG_EQUIV note may not refer registers that
+		   are not live at the point of the EQ_USE.  Notes referencing
+		   registers without a definition are also dead.  (And wrong,
+		   but that's another story...)  */
+		if ((DF_REF_REGNO (use) > FIRST_PSEUDO_REGISTER
+		     && ! bitmap_bit_p (live, DF_REF_REGNO (use)))
+		    || DF_REG_DEF_COUNT (DF_REF_REGNO (use)) == 0)
 		  {
+		    gcc_checking_assert (reg_mentioned_p (DF_REF_REAL_REG (use),
+							  XEXP (link, 0)));
 		    deleted = true;
 		    break;
 		  }
Index: df-scan.c
===================================================================
--- df-scan.c	(revision 193394)
+++ df-scan.c	(working copy)
@@ -682,13 +682,16 @@ df_scan_blocks (void)
 }
 
 /* Create new refs under address LOC within INSN.  This function is
-   only used externally.  REF_FLAGS must be either 0 or DF_REF_IN_NOTE,
-   depending on whether LOC is inside PATTERN (INSN) or a note.  */
+   only used externally.  REF_FLAGS must be either 0 or one of the
+   flavors of DF_REF_IN_NOTE flags, depending on whether LOC is
+   inside PATTERN (INSN) or a note.  */
 
 void
 df_uses_create (rtx *loc, rtx insn, int ref_flags)
 {
-  gcc_assert (!(ref_flags & ~DF_REF_IN_NOTE));
+  gcc_checking_assert (ref_flags == 0
+		       || ref_flags == DF_REF_IN_EQUAL_NOTE
+		       || ref_flags == DF_REF_IN_EQUIV_NOTE);
   df_uses_record (NULL, loc, DF_REF_REG_USE,
                   BLOCK_FOR_INSN (insn),
                   DF_INSN_INFO_GET (insn),
@@ -2212,10 +2215,17 @@ df_notes_rescan (rtx insn)
 	  switch (REG_NOTE_KIND (note))
 	    {
 	    case REG_EQUIV:
+	      df_uses_record (&collection_rec,
+			      &XEXP (note, 0), DF_REF_REG_USE,
+			      bb, insn_info, DF_REF_IN_EQUIV_NOTE);
+	      break;
+
 	    case REG_EQUAL:
 	      df_uses_record (&collection_rec,
 			      &XEXP (note, 0), DF_REF_REG_USE,
-			      bb, insn_info, DF_REF_IN_NOTE);
+			      bb, insn_info, DF_REF_IN_EQUAL_NOTE);
+	      break;
+
 	    default:
 	      break;
 	    }
@@ -2975,8 +2985,6 @@ df_def_record_1 (struct df_collection_re
       if (df_read_modify_subreg_p (dst))
 	flags |= DF_REF_READ_WRITE | DF_REF_PARTIAL;
 
-      flags |= DF_REF_SUBREG;
-
       df_ref_record (DF_REF_REGULAR, collection_rec,
 		     dst, loc, bb, insn_info, DF_REF_REG_DEF, flags);
     }
@@ -3190,7 +3198,7 @@ df_uses_record (struct df_collection_rec
 		{
 		  df_uses_record (collection_rec, &SUBREG_REG (dst),
 				  DF_REF_REG_USE, bb, insn_info,
-				  flags | DF_REF_READ_WRITE | DF_REF_SUBREG);
+				  flags | DF_REF_READ_WRITE);
 		  break;
 		}
 	      /* Fall through.  */
@@ -3463,17 +3471,21 @@ df_insn_refs_collect (struct df_collecti
   VEC_truncate (df_ref, collection_rec->eq_use_vec, 0);
   VEC_truncate (df_mw_hardreg_ptr, collection_rec->mw_vec, 0);
 
-  /* Process REG_EQUIV/REG_EQUAL notes.  */
+  /* Process REG_EQUIV/REG_EQUAL/REG_NON_LOCAL_GOTO notes.  */
   for (note = REG_NOTES (insn_info->insn); note;
        note = XEXP (note, 1))
     {
       switch (REG_NOTE_KIND (note))
         {
         case REG_EQUIV:
+          df_uses_record (collection_rec,
+                          &XEXP (note, 0), DF_REF_REG_USE,
+                          bb, insn_info, DF_REF_IN_EQUIV_NOTE);
+          break;
         case REG_EQUAL:
           df_uses_record (collection_rec,
                           &XEXP (note, 0), DF_REF_REG_USE,
-                          bb, insn_info, DF_REF_IN_NOTE);
+                          bb, insn_info, DF_REF_IN_EQUAL_NOTE);
           break;
         case REG_NON_LOCAL_GOTO:
           /* The frame ptr is used by a non-local goto.  */
Index: fwprop.c
===================================================================
--- fwprop.c	(revision 193394)
+++ fwprop.c	(working copy)
@@ -934,7 +934,7 @@ update_df (rtx insn, rtx note)
 
   if (note)
     {
-      df_uses_create (&XEXP (note, 0), insn, DF_REF_IN_NOTE);
+      df_uses_create (&XEXP (note, 0), insn, DF_REF_IN_EQUAL_NOTE);
       df_notes_rescan (insn);
     }
   else
@@ -1311,7 +1311,7 @@ forward_propagate_and_simplify (df_ref u
   else
     {
       rtx note = find_reg_note (use_insn, REG_EQUAL, NULL_RTX);
-      if (DF_REF_FLAGS (use) & DF_REF_IN_NOTE)
+      if (DF_REF_FLAGS (use) & DF_REF_IN_EQUAL_NOTE)
 	loc = &XEXP (note, 0);
       else
 	loc = &SET_SRC (use_set);
@@ -1319,9 +1319,12 @@ forward_propagate_and_simplify (df_ref u
       /* Do not replace an existing REG_EQUAL note if the insn is not
 	 recognized.  Either we're already replacing in the note, or we'll
 	 separately try plugging the definition in the note and simplifying.
-	 And only install a REQ_EQUAL note when the destination is a REG,
-	 as the note would be invalid otherwise.  */
-      set_reg_equal = (note == NULL_RTX && REG_P (SET_DEST (use_set)));
+	 And only install a REQ_EQUAL note when the destination is a REG
+	 that isn't mentioned in USE_SET, as the note would be invalid
+	 otherwise.  */
+      set_reg_equal = (note == NULL_RTX && REG_P (SET_DEST (use_set))
+		       && ! reg_mentioned_p (SET_DEST (use_set),
+					     SET_SRC (use_set)));
     }
 
   if (GET_MODE (*loc) == VOIDmode)
@@ -1370,7 +1373,7 @@ forward_propagate_into (df_ref use)
 
   /* Check if the use is still present in the insn!  */
   use_insn = DF_REF_INSN (use);
-  if (DF_REF_FLAGS (use) & DF_REF_IN_NOTE)
+  if (DF_REF_FLAGS (use) & DF_REF_IN_EQUAL_NOTE)
     parent = find_reg_note (use_insn, REG_EQUAL, NULL_RTX);
   else
     parent = PATTERN (use_insn);
Index: gcse.c
===================================================================
--- gcse.c	(revision 193394)
+++ gcse.c	(working copy)
@@ -501,7 +501,6 @@ static void trim_ld_motion_mems (void);
 static void update_ld_motion_stores (struct expr *);
 static void clear_modify_mem_tables (void);
 static void free_modify_mem_tables (void);
-static rtx gcse_emit_move_after (rtx, rtx, rtx);
 static bool is_too_expensive (const char *);
 
 #define GNEW(T)			((T *) gmalloc (sizeof (T)))
@@ -2473,36 +2472,6 @@ pre_insert_copies (void)
       }
 }
 
-/* Emit move from SRC to DEST noting the equivalence with expression computed
-   in INSN.  */
-
-static rtx
-gcse_emit_move_after (rtx dest, rtx src, rtx insn)
-{
-  rtx new_rtx;
-  rtx set = single_set (insn), set2;
-  rtx note;
-  rtx eqv;
-
-  /* This should never fail since we're creating a reg->reg copy
-     we've verified to be valid.  */
-
-  new_rtx = emit_insn_after (gen_move_insn (dest, src), insn);
-
-  /* Note the equivalence for local CSE pass.  */
-  set2 = single_set (new_rtx);
-  if (!set2 || !rtx_equal_p (SET_DEST (set2), dest))
-    return new_rtx;
-  if ((note = find_reg_equal_equiv_note (insn)))
-    eqv = XEXP (note, 0);
-  else
-    eqv = SET_SRC (set);
-
-  set_unique_reg_note (new_rtx, REG_EQUAL, copy_insn_1 (eqv));
-
-  return new_rtx;
-}
-
 /* Delete redundant computations.
    Deletion is done by changing the insn to copy the `reaching_reg' of
    the expression into the result of the SET.  It is left to later passes
@@ -2542,7 +2511,8 @@ pre_delete (void)
 		if (expr->reaching_reg == NULL)
 		  expr->reaching_reg = gen_reg_rtx_and_attrs (SET_DEST (set));
 
-		gcse_emit_move_after (SET_DEST (set), expr->reaching_reg, insn);
+		emit_insn_after (gen_move_insn (SET_DEST (set),
+						expr->reaching_reg), insn);
 		delete_insn (insn);
 		occr->deleted_p = 1;
 		changed = 1;
@@ -3252,8 +3222,8 @@ hoist_code (void)
 		    expr->reaching_reg
 		      = gen_reg_rtx_and_attrs (SET_DEST (set));
 
-		  gcse_emit_move_after (SET_DEST (set), expr->reaching_reg,
-					insn);
+		  emit_insn_after (gen_move_insn (SET_DEST (set),
+						  expr->reaching_reg), insn);
 		  delete_insn (insn);
 		  occr->deleted_p = 1;
 		  changed = 1;
Index: cprop.c
===================================================================
--- cprop.c	(revision 193394)
+++ cprop.c	(working copy)
@@ -774,10 +774,11 @@ try_replace_reg (rtx from, rtx to, rtx i
 	success = 1;
 
       /* If we've failed perform the replacement, have a single SET to
-	 a REG destination and don't yet have a note, add a REG_EQUAL note
-	 to not lose information.  */
-      if (!success && note == 0 && set != 0 && REG_P (SET_DEST (set)))
-	note = set_unique_reg_note (insn, REG_EQUAL, copy_rtx (src));
+	 a REG destination, don't yet have a note, and have simplified
+	 SRC to a constant, add a REG_EQUAL note to not lose information.  */
+      if (!success && note == 0 && set != 0 && REG_P (SET_DEST (set))
+	  && cprop_constant_p (src))
+	note = set_unique_reg_note (insn, REG_EQUAL, src);
     }
 
   if (set && MEM_P (SET_DEST (set)) && reg_mentioned_p (from, SET_DEST (set)))
@@ -936,8 +937,9 @@ cprop_jump (basic_block bb, rtx setcc, r
 	     we need to attach a note to the branch itself to make this
 	     optimization work.  */
 
-	  if (!rtx_equal_p (new_rtx, note_src))
-	    set_unique_reg_note (jump, REG_EQUAL, copy_rtx (new_rtx));
+	  if (!rtx_equal_p (new_rtx, note_src)
+	      && cprop_constant_p (new_rtx))
+	    set_unique_reg_note (jump, REG_EQUAL, new_rtx);
 	  return 0;
 	}
 
Index: optabs.c
===================================================================
--- optabs.c	(revision 193394)
+++ optabs.c	(working copy)
@@ -170,14 +170,14 @@ optab_libfunc (optab optab, enum machine
 
    If the last insn does not set TARGET, don't do anything, but return 1.
 
-   If a previous insn sets TARGET and TARGET is one of OP0 or OP1,
-   don't add the REG_EQUAL note but return 0.  Our caller can then try
-   again, ensuring that TARGET is not one of the operands.  */
+   If the last insn or a previous insn sets TARGET and TARGET is one of OP0
+   or OP1, don't add the REG_EQUAL note but return 0.  Our caller can then
+   try again, ensuring that TARGET is not one of the operands.  */
 
 static int
 add_equal_note (rtx insns, rtx target, enum rtx_code code, rtx op0, rtx op1)
 {
-  rtx last_insn, insn, set;
+  rtx last_insn, set;
   rtx note;
 
   gcc_assert (insns && INSN_P (insns) && NEXT_INSN (insns));
@@ -192,6 +192,12 @@ add_equal_note (rtx insns, rtx target, e
   if (GET_CODE (target) == ZERO_EXTRACT)
     return 1;
 
+  /* If TARGET is in OP0 or OP1, punt.  We'd end up with a note referencing
+     a value changing in the insn, so the note would be invalid for CSE.  */
+  if (reg_overlap_mentioned_p (target, op0)
+      || (op1 && reg_overlap_mentioned_p (target, op1)))
+    return 0;
+
   for (last_insn = insns;
        NEXT_INSN (last_insn) != NULL_RTX;
        last_insn = NEXT_INSN (last_insn))
@@ -207,21 +213,6 @@ add_equal_note (rtx insns, rtx target, e
 	  || ! rtx_equal_p (XEXP (SET_DEST (set), 0), target)))
     return 1;
 
-  /* If TARGET is in OP0 or OP1, check if anything in SEQ sets TARGET
-     besides the last insn.  */
-  if (reg_overlap_mentioned_p (target, op0)
-      || (op1 && reg_overlap_mentioned_p (target, op1)))
-    {
-      insn = PREV_INSN (last_insn);
-      while (insn != NULL_RTX)
-	{
-	  if (reg_set_p (target, insn))
-	    return 0;
-
-	  insn = PREV_INSN (insn);
-	}
-    }
-
   if (GET_RTX_CLASS (code) == RTX_UNARY)
     switch (code)
       {
Index: cse.c
===================================================================
--- cse.c	(revision 193394)
+++ cse.c	(working copy)
@@ -6521,6 +6521,7 @@ cse_main (rtx f ATTRIBUTE_UNUSED, int nr
   int i, n_blocks;
 
   df_set_flags (DF_LR_RUN_DCE);
+  df_note_add_problem ();
   df_analyze ();
   df_set_flags (DF_DEFER_INSN_RESCAN);
 

  reply	other threads:[~2012-11-28 23:54 UTC|newest]

Thread overview: 68+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2012-11-18 23:15 Dominique Dhumieres
2012-11-19  9:50 ` Steven Bosscher
2012-11-19 11:27   ` Eric Botcazou
2012-11-19 20:35     ` Steven Bosscher
2012-11-19 21:20       ` Steven Bosscher
2012-11-19 21:52         ` Eric Botcazou
2012-11-19 22:03           ` Steven Bosscher
2012-11-19 22:44             ` Eric Botcazou
2012-11-19 22:48               ` Steven Bosscher
2012-11-23 22:46                 ` Steven Bosscher
2012-11-24  1:10                   ` Steven Bosscher
2012-11-25 20:44                     ` Steven Bosscher
2012-11-25 22:38                       ` Steven Bosscher
2012-11-26 14:38                         ` Dominique Dhumieres
2012-11-26 15:46                           ` Steven Bosscher
2012-11-27  9:58                         ` Eric Botcazou
2012-11-27 10:35                           ` Steven Bosscher
2012-11-27 12:01                             ` Steven Bosscher
2012-11-27 12:29                               ` Steven Bosscher
2012-11-27 13:04                                 ` Steven Bosscher
2012-11-27 14:25                                   ` Dominique Dhumieres
2012-11-27 14:49                                     ` Steven Bosscher
2012-11-27 13:48                                 ` Dominique Dhumieres
2012-11-27 14:33                               ` Paolo Bonzini
2012-11-27 16:59                                 ` Eric Botcazou
2012-11-27 23:29                                   ` Steven Bosscher
2012-11-27 23:50                                     ` Eric Botcazou
2012-11-27 23:54                                       ` Steven Bosscher
2012-11-27 23:59                                         ` Steven Bosscher
2012-11-28  0:43                                           ` Steven Bosscher
2012-11-28  7:46                                             ` Eric Botcazou
2012-11-28 15:57                                               ` Steven Bosscher
2012-11-28 22:12                                                 ` Eric Botcazou
2012-11-28 23:54                                                   ` Steven Bosscher [this message]
2012-12-01 14:57                                                     ` Eric Botcazou
2012-12-01 16:45                                                       ` Steven Bosscher
2012-12-03 18:26                                                         ` Eric Botcazou
2012-12-03 20:20                                                           ` Steven Bosscher
2012-12-03 21:12                                                             ` Eric Botcazou
2012-12-03 23:28                                                             ` Steven Bosscher
2012-12-03 20:15                                                       ` Paolo Bonzini
2012-11-19 21:29       ` Eric Botcazou
  -- strict thread matches above, loose matches on Subject: below --
2012-10-12 20:14 Jan Hubicka
2012-10-12 20:36 ` Markus Trippelsdorf
2012-10-12 20:44 ` Steven Bosscher
2012-10-12 21:16   ` Jan Hubicka
2012-10-12 21:19     ` Steven Bosscher
2012-10-12 21:31       ` Jan Hubicka
2012-10-12 22:41         ` Steven Bosscher
2012-10-14  9:13           ` Paolo Bonzini
2012-10-14 21:27             ` Steven Bosscher
2012-10-14 21:35               ` Eric Botcazou
2012-10-14 21:45                 ` Steven Bosscher
2012-10-15  8:14               ` Paolo Bonzini
2012-10-15  8:23                 ` Steven Bosscher
2012-10-15  8:35                   ` Paolo Bonzini
2012-10-15  8:38                     ` Steven Bosscher
2012-10-15 10:49                       ` Steven Bosscher
2012-10-15 12:28                       ` Paolo Bonzini
2012-10-15 13:19                         ` Steven Bosscher
2012-10-15 13:29                           ` Paolo Bonzini
2012-10-15 13:49                             ` Steven Bosscher
2012-10-16 10:35                               ` Steven Bosscher
2012-10-16 11:05                                 ` Steven Bosscher
2012-10-16 11:42                                   ` Paolo Bonzini
2012-10-16 22:57                                     ` Steven Bosscher
2012-10-19  5:14                                       ` Bin.Cheng
2012-10-12 21:05 ` Steven Bosscher

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to='CABu31nN0zpEpi4V4JJU=h+NZgUS89gYDrmhRozz3Z+LATwJ6bg@mail.gmail.com' \
    --to=stevenb.gcc@gmail.com \
    --cc=bonzini@gnu.org \
    --cc=dominiq@lps.ens.fr \
    --cc=ebotcazou@adacore.com \
    --cc=gcc-patches@gcc.gnu.org \
    --cc=hubicka@ucw.cz \
    --cc=law@redhat.com \
    --cc=rth@redhat.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).