Re: [PATCH 17/50] df-problems.c:find_memory

2014-08-06 Thread Richard Earnshaw 
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.

Re: [PATCH 17/50] df-problems.c:find_memory

2014-08-05 Thread Jeff Law 

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

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

2014-08-03 Thread Richard Sandiford 
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.

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;