On 05/03/2013 07:10 AM, Julian Brown wrote:
Hi,

This is a patch which fixes a latent bug in RTL GCSE/PRE, described
more fully in:

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=57159

I haven't been able to reproduce the problem on mainline (nor on a
supported target). Maybe someone more familiar with the code in question
than I am can tell if the patch is correct nonetheless?

Thanks,

Julian

ChangeLog

     gcc/
     * gcse.c (compute_ld_motion_mems): Invalidate non-simple mem refs
     in REG_EQUAL notes.


pre-bugfix-1.diff


Index: gcc/gcse.c
===================================================================
--- gcc/gcse.c  (revision 198175)
+++ gcc/gcse.c  (working copy)
@@ -3888,6 +3888,13 @@ compute_ld_motion_mems (void)
                {
                  rtx src = SET_SRC (PATTERN (insn));
                  rtx dest = SET_DEST (PATTERN (insn));
+                 rtx note = find_reg_equal_equiv_note (insn);
+                 rtx src_eq;
+
+                 if (note != 0 && REG_NOTE_KIND (note) == REG_EQUAL)
+                   src_eq = XEXP (note, 0);
+                 else
+                   src_eq = NULL_RTX;

                  /* Check for a simple LOAD...  */
                  if (MEM_P (src) && simple_mem (src))
@@ -3904,6 +3911,12 @@ compute_ld_motion_mems (void)
                      invalidate_any_buried_refs (src);
                    }

+                 /* Also invalidate any buried loads which may be present in
+                    REG_EQUAL notes.  */
+                 if (src_eq != NULL_RTX
+                     && !(MEM_P (src_eq) && simple_mem (src_eq)))
+                   invalidate_any_buried_refs (src_eq);
+
                  /* Check for stores. Don't worry about aliased ones, they
                     will block any movement we might do later. We only care
                     about this exact pattern since those are the only

Is there any good reason why the search for the note is separated from the invalidation code. As far as I can tell, both the search for the note and the call to invalidate_any_buried_refs ought to be in a single block of uninterrupted code.

What happens if the code contains a simple mem? We don't invalidate it as far as I can tell. Doesn't that open us up to the same problems that we're seeing with with the non-simple mem?

jeff

Reply via email to