This is the mail archive of the gcc@gcc.gnu.org mailing list for the GCC project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

Is nonoverlapping_memrefs_p wrong for unknown offsets?


We recently tracked down an aliasing bug in gcc-4.4.3 (found by our
TILE-Gx back end, not yet ready to contribute), and we wanted to make
sure the fix is right.

try_crossjump_bb identifies some common insns in SPEC2000.eon and uses
merge_memattrs to merge them.  To do so, it has to unify their
aliasing data such that any insn that aliased either of the original
insns aliases the merged insn.  In our case we have two
identical-looking insns that are actually referencing different stack
spill slots, so their MEM_OFFSETs are different.  merge_memattrs
correctly NULLs out the MEM_OFFSET of the merged insn to indicate it's
not sure what the offset is, although it leaves a non-NULL MEM_SIZE:

	  else if (MEM_OFFSET (x) != MEM_OFFSET (y))
	    {
	      set_mem_offset (x, 0);
	      set_mem_offset (y, 0);
	    }

Later, nonoverlapping_memrefs_p decides that this merged insn does not
alias another spill slot insn (one which has a valid MEM_OFFSET), but
in fact they do alias. The scheduler then creates an incorrect schedule.

The bug seems to be that nonoverlapping_memrefs_p does not
conservatively assume that a NULL MEM_OFFSET can alias any offset,
despite comments to the contrary. Instead it essentially treats it
like a known offset of zero.  We don't understand why it doesn't just
give up if the references have the same base but one of the offsets is
unknown.  A minimal patch to do so would look like this, but if this
is correct, then code after this short-circuit can be simplified,
yielding a larger patch (which I could produce):

--- //tilera/branch/gcc/tools/gcc/gcc/alias.c   2010/04/12 10:29:48
+++ //tilera/branch/gcc/tools/gcc/gcc/alias.c   2010/04/12 10:36:35
@@ -2150,6 +2150,11 @@
           : MEM_SIZE (rtly) ? INTVAL (MEM_SIZE (rtly)) :
           -1);
 
+  /* If we don't have an offset for both memrefs, we have to assume they can
+     be anywhere in the DECL's MEM. */
+  if (!moffsetx || !moffsety)
+    return 0;
+
   /* If we have an offset for either memref, it can update the values computed
      above.  */
   if (moffsetx)

-Mat


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]