[17/50] df-problems.c:find_memory
diff mbox

Message ID 87wqapacyp.fsf@googlemail.com
State New
Headers show

Commit Message

Richard Sandiford Aug. 3, 2014, 2:02 p.m. UTC
This also fixes what I think is a bug: find_memory used to stop at the
first MEM it found.  If that MEM was nonvolatile and nonconstant, we'd
return MEMREF_NORMAL even if there was another volatile MEM.


gcc/
	* df-problems.c: Include rtl-iter.h.
	(find_memory): Turn from being a for_each_rtx callback to being
	a function that examines each subrtx itself.  Continue to look for
	volatile references even after a nonvolatile one has been found.
	(can_move_insns_across): Update calls accordingly.

Comments

Jeff Law Aug. 5, 2014, 9:29 p.m. UTC | #1
On 08/03/14 08:02, Richard Sandiford wrote:
> This also fixes what I think is a bug: find_memory used to stop at the
> first MEM it found.  If that MEM was nonvolatile and nonconstant, we'd
> return MEMREF_NORMAL even if there was another volatile MEM.
>
>
> gcc/
> 	* df-problems.c: Include rtl-iter.h.
> 	(find_memory): Turn from being a for_each_rtx callback to being
> 	a function that examines each subrtx itself.  Continue to look for
> 	volatile references even after a nonvolatile one has been found.
> 	(can_move_insns_across): Update calls accordingly.
OK.

It'd probably be fairly difficult to test for that bug as most of our 
targets don't allow multiple memory operands in a single insn.  But I 
agree with your assessment.  Good catch.

jeff
Richard Earnshaw Aug. 6, 2014, 8:35 a.m. UTC | #2
On 05/08/14 22:29, Jeff Law wrote:
> On 08/03/14 08:02, Richard Sandiford wrote:
>> This also fixes what I think is a bug: find_memory used to stop at the
>> first MEM it found.  If that MEM was nonvolatile and nonconstant, we'd
>> return MEMREF_NORMAL even if there was another volatile MEM.
>>
>>
>> gcc/
>> 	* df-problems.c: Include rtl-iter.h.
>> 	(find_memory): Turn from being a for_each_rtx callback to being
>> 	a function that examines each subrtx itself.  Continue to look for
>> 	volatile references even after a nonvolatile one has been found.
>> 	(can_move_insns_across): Update calls accordingly.
> OK.
> 
> It'd probably be fairly difficult to test for that bug as most of our 
> targets don't allow multiple memory operands in a single insn.  But I 
> agree with your assessment.  Good catch.
> 
> jeff
> 
> 

ARM (and AArch64) have patterns with multiple MEMs; but the mems have to
be related addresses and (I think) be non-volatile (certainly if
volatile is permitted in one MEM it must also be in the others within
the pattern).  Patterns generally enforce all of this through the
pattern itself, the constraints or the condition on the insn.

R.

Patch
diff mbox

Index: gcc/df-problems.c
===================================================================
--- gcc/df-problems.c	2014-08-03 11:25:10.163956663 +0100
+++ gcc/df-problems.c	2014-08-03 11:25:24.920102551 +0100
@@ -44,6 +44,7 @@  Software Foundation; either version 3, o
 #include "dce.h"
 #include "valtrack.h"
 #include "dumpfile.h"
+#include "rtl-iter.h"
 
 /* Note that turning REG_DEAD_DEBUGGING on will cause
    gcc.c-torture/unsorted/dump-noaddr.c to fail because it prints
@@ -3584,25 +3585,27 @@  df_simulate_one_insn_forwards (basic_blo
 #define MEMREF_NORMAL 1
 #define MEMREF_VOLATILE 2
 
-/* A subroutine of can_move_insns_across_p called through for_each_rtx.
-   Return either MEMREF_NORMAL or MEMREF_VOLATILE if a memory is found.  */
+/* Return an OR of MEMREF_NORMAL or MEMREF_VOLATILE for the MEMs in X.  */
 
 static int
-find_memory (rtx *px, void *data ATTRIBUTE_UNUSED)
+find_memory (rtx insn)
 {
-  rtx x = *px;
-
-  if (GET_CODE (x) == ASM_OPERANDS && MEM_VOLATILE_P (x))
-    return MEMREF_VOLATILE;
-
-  if (!MEM_P (x))
-    return 0;
-  if (MEM_VOLATILE_P (x))
-    return MEMREF_VOLATILE;
-  if (MEM_READONLY_P (x))
-    return 0;
-
-  return MEMREF_NORMAL;
+  int flags = 0;
+  subrtx_iterator::array_type array;
+  FOR_EACH_SUBRTX (iter, array, PATTERN (insn), NONCONST)
+    {
+      const_rtx x = *iter;
+      if (GET_CODE (x) == ASM_OPERANDS && MEM_VOLATILE_P (x))
+	flags |= MEMREF_VOLATILE;
+      else if (MEM_P (x))
+	{
+	  if (MEM_VOLATILE_P (x))
+	    flags |= MEMREF_VOLATILE;
+	  else if (!MEM_READONLY_P (x))
+	    flags |= MEMREF_NORMAL;
+	}
+    }
+  return flags;
 }
 
 /* A subroutine of can_move_insns_across_p called through note_stores.
@@ -3705,8 +3708,7 @@  can_move_insns_across (rtx from, rtx to,
 	{
 	  if (volatile_insn_p (PATTERN (insn)))
 	    return false;
-	  memrefs_in_across |= for_each_rtx (&PATTERN (insn), find_memory,
-					     NULL);
+	  memrefs_in_across |= find_memory (insn);
 	  note_stores (PATTERN (insn), find_memory_stores,
 		       &mem_sets_in_across);
 	  /* This is used just to find sets of the stack pointer.  */
@@ -3788,8 +3790,7 @@  can_move_insns_across (rtx from, rtx to,
 	      int mem_ref_flags = 0;
 	      int mem_set_flags = 0;
 	      note_stores (PATTERN (insn), find_memory_stores, &mem_set_flags);
-	      mem_ref_flags = for_each_rtx (&PATTERN (insn), find_memory,
-					    NULL);
+	      mem_ref_flags = find_memory (insn);
 	      /* Catch sets of the stack pointer.  */
 	      mem_ref_flags |= mem_set_flags;