This is the mail archive of the gcc-patches@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]

[RFA:] Fix PR rtl-optimization/20466, flow not invalidating knowledge for (mem (mem ...))


For this source:
int f (int **ipp, int *i1p, int *i2p, int **i3, int **i4)
{
  **ipp = *i1p;
  *ipp = i2p;
  *i3 = *i4;
  **ipp = 99;
  return 3;
}

For cris-axis-elf at -O2, we have the following (pruned) RTL
right before .flow2:

(insn 38 17 18 0 (set (reg:SI 11 r11)
        (mem:SI (reg:SI 11 r11 [ i1p ]) [2 S4 A8])) 32 {*movsi_internal} (nil)
    (nil))

(insn 18 38 20 0 (set (mem:SI (mem/f:SI (reg/v/f:SI 10 r10 [orig:24 ipp ] [24]) [3 S4 A8]) [2 S4 A8])
        (reg:SI 11 r11)) 32 {*movsi_internal} (insn_list:REG_DEP_TRUE 6 (nil))
    (nil))

(insn 20 18 22 0 (set (mem/f:SI (reg/v/f:SI 10 r10 [orig:24 ipp ] [24]) [3 S4 A8])
        (reg:SI 12 r12 [ i2p ])) 32 {*movsi_internal} (nil)
    (nil))

(insn 41 22 42 0 (set (reg:SI 9 r9)
        (mem/i:SI (reg/f:SI 14 sp) [4 i4+0 S4 A32])) 32 {*movsi_internal} (nil)
    (nil))

(insn 42 41 23 0 (set (reg:SI 9 r9)
        (mem:SI (reg:SI 9 r9) [3 S4 A8])) 32 {*movsi_internal} (nil)
    (nil))

(insn 23 42 25 0 (set (mem/f:SI (reg:SI 13 r13 [ i3 ]) [3 S4 A8])
        (reg:SI 9 r9)) 32 {*movsi_internal} (nil)
    (nil))

(insn 40 26 27 0 (set (reg:SI 9 r9)
        (const_int 99 [0x63])) 32 {*movsi_internal} (nil)
    (nil))

(insn 27 40 28 0 (set (mem:SI (mem/f:SI (reg/v/f:SI 10 r10 [orig:24 ipp ] [24]) [3 S4 A8]) [2 S4 A8])
        (reg:SI 9 r9)) 32 {*movsi_internal} (nil)
    (nil))

In the .flow2 dump, insn 38 and 18 (corresponding to the first
"**ipp = *i1p" assignment) are deleted.  They are perceived as
dead due to the later assignment, "**ipp = 99".  But GCC misses
that the later **ipp is different to the first **ipp due to the
*ipp assignment in between; GCC does not look at SETs of memory
as invalidating knowledge of *later* SETs of memory; for that,
it only checks SETs of registers.  I contemplated just adding
the "|| MEM_P (reg)" mark_set_1 patch, but that would have cause
any SET of memory to conflict with any later SET of memory:
likely a noticable performance regression.  I also contemplated
a new invalidate_mems_from_set_of_mem instead of the
invalidate_mems_from_set, but thought that'd cause loss of
readbility just to avoid a REG_P and a MEM_P test.  An
alternative solution would be to add a new test function to look
for nested mems; (mem X) where X contains another mem, and not
call add_to_mem_set_list for them.  I didn't like that, because
it'd codify the knowledge that reg_overlap_mentioned_p
(misnomer, since it works for MEMs) always returns positive on
checking MEM overlap.

Built and checked with Geoff's script on cris-axis-elf,
mmix-knuth-mmixware, frv-elf, bootstrapped and checked on
i686-pc-linux-gnu.

Ok to commit?

gcc/testsuite:

	PR rtl-optimization/20466
	* gcc.c-torture/execute/pr20466-1.c: New test.

gcc:
	PR rtl-optimization/20466
	* flow.c (invalidate_mems_from_set): Handle a MEM by checking it
	for overlap of the address of each list member.
	(mark_set_1): Call invalidate_mems_from_set for MEMs too.	

--- /dev/null	Tue Oct 29 15:57:07 2002
+++ gcc.c-torture/execute/pr20466-1.c	Mon Mar 14 06:19:12 2005
@@ -0,0 +1,26 @@
+int f (int **, int *, int *, int **, int **) __attribute__ ((__noinline__));
+int
+f (int **ipp, int *i1p, int *i2p, int **i3, int **i4)
+{
+  **ipp = *i1p;
+  *ipp = i2p;
+  *i3 = *i4;
+  **ipp = 99;
+  return 3;
+}
+
+extern void exit (int);
+extern void abort (void);
+
+int main (void)
+{
+  int i = 42, i1 = 66, i2 = 1, i3 = -1, i4 = 55;
+  int *ip = &i;
+  int *i3p = &i3;
+  int *i4p = &i4;
+
+  f (&ip, &i1, &i2, &i3p, &i4p);
+  if (i != 66 || ip != &i2 || i2 != 99 || i3 != -1 || i3p != i4p || i4 != 55)
+    abort ();
+  exit (0);
+}

Index: flow.c
===================================================================
RCS file: /cvs/gcc/gcc/gcc/flow.c,v
retrieving revision 1.621
diff -c -p -u -p -r1.621 flow.c
--- flow.c	7 Mar 2005 13:47:37 -0000	1.621
+++ flow.c	14 Mar 2005 18:14:52 -0000
@@ -2530,7 +2530,8 @@ invalidate_mems_from_autoinc (rtx *px, v
   return 0;
 }
 
-/* EXP is a REG.  Remove any dependent entries from pbi->mem_set_list.  */
+/* EXP is a REG or MEM.  Remove any dependent entries from
+   pbi->mem_set_list.  */
 
 static void
 invalidate_mems_from_set (struct propagate_block_info *pbi, rtx exp)
@@ -2542,7 +2543,12 @@ invalidate_mems_from_set (struct propaga
   while (temp)
     {
       next = XEXP (temp, 1);
-      if (reg_overlap_mentioned_p (exp, XEXP (temp, 0)))
+      if ((REG_P (exp) && reg_overlap_mentioned_p (exp, XEXP (temp, 0)))
+	  /* When we get an EXP that is a mem here, we want to check if EXP
+	     overlaps the *address* of any of the mems in the list (i.e. not
+	     whether the mems actually overlap; that's done elsewhere).  */
+	  || (MEM_P (exp)
+	      && reg_overlap_mentioned_p (exp, XEXP (XEXP (temp, 0), 0))))
 	{
 	  /* Splice this entry out of the list.  */
 	  if (prev)
@@ -2748,11 +2754,12 @@ mark_set_1 (struct propagate_block_info 
       break;
     }
 
-  /* If this set is a MEM, then it kills any aliased writes.
+  /* If this set is a MEM, then it kills any aliased writes and any
+     other MEMs which use it.
      If this set is a REG, then it kills any MEMs which use the reg.  */
   if (optimize && (flags & PROP_SCAN_DEAD_STORES))
     {
-      if (REG_P (reg))
+      if (REG_P (reg) || MEM_P (reg))
 	invalidate_mems_from_set (pbi, reg);
 
       /* If the memory reference had embedded side effects (autoincrement

brgds, H-P


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