[PATCH 0/50] Faster for_each_rtx-like iterators

[PATCH 0/50] Faster for_each_rtx-like iterators

[Richard Sandiford]

In May I posted an RFC about a worklist-based replacement for for_each_rtx:
https://gcc.gnu.org/ml/gcc-patches/2014-05/msg00435.html (see there for
rationale).

This series is the first part of the process.  It adds the new iterators
and converts all for_each_rtx users in generic code.  If these patches
are OK, the remaining changes would be:

(1) do the same for config/
(2) get rid of for_each_rtx
(3) (optionally) convert recursive format walkers to use the new
    iterators too, if that makes things better

I have local patches for some of (3) but they're a lower priority than
(1) or (2).

I think the only changes I made since last time are to address the
feedback from the RFC.

Bootstrapped & regression-tested on x86_64-linux-gnu.  Also tested by
making sure there were no assembly differences for gcc.dg, g++.dg and
gcc.c-torture for:

  x86_64-linux-gnu
  i686-linux-gnu
  sh64-linux-gnu
  powerpc64-linux-gnu
  mips64-linux-gnu
  hppa64-hp-hpux11.23
  aarch64-linux-gnueabi
  arm-linux-gnueabi

which includes auto-inc/dec targets and a user of bt-load.c (sh64-elf).

Many of the patches are pretty mechanical conversions to FOR_EACH_SUBRTX.
I've only added a covering note if there's something unusual going on.

[PATCH 01/50] Add rtl-iter.h

[Richard Sandiford]

This patch adds the new iterators.


gcc/
	* rtl-iter.h: New file.
	* rtlanal.c: Include it.
	(rtx_all_subrtx_bounds, rtx_nonconst_subrtx_bounds): New variables.
	(generic_subrtx_iterator <T>::add_single_to_queue)
	(generic_subrtx_iterator <T>::add_subrtxes_to_queue)
	(generic_subrtx_iterator <T>::free_array): New functions.
	(generic_subrtx_iterator <T>::LOCAL_ELEMS): Define.
	(generic_subrtx_iterator <const_rtx_accessor>)
	(generic_subrtx_iterator <rtx_var_accessor>
	(generic_subrtx_iterator <rtx_ptr_accessor>): Instantiate.
	(setup_reg_subrtx_bounds): New function.
	(init_rtlanal): Call it.

Index: gcc/rtl-iter.h
===================================================================
--- /dev/null	2014-08-02 10:04:40.408500658 +0100
+++ gcc/rtl-iter.h	2014-08-03 11:25:20.308056954 +0100
@@ -0,0 +1,291 @@
+/* RTL iterators
+   Copyright (C) 2014 Free Software Foundation, Inc.
+
+This file is part of GCC.
+
+GCC is free software; you can redistribute it and/or modify it under
+the terms of the GNU General Public License as published by the Free
+Software Foundation; either version 3, or (at your option) any later
+version.
+
+GCC is distributed in the hope that it will be useful, but WITHOUT ANY
+WARRANTY; without even the implied warranty of MERCHANTABILITY or
+FITNESS FOR A PARTICULAR PURPOSE.  See the GNU General Public License
+for more details.
+
+You should have received a copy of the GNU General Public License
+along with GCC; see the file COPYING3.  If not see
+<http://www.gnu.org/licenses/>.  */
+
+/* This structure describes the subrtxes of an rtx as follows:
+
+   - if the rtx has no subrtxes, START and COUNT are both 0.
+
+   - if all the subrtxes of an rtx are stored in a contiguous block
+     of XEXPs ("e"s), START is the index of the first XEXP and COUNT
+     is the number of them.
+
+   - otherwise START is arbitrary and COUNT is UCHAR_MAX.
+
+   rtx_all_subrtx_bounds applies to all codes.  rtx_nonconst_subrtx_bounds
+   is like rtx_all_subrtx_bounds except that all constant rtxes are treated
+   as having no subrtxes.  */
+struct rtx_subrtx_bound_info {
+  unsigned char start;
+  unsigned char count;
+};
+extern rtx_subrtx_bound_info rtx_all_subrtx_bounds[];
+extern rtx_subrtx_bound_info rtx_nonconst_subrtx_bounds[];
+
+/* Return true if CODE has no subrtxes.  */
+
+static inline bool
+leaf_code_p (enum rtx_code code)
+{
+  return rtx_all_subrtx_bounds[code].count == 0;
+}
+
+/* Used to iterate over subrtxes of an rtx.  T abstracts the type of
+   access.  */
+template <typename T>
+class generic_subrtx_iterator
+{
+  static const size_t LOCAL_ELEMS = 16;
+  typedef typename T::value_type value_type;
+  typedef typename T::rtx_type rtx_type;
+  typedef typename T::rtunion_type rtunion_type;
+
+public:
+  struct array_type
+  {
+    array_type ();
+    ~array_type ();
+    value_type stack[LOCAL_ELEMS];
+    vec <value_type, va_heap, vl_embed> *heap;
+  };
+  generic_subrtx_iterator (array_type &, value_type,
+			   const rtx_subrtx_bound_info *);
+
+  value_type operator * () const;
+  bool at_end () const;
+  void next ();
+  void skip_subrtxes ();
+  void substitute (value_type);
+
+private:
+  /* The bounds to use for iterating over subrtxes.  */
+  const rtx_subrtx_bound_info *m_bounds;
+
+  /* The storage used for the worklist.  */
+  array_type &m_array;
+
+  /* The current rtx.  */
+  value_type m_current;
+
+  /* The base of the current worklist.  */
+  value_type *m_base;
+
+  /* The number of subrtxes in M_BASE.  */
+  size_t m_end;
+
+  /* The following booleans shouldn't end up in registers or memory
+     but just direct control flow.  */
+
+  /* True if the iteration is over.  */
+  bool m_done;
+
+  /* True if we should skip the subrtxes of M_CURRENT.  */
+  bool m_skip;
+
+  /* True if M_CURRENT has been replaced with a different rtx.  */
+  bool m_substitute;
+
+  static void free_array (array_type &);
+  static size_t add_subrtxes_to_queue (array_type &, value_type *, size_t,
+				       rtx_type);
+  static value_type *add_single_to_queue (array_type &, value_type *, size_t,
+					  value_type);
+};
+
+template <typename T>
+inline generic_subrtx_iterator <T>::array_type::array_type () : heap (0) {}
+
+template <typename T>
+inline generic_subrtx_iterator <T>::array_type::~array_type ()
+{
+  if (__builtin_expect (heap != 0, false))
+    free_array (*this);
+}
+
+/* Iterate over X and its subrtxes, in arbitrary order.  Use ARRAY to
+   store the worklist.  We use an external array in order to avoid
+   capturing the fields of this structure when taking the address of
+   the array.  Use BOUNDS to find the bounds of simple "e"-string codes.  */
+
+template <typename T>
+inline generic_subrtx_iterator <T>::
+generic_subrtx_iterator (array_type &array, value_type x,
+			 const rtx_subrtx_bound_info *bounds)
+  : m_bounds (bounds),
+    m_array (array),
+    m_current (x),
+    m_base (m_array.stack),
+    m_end (0),
+    m_done (false),
+    m_skip (false),
+    m_substitute (false)
+{
+}
+
+/* Return the current subrtx.  */
+
+template <typename T>
+inline typename T::value_type
+generic_subrtx_iterator <T>::operator * () const
+{
+  return m_current;
+}
+
+/* Return true if the iteration has finished.  */
+
+template <typename T>
+inline bool
+generic_subrtx_iterator <T>::at_end () const
+{
+  return m_done;
+}
+
+/* Move on to the next subrtx.  */
+
+template <typename T>
+inline void
+generic_subrtx_iterator <T>::next ()
+{
+  if (m_substitute)
+    {
+      m_substitute = false;
+      m_skip = false;
+      return;
+    }
+  if (!m_skip)
+    {
+      /* Add the subrtxes of M_CURRENT.  */
+      rtx_type x = T::get_rtx (m_current);
+      if (__builtin_expect (x != 0, true))
+	{
+	  enum rtx_code code = GET_CODE (x);
+	  ssize_t count = m_bounds[code].count;
+	  if (count > 0)
+	    {
+	      /* Handle the simple case of a single "e" block that is known
+		 to fit into the current array.  */
+	      if (__builtin_expect (m_end + count <= LOCAL_ELEMS + 1, true))
+		{
+		  /* Set M_CURRENT to the first subrtx and queue the rest.  */
+		  ssize_t start = m_bounds[code].start;
+		  rtunion_type *src = &x->u.fld[start];
+		  if (__builtin_expect (count > 2, false))
+		    m_base[m_end++] = T::get_value (src[2].rt_rtx);
+		  if (count > 1)
+		    m_base[m_end++] = T::get_value (src[1].rt_rtx);
+		  m_current = T::get_value (src[0].rt_rtx);
+		  return;
+		}
+	      /* Handle cases which aren't simple "e" sequences or where
+		 the sequence might overrun M_BASE.  */
+	      count = add_subrtxes_to_queue (m_array, m_base, m_end, x);
+	      if (count > 0)
+		{
+		  m_end += count;
+		  if (m_end > LOCAL_ELEMS)
+		    m_base = m_array.heap->address ();
+		  m_current = m_base[--m_end];
+		  return;
+		}
+	    }
+	}
+    }
+  else
+    m_skip = false;
+  if (m_end == 0)
+    m_done = true;
+  else
+    m_current = m_base[--m_end];
+}
+
+/* Skip the subrtxes of the current rtx.  */
+
+template <typename T>
+inline void
+generic_subrtx_iterator <T>::skip_subrtxes ()
+{
+  m_skip = true;
+}
+
+/* Ignore the subrtxes of the current rtx and look at X instead.  */
+
+template <typename T>
+inline void
+generic_subrtx_iterator <T>::substitute (value_type x)
+{
+  m_substitute = true;
+  m_current = x;
+}
+
+/* Iterators for const_rtx.  */
+struct const_rtx_accessor
+{
+  typedef const_rtx value_type;
+  typedef const_rtx rtx_type;
+  typedef const rtunion rtunion_type;
+  static rtx_type get_rtx (value_type x) { return x; }
+  static value_type get_value (rtx_type x) { return x; }
+};
+typedef generic_subrtx_iterator <const_rtx_accessor> subrtx_iterator;
+
+/* Iterators for non-constant rtx.  */
+struct rtx_var_accessor
+{
+  typedef rtx value_type;
+  typedef rtx rtx_type;
+  typedef rtunion rtunion_type;
+  static rtx_type get_rtx (value_type x) { return x; }
+  static value_type get_value (rtx_type x) { return x; }
+};
+typedef generic_subrtx_iterator <rtx_var_accessor> subrtx_var_iterator;
+
+/* Iterators for rtx *.  */
+struct rtx_ptr_accessor
+{
+  typedef rtx *value_type;
+  typedef rtx rtx_type;
+  typedef rtunion rtunion_type;
+  static rtx_type get_rtx (value_type ptr) { return *ptr; }
+  static value_type get_value (rtx_type &x) { return &x; }
+};
+typedef generic_subrtx_iterator <rtx_ptr_accessor> subrtx_ptr_iterator;
+
+#define ALL_BOUNDS rtx_all_subrtx_bounds
+#define NONCONST_BOUNDS rtx_nonconst_subrtx_bounds
+
+/* Use ITER to iterate over const_rtx X and its recursive subrtxes,
+   using subrtx_iterator::array ARRAY as the storage for the worklist.
+   ARRAY can be reused for multiple consecutive iterations but shouldn't
+   be shared by two concurrent iterations.  TYPE is ALL if all subrtxes
+   are of interest or NONCONST if it is safe to ignore subrtxes of
+   constants.  */
+#define FOR_EACH_SUBRTX(ITER, ARRAY, X, TYPE) \
+  for (subrtx_iterator ITER (ARRAY, X, TYPE##_BOUNDS); !ITER.at_end (); \
+       ITER.next ())
+
+/* Like FOR_EACH_SUBRTX, but iterate over subrtxes of an rtx X.  */
+#define FOR_EACH_SUBRTX_VAR(ITER, ARRAY, X, TYPE) \
+  for (subrtx_var_iterator ITER (ARRAY, X, TYPE##_BOUNDS); !ITER.at_end (); \
+       ITER.next ())
+
+/* Like FOR_EACH_SUBRTX, but iterate over subrtx pointers of rtx pointer X.
+   For example, if X is &PATTERN (insn) and the pattern is a SET, iterate
+   over &PATTERN (insn), &SET_DEST (PATTERN (insn)), etc.  */
+#define FOR_EACH_SUBRTX_PTR(ITER, ARRAY, X, TYPE) \
+  for (subrtx_ptr_iterator ITER (ARRAY, X, TYPE##_BOUNDS); !ITER.at_end (); \
+       ITER.next ())
Index: gcc/rtlanal.c
===================================================================
--- gcc/rtlanal.c	2014-08-03 11:25:10.713962101 +0100
+++ gcc/rtlanal.c	2014-08-03 11:25:20.309056964 +0100
@@ -37,6 +37,7 @@ Software Foundation; either version 3, o
 #include "tree.h"
 #include "emit-rtl.h"  /* FIXME: Can go away once crtl is moved to rtl.h.  */
 #include "addresses.h"
+#include "rtl-iter.h"
 
 /* Forward declarations */
 static void set_of_1 (rtx, const_rtx, void *);
@@ -62,6 +63,9 @@ static unsigned int num_sign_bit_copies1
    -1 if a code has no such operand.  */
 static int non_rtx_starting_operands[NUM_RTX_CODE];
 
+rtx_subrtx_bound_info rtx_all_subrtx_bounds[NUM_RTX_CODE];
+rtx_subrtx_bound_info rtx_nonconst_subrtx_bounds[NUM_RTX_CODE];
+
 /* Truncation narrows the mode from SOURCE mode to DESTINATION mode.
    If TARGET_MODE_REP_EXTENDED (DESTINATION, DESTINATION_REP) is
    SIGN_EXTEND then while narrowing we also have to enforce the
@@ -78,6 +82,93 @@ static unsigned int num_sign_bit_copies1
 static unsigned int
 num_sign_bit_copies_in_rep[MAX_MODE_INT + 1][MAX_MODE_INT + 1];
 \f
+/* Store X into index I of ARRAY.  ARRAY is known to have at least I
+   elements.  Return the new base of ARRAY.  */
+
+template <typename T>
+typename T::value_type *
+generic_subrtx_iterator <T>::add_single_to_queue (array_type &array,
+						  value_type *base,
+						  size_t i, value_type x)
+{
+  if (base == array.stack)
+    {
+      if (i < LOCAL_ELEMS)
+	{
+	  base[i] = x;
+	  return base;
+	}
+      gcc_checking_assert (i == LOCAL_ELEMS);
+      vec_safe_grow (array.heap, i + 1);
+      base = array.heap->address ();
+      memcpy (base, array.stack, sizeof (array.stack));
+      base[LOCAL_ELEMS] = x;
+      return base;
+    }
+  unsigned int length = array.heap->length ();
+  if (length > i)
+    {
+      gcc_checking_assert (base == array.heap->address ());
+      base[i] = x;
+      return base;
+    }
+  else
+    {
+      gcc_checking_assert (i == length);
+      vec_safe_push (array.heap, x);
+      return array.heap->address ();
+    }
+}
+
+/* Add the subrtxes of X to worklist ARRAY, starting at END.  Return the
+   number of elements added to the worklist.  */
+
+template <typename T>
+size_t
+generic_subrtx_iterator <T>::add_subrtxes_to_queue (array_type &array,
+						    value_type *base,
+						    size_t end, rtx_type x)
+{
+  const char *format = GET_RTX_FORMAT (GET_CODE (x));
+  size_t orig_end = end;
+  for (int i = 0; format[i]; ++i)
+    if (format[i] == 'e')
+      {
+	value_type subx = T::get_value (x->u.fld[i].rt_rtx);
+	if (__builtin_expect (end < LOCAL_ELEMS, true))
+	  base[end++] = subx;
+	else
+	  base = add_single_to_queue (array, base, end++, subx);
+      }
+    else if (format[i] == 'E')
+      {
+	int length = GET_NUM_ELEM (x->u.fld[i].rt_rtvec);
+	rtx *vec = x->u.fld[i].rt_rtvec->elem;
+	if (__builtin_expect (end + length <= LOCAL_ELEMS, true))
+	  for (int j = 0; j < length; j++)
+	    base[end++] = T::get_value (vec[j]);
+	else
+	  for (int j = 0; j < length; j++)
+	    base = add_single_to_queue (array, base, end++,
+					T::get_value (vec[j]));
+      }
+  return end - orig_end;
+}
+
+template <typename T>
+void
+generic_subrtx_iterator <T>::free_array (array_type &array)
+{
+  vec_free (array.heap);
+}
+
+template <typename T>
+const size_t generic_subrtx_iterator <T>::LOCAL_ELEMS;
+
+template class generic_subrtx_iterator <const_rtx_accessor>;
+template class generic_subrtx_iterator <rtx_var_accessor>;
+template class generic_subrtx_iterator <rtx_ptr_accessor>;
+
 /* Return 1 if the value of X is unstable
    (would be different at a different point in the program).
    The frame pointer, arg pointer, etc. are considered stable
@@ -5328,8 +5419,42 @@ truncated_to_mode (enum machine_mode mod
   return false;
 }
 \f
+/* Return true if RTX code CODE has a single sequence of zero or more
+   "e" operands and no rtvec operands.  Initialize its rtx_all_subrtx_bounds
+   entry in that case.  */
+
+static bool
+setup_reg_subrtx_bounds (unsigned int code)
+{
+  const char *format = GET_RTX_FORMAT ((enum rtx_code) code);
+  unsigned int i = 0;
+  for (; format[i] != 'e'; ++i)
+    {
+      if (!format[i])
+	/* No subrtxes.  Leave start and count as 0.  */
+	return true;
+      if (format[i] == 'E' || format[i] == 'V')
+	return false;
+    }
+
+  /* Record the sequence of 'e's.  */
+  rtx_all_subrtx_bounds[code].start = i;
+  do
+    ++i;
+  while (format[i] == 'e');
+  rtx_all_subrtx_bounds[code].count = i - rtx_all_subrtx_bounds[code].start;
+  /* rtl-iter.h relies on this.  */
+  gcc_checking_assert (rtx_all_subrtx_bounds[code].count <= 3);
+
+  for (; format[i]; ++i)
+    if (format[i] == 'E' || format[i] == 'V' || format[i] == 'e')
+      return false;
+
+  return true;
+}
+
 /* Initialize non_rtx_starting_operands, which is used to speed up
-   for_each_rtx.  */
+   for_each_rtx, and rtx_all_subrtx_bounds.  */
 void
 init_rtlanal (void)
 {
@@ -5339,6 +5464,10 @@ init_rtlanal (void)
       const char *format = GET_RTX_FORMAT (i);
       const char *first = strpbrk (format, "eEV");
       non_rtx_starting_operands[i] = first ? first - format : -1;
+      if (!setup_reg_subrtx_bounds (i))
+	rtx_all_subrtx_bounds[i].count = UCHAR_MAX;
+      if (GET_RTX_CLASS (i) != RTX_CONST_OBJ)
+	rtx_nonconst_subrtx_bounds[i] = rtx_all_subrtx_bounds[i];
     }
 
   init_num_sign_bit_copies_in_rep ();

Re: [PATCH 01/50] Add rtl-iter.h

[Jeff Law]

On 08/03/14 07:39, Richard Sandiford wrote:
> This patch adds the new iterators.
>
>
> gcc/
> 	* rtl-iter.h: New file.
> 	* rtlanal.c: Include it.
> 	(rtx_all_subrtx_bounds, rtx_nonconst_subrtx_bounds): New variables.
> 	(generic_subrtx_iterator <T>::add_single_to_queue)
> 	(generic_subrtx_iterator <T>::add_subrtxes_to_queue)
> 	(generic_subrtx_iterator <T>::free_array): New functions.
> 	(generic_subrtx_iterator <T>::LOCAL_ELEMS): Define.
> 	(generic_subrtx_iterator <const_rtx_accessor>)
> 	(generic_subrtx_iterator <rtx_var_accessor>
> 	(generic_subrtx_iterator <rtx_ptr_accessor>): Instantiate.
> 	(setup_reg_subrtx_bounds): New function.
> 	(init_rtlanal): Call it.
OK.  Just one nit...



> +
> +/* This structure describes the subrtxes of an rtx as follows:
> +
> +   - if the rtx has no subrtxes, START and COUNT are both 0.
Seems reasonable.


> +static inline bool
> +leaf_code_p (enum rtx_code code)
> +{
> +  return rtx_all_subrtx_bounds[code].count == 0;
> +}
But we only check COUNT here.

It's a minor inconsistency.  Your call what (if anything) to do about it.

Jeff

Re: [PATCH 01/50] Add rtl-iter.h

[Richard Henderson]

On 08/03/2014 03:39 AM, Richard Sandiford wrote:
> +struct rtx_subrtx_bound_info {
> +  unsigned char start;
> +  unsigned char count;
> +};

Given this structure is only two bytes...

> +  /* The bounds to use for iterating over subrtxes.  */
> +  const rtx_subrtx_bound_info *m_bounds;

... wouldn't it be better to pass by value instead of by reference?


r~

Re: [PATCH 01/50] Add rtl-iter.h

[Richard Sandiford]

Richard Henderson <rth@redhat.com> writes:
> On 08/03/2014 03:39 AM, Richard Sandiford wrote:
>> +struct rtx_subrtx_bound_info {
>> +  unsigned char start;
>> +  unsigned char count;
>> +};
>
> Given this structure is only two bytes...
>
>> +  /* The bounds to use for iterating over subrtxes.  */
>> +  const rtx_subrtx_bound_info *m_bounds;
>
> ... wouldn't it be better to pass by value instead of by reference?

This shows I should expand the comment, but m_bounds is an array indexed
by rtx code.

Thanks,
Richard

[PATCH 02/50] alias.c:refs_newer_value_p

[Richard Sandiford]

gcc/
	* alias.c: Include rtl-iter.h.
	(refs_newer_value_cb): Delete.
	(refs_newer_value_p): Use FOR_EACH_SUBRTX instead of for_each_rtx.

Index: gcc/alias.c
===================================================================
--- gcc/alias.c	2014-08-03 11:25:10.551960499 +0100
+++ gcc/alias.c	2014-08-03 11:25:20.611059950 +0100
@@ -48,6 +48,7 @@ Software Foundation; either version 3, o
 #include "is-a.h"
 #include "gimple.h"
 #include "gimple-ssa.h"
+#include "rtl-iter.h"
 
 /* The aliasing API provided here solves related but different problems:
 
@@ -1853,27 +1854,18 @@ base_alias_check (rtx x, rtx x_base, rtx
   return 1;
 }
 
-/* Callback for for_each_rtx, that returns 1 upon encountering a VALUE
-   whose UID is greater than the int uid that D points to.  */
-
-static int
-refs_newer_value_cb (rtx *x, void *d)
-{
-  if (GET_CODE (*x) == VALUE && CSELIB_VAL_PTR (*x)->uid > *(int *)d)
-    return 1;
-
-  return 0;
-}
-
 /* Return TRUE if EXPR refers to a VALUE whose uid is greater than
    that of V.  */
 
 static bool
-refs_newer_value_p (rtx expr, rtx v)
+refs_newer_value_p (const_rtx expr, rtx v)
 {
   int minuid = CSELIB_VAL_PTR (v)->uid;
-
-  return for_each_rtx (&expr, refs_newer_value_cb, &minuid);
+  subrtx_iterator::array_type array;
+  FOR_EACH_SUBRTX (iter, array, expr, NONCONST)
+    if (GET_CODE (*iter) == VALUE && CSELIB_VAL_PTR (*iter)->uid > minuid)
+      return true;
+  return false;
 }
 
 /* Convert the address X into something we can use.  This is done by returning

Re: [PATCH 02/50] alias.c:refs_newer_value_p

[Jeff Law]

On 08/03/14 07:40, Richard Sandiford wrote:
> gcc/
> 	* alias.c: Include rtl-iter.h.
> 	(refs_newer_value_cb): Delete.
> 	(refs_newer_value_p): Use FOR_EACH_SUBRTX instead of for_each_rtx.
OK.

Just a few notes.

1. I really like when we can make these callbacks go away :-)

2. When the callbacks go away, the opaque data we pass them goes away as 
well which is good for readability.

3. That opaque data is often a local and we had to take its address and 
thus force it to memory.  With the direct uses in the updated code those 
locals don't have to live in memory, which is goodness.

Anyway, just wanted to point those out for anyone looking at this in 
parallel or in the future.

jeff

[PATCH 03/50] bt-load.c:find_btr_use

[Richard Sandiford]

With the move away from for_each_rtx and the separate static variable
used to communicate the found reference, there's not really any need
to keep btr_referenced_p and find_btr_use separate.


gcc/
	* bt-load.c: Include rtl-iter.h.
	(btr_reference_found, find_btr_reference, btr_referenced_p): Delete.
	(find_btr_use): Move further up file.  Use FOR_EACH_SUBRTX_PTR
	to iterate over subrtxes.
	(insn_sets_btr_p, new_btr_user, compute_defs_uses_and_gen): Use
	find_btr_use rather than btr_referenced_p.

Index: gcc/bt-load.c
===================================================================
--- gcc/bt-load.c	2014-08-03 11:25:10.533960321 +0100
+++ gcc/bt-load.c	2014-08-03 11:25:20.866062471 +0100
@@ -38,6 +38,7 @@ Software Foundation; either version 3, o
 #include "recog.h"
 #include "df.h"
 #include "cfgloop.h"
+#include "rtl-iter.h"
 
 /* Target register optimizations - these are performed after reload.  */
 
@@ -113,9 +114,6 @@ typedef struct btr_def_s
 
 static int basic_block_freq (const_basic_block);
 static int insn_sets_btr_p (const_rtx, int, int *);
-static rtx *find_btr_use (rtx);
-static int btr_referenced_p (rtx, rtx *);
-static int find_btr_reference (rtx *, void *);
 static void find_btr_def_group (btr_def_group *, btr_def);
 static btr_def add_btr_def (fibheap_t, basic_block, int, rtx,
 			    unsigned int, int, btr_def_group *);
@@ -184,37 +182,27 @@ basic_block_freq (const_basic_block bb)
   return bb->frequency;
 }
 
-static rtx *btr_reference_found;
-
-/* A subroutine of btr_referenced_p, called through for_each_rtx.
-   PREG is a pointer to an rtx that is to be excluded from the
-   traversal.  If we find a reference to a target register anywhere
-   else, return 1, and put a pointer to it into btr_reference_found.  */
-static int
-find_btr_reference (rtx *px, void *preg)
+/* If X references (sets or reads) any branch target register, return one
+   such register.  If EXCLUDEP is set, disregard any references within
+   that location.  */
+static rtx *
+find_btr_use (rtx x, rtx *excludep = 0)
 {
-  rtx x;
-
-  if (px == preg)
-    return -1;
-  x = *px;
-  if (!REG_P (x))
-    return 0;
-  if (overlaps_hard_reg_set_p (all_btrs, GET_MODE (x), REGNO (x)))
+  subrtx_ptr_iterator::array_type array;
+  FOR_EACH_SUBRTX_PTR (iter, array, &x, NONCONST)
     {
-      btr_reference_found = px;
-      return 1;
+      rtx *loc = *iter;
+      if (loc == excludep)
+	iter.skip_subrtxes ();
+      else
+	{
+	  const_rtx x = *loc;
+	  if (REG_P (x)
+	      && overlaps_hard_reg_set_p (all_btrs, GET_MODE (x), REGNO (x)))
+	    return loc;
+	}
     }
-  return -1;
-}
-
-/* Return nonzero if X references (sets or reads) any branch target register.
-   If EXCLUDEP is set, disregard any references within the rtx pointed to
-   by it.  If returning nonzero, also set btr_reference_found as above.  */
-static int
-btr_referenced_p (rtx x, rtx *excludep)
-{
-  return for_each_rtx (&x, find_btr_reference, excludep);
+  return 0;
 }
 
 /* Return true if insn is an instruction that sets a target register.
@@ -238,7 +226,7 @@ insn_sets_btr_p (const_rtx insn, int che
       if (REG_P (dest)
 	  && TEST_HARD_REG_BIT (all_btrs, REGNO (dest)))
 	{
-	  gcc_assert (!btr_referenced_p (src, NULL));
+	  gcc_assert (!find_btr_use (src));
 
 	  if (!check_const || CONSTANT_P (src))
 	    {
@@ -251,13 +239,6 @@ insn_sets_btr_p (const_rtx insn, int che
   return 0;
 }
 
-/* Find and return a use of a target register within an instruction INSN.  */
-static rtx *
-find_btr_use (rtx insn)
-{
-  return btr_referenced_p (insn, NULL) ? btr_reference_found : NULL;
-}
-
 /* Find the group that the target register definition DEF belongs
    to in the list starting with *ALL_BTR_DEF_GROUPS.  If no such
    group exists, create one.  Add def to the group.  */
@@ -347,7 +328,7 @@ new_btr_user (basic_block bb, int insn_l
       /* We want to ensure that USE is the only use of a target
 	 register in INSN, so that we know that to rewrite INSN to use
 	 a different target register, all we have to do is replace USE.  */
-      unambiguous_single_use = !btr_referenced_p (PATTERN (insn), usep);
+      unambiguous_single_use = !find_btr_use (PATTERN (insn), usep);
       if (!unambiguous_single_use)
 	usep = NULL;
     }
@@ -523,7 +504,7 @@ compute_defs_uses_and_gen (fibheap_t all
 		}
 	      else
 		{
-		  if (btr_referenced_p (PATTERN (insn), NULL))
+		  if (find_btr_use (PATTERN (insn)))
 		    {
 		      btr_user user = new_btr_user (bb, insn_luid, insn);
 

Re: [PATCH 03/50] bt-load.c:find_btr_use

[Jeff Law]

On 08/03/14 07:44, Richard Sandiford wrote:
> With the move away from for_each_rtx and the separate static variable
> used to communicate the found reference, there's not really any need
> to keep btr_referenced_p and find_btr_use separate.
>
>
> gcc/
> 	* bt-load.c: Include rtl-iter.h.
> 	(btr_reference_found, find_btr_reference, btr_referenced_p): Delete.
> 	(find_btr_use): Move further up file.  Use FOR_EACH_SUBRTX_PTR
> 	to iterate over subrtxes.
> 	(insn_sets_btr_p, new_btr_user, compute_defs_uses_and_gen): Use
> 	find_btr_use rather than btr_referenced_p.
OK.

Jeff

[PATCH 04/50] caller-save.c:add_used_regs

[Richard Sandiford]

As noted in https://gcc.gnu.org/ml/gcc-patches/2014-02/msg01391.html
a bitmap-related cleanup turned add_used_regs_1 into a no-op for pseudo
registers, because the result of:

  regno = reg_renumber[regno];

is never used.  This patch does as Steven requested and adds an
assert that no allocated pseudos are seen here.


gcc/
	* caller-save.c: Include rtl-iter.h.
	(add_used_regs_1): Delete.
	(add_used_regs): Use FOR_EACH_SUBRTX rather than for_each_rtx
	to iterate over subrtxes.  Assert that any remaining pseudos
	have been spilled.

Index: gcc/caller-save.c
===================================================================
--- gcc/caller-save.c	2014-08-03 11:25:10.514960134 +0100
+++ gcc/caller-save.c	2014-08-03 11:25:21.155065328 +0100
@@ -37,6 +37,7 @@ Software Foundation; either version 3, o
 #include "addresses.h"
 #include "ggc.h"
 #include "dumpfile.h"
+#include "rtl-iter.h"
 
 #define MOVE_MAX_WORDS (MOVE_MAX / UNITS_PER_WORD)
 
@@ -1336,36 +1337,26 @@ insert_save (struct insn_chain *chain, i
   return numregs - 1;
 }
 
-/* A for_each_rtx callback used by add_used_regs.  Add the hard-register
-   equivalent of each REG to regset DATA.  */
-
-static int
-add_used_regs_1 (rtx *loc, void *data)
-{
-  unsigned int regno;
-  regset live;
-  rtx x;
-
-  x = *loc;
-  live = (regset) data;
-  if (REG_P (x))
-    {
-      regno = REGNO (x);
-      if (HARD_REGISTER_NUM_P (regno))
-	bitmap_set_range (live, regno, hard_regno_nregs[regno][GET_MODE (x)]);
-      else
-	regno = reg_renumber[regno];
-    }
-  return 0;
-}
-
 /* A note_uses callback used by insert_one_insn.  Add the hard-register
    equivalent of each REG to regset DATA.  */
 
 static void
 add_used_regs (rtx *loc, void *data)
 {
-  for_each_rtx (loc, add_used_regs_1, data);
+  subrtx_iterator::array_type array;
+  FOR_EACH_SUBRTX (iter, array, *loc, NONCONST)
+    {
+      const_rtx x = *iter;
+      if (REG_P (x))
+	{
+	  unsigned int regno = REGNO (x);
+	  if (HARD_REGISTER_NUM_P (regno))
+	    bitmap_set_range ((regset) data, regno,
+			      hard_regno_nregs[regno][GET_MODE (x)]);
+	  else
+	    gcc_checking_assert (reg_renumber[regno] < 0);
+	}
+    }
 }
 
 /* Emit a new caller-save insn and set the code.  */

Re: [PATCH 04/50] caller-save.c:add_used_regs

[Jeff Law]

On 08/03/14 07:48, Richard Sandiford wrote:
> As noted in https://gcc.gnu.org/ml/gcc-patches/2014-02/msg01391.html
> a bitmap-related cleanup turned add_used_regs_1 into a no-op for pseudo
> registers, because the result of:
>
>    regno = reg_renumber[regno];
>
> is never used.  This patch does as Steven requested and adds an
> assert that no allocated pseudos are seen here.
>
>
> gcc/
> 	* caller-save.c: Include rtl-iter.h.
> 	(add_used_regs_1): Delete.
> 	(add_used_regs): Use FOR_EACH_SUBRTX rather than for_each_rtx
> 	to iterate over subrtxes.  Assert that any remaining pseudos
> 	have been spilled.
OK.
jeff

[PATCH 05/50] calls.c:internal_arg_pointer_based_exp

[Richard Sandiford]

gcc/
	* calls.c: Include rtl-iter.h.
	(internal_arg_pointer_based_exp_1): Delete.
	(internal_arg_pointer_based_exp): Take a const_rtx.
	Use FOR_EACH_SUBRTX to iterate over subrtxes.

Index: gcc/calls.c
===================================================================
--- gcc/calls.c	2014-08-03 11:25:10.496959956 +0100
+++ gcc/calls.c	2014-08-03 11:25:21.433068077 +0100
@@ -49,6 +49,7 @@ Software Foundation; either version 3, o
 #include "cgraph.h"
 #include "except.h"
 #include "dbgcnt.h"
+#include "rtl-iter.h"
 
 /* Like PREFERRED_STACK_BOUNDARY but in units of bytes, not bits.  */
 #define STACK_BYTES (PREFERRED_STACK_BOUNDARY / BITS_PER_UNIT)
@@ -1693,7 +1694,7 @@ rtx_for_function_call (tree fndecl, tree
   vec<rtx> cache;
 } internal_arg_pointer_exp_state;
 
-static rtx internal_arg_pointer_based_exp (rtx, bool);
+static rtx internal_arg_pointer_based_exp (const_rtx, bool);
 
 /* Helper function for internal_arg_pointer_based_exp.  Scan insns in
    the tail call sequence, starting with first insn that hasn't been
@@ -1741,28 +1742,13 @@ internal_arg_pointer_based_exp_scan (voi
   internal_arg_pointer_exp_state.scan_start = scan_start;
 }
 
-/* Helper function for internal_arg_pointer_based_exp, called through
-   for_each_rtx.  Return 1 if *LOC is a register based on
-   crtl->args.internal_arg_pointer.  Return -1 if *LOC is not based on it
-   and the subexpressions need not be examined.  Otherwise return 0.  */
-
-static int
-internal_arg_pointer_based_exp_1 (rtx *loc, void *data ATTRIBUTE_UNUSED)
-{
-  if (REG_P (*loc) && internal_arg_pointer_based_exp (*loc, false) != NULL_RTX)
-    return 1;
-  if (MEM_P (*loc))
-    return -1;
-  return 0;
-}
-
 /* Compute whether RTL is based on crtl->args.internal_arg_pointer.  Return
    NULL_RTX if RTL isn't based on it, a CONST_INT offset if RTL is based on
    it with fixed offset, or PC if this is with variable or unknown offset.
    TOPLEVEL is true if the function is invoked at the topmost level.  */
 
 static rtx
-internal_arg_pointer_based_exp (rtx rtl, bool toplevel)
+internal_arg_pointer_based_exp (const_rtx rtl, bool toplevel)
 {
   if (CONSTANT_P (rtl))
     return NULL_RTX;
@@ -1796,8 +1782,15 @@ internal_arg_pointer_based_exp (rtx rtl,
       return NULL_RTX;
     }
 
-  if (for_each_rtx (&rtl, internal_arg_pointer_based_exp_1, NULL))
-    return pc_rtx;
+  subrtx_iterator::array_type array;
+  FOR_EACH_SUBRTX (iter, array, rtl, NONCONST)
+    {
+      const_rtx x = *iter;
+      if (REG_P (x) && internal_arg_pointer_based_exp (x, false) != NULL_RTX)
+	return pc_rtx;
+      if (MEM_P (x))
+	iter.skip_subrtxes ();
+    }
 
   return NULL_RTX;
 }

Re: [PATCH 05/50] calls.c:internal_arg_pointer_based_exp

[Jeff Law]

On 08/03/14 07:48, Richard Sandiford wrote:
> gcc/
> 	* calls.c: Include rtl-iter.h.
> 	(internal_arg_pointer_based_exp_1): Delete.
> 	(internal_arg_pointer_based_exp): Take a const_rtx.
> 	Use FOR_EACH_SUBRTX to iterate over subrtxes.
OK.
jeff

[PATCH 06/50] combine.c:unmentioned_reg_p

[Richard Sandiford]

gcc/
	* combine.c: Include rtl-iter.h.
	(unmentioned_reg_p_1): Delete.
	(unmentioned_reg_p): Use FOR_EACH_SUBRTX rather than for_each_rtx.
	Don't handle null rtxes.

Index: gcc/combine.c
===================================================================
--- gcc/combine.c	2014-08-03 11:25:10.479959788 +0100
+++ gcc/combine.c	2014-08-03 11:25:21.711070825 +0100
@@ -106,6 +106,7 @@ Software Foundation; either version 3, o
 #include "obstack.h"
 #include "statistics.h"
 #include "params.h"
+#include "rtl-iter.h"
 
 /* Number of attempts to combine instructions in this function.  */
 
@@ -467,7 +468,6 @@ static void distribute_notes (rtx, rtx,
 static void distribute_links (struct insn_link *);
 static void mark_used_regs_combine (rtx);
 static void record_promoted_value (rtx, rtx);
-static int unmentioned_reg_p_1 (rtx *, void *);
 static bool unmentioned_reg_p (rtx, rtx);
 static int record_truncated_value (rtx *, void *);
 static void record_truncated_values (rtx *, void *);
@@ -13835,23 +13835,6 @@ distribute_links (struct insn_link *link
     }
 }
 \f
-/* Subroutine of unmentioned_reg_p and callback from for_each_rtx.
-   Check whether the expression pointer to by LOC is a register or
-   memory, and if so return 1 if it isn't mentioned in the rtx EXPR.
-   Otherwise return zero.  */
-
-static int
-unmentioned_reg_p_1 (rtx *loc, void *expr)
-{
-  rtx x = *loc;
-
-  if (x != NULL_RTX
-      && (REG_P (x) || MEM_P (x))
-      && ! reg_mentioned_p (x, (rtx) expr))
-    return 1;
-  return 0;
-}
-
 /* Check for any register or memory mentioned in EQUIV that is not
    mentioned in EXPR.  This is used to restrict EQUIV to "specializations"
    of EXPR where some registers may have been replaced by constants.  */
@@ -13859,7 +13842,15 @@ unmentioned_reg_p_1 (rtx *loc, void *exp
 static bool
 unmentioned_reg_p (rtx equiv, rtx expr)
 {
-  return for_each_rtx (&equiv, unmentioned_reg_p_1, expr);
+  subrtx_iterator::array_type array;
+  FOR_EACH_SUBRTX (iter, array, equiv, NONCONST)
+    {
+      const_rtx x = *iter;
+      if ((REG_P (x) || MEM_P (x))
+	  && !reg_mentioned_p (x, expr))
+	return true;
+    }
+  return false;
 }
 \f
 DEBUG_FUNCTION void

Re: [PATCH 06/50] combine.c:unmentioned_reg_p

[Jeff Law]

On 08/03/14 07:50, Richard Sandiford wrote:
> gcc/
> 	* combine.c: Include rtl-iter.h.
> 	(unmentioned_reg_p_1): Delete.
> 	(unmentioned_reg_p): Use FOR_EACH_SUBRTX rather than for_each_rtx.
> 	Don't handle null rtxes.
OK.
Jeff

[PATCH 07/50] combine.c:record_truncated_values

[Richard Sandiford]

gcc/
	* combine.c (record_truncated_value): Turn from being a for_each_rtx
	callback to a function that takes an rtx and returns a bool
	(record_truncated_values): Use FOR_EACH_SUBRTX_VAR instead of
	for_each_rtx.

Index: gcc/combine.c
===================================================================
--- gcc/combine.c	2014-08-03 11:25:21.711070825 +0100
+++ gcc/combine.c	2014-08-03 11:25:22.005073732 +0100
@@ -469,7 +469,6 @@ static void distribute_links (struct ins
 static void mark_used_regs_combine (rtx);
 static void record_promoted_value (rtx, rtx);
 static bool unmentioned_reg_p (rtx, rtx);
-static int record_truncated_value (rtx *, void *);
 static void record_truncated_values (rtx *, void *);
 static bool reg_truncated_to_mode (enum machine_mode, const_rtx);
 static rtx gen_lowpart_or_truncate (enum machine_mode, rtx);
@@ -12493,15 +12492,14 @@ reg_truncated_to_mode (enum machine_mode
   return false;
 }
 
-/* Callback for for_each_rtx.  If *P is a hard reg or a subreg record the mode
-   that the register is accessed in.  For non-TRULY_NOOP_TRUNCATION targets we
-   might be able to turn a truncate into a subreg using this information.
-   Return -1 if traversing *P is complete or 0 otherwise.  */
+/* If X is a hard reg or a subreg record the mode that the register is
+   accessed in.  For non-TRULY_NOOP_TRUNCATION targets we might be able
+   to turn a truncate into a subreg using this information.  Return true
+   if traversing X is complete.  */
 
-static int
-record_truncated_value (rtx *p, void *data ATTRIBUTE_UNUSED)
+static bool
+record_truncated_value (rtx x)
 {
-  rtx x = *p;
   enum machine_mode truncated_mode;
   reg_stat_type *rsp;
 
@@ -12511,10 +12509,10 @@ record_truncated_value (rtx *p, void *da
       truncated_mode = GET_MODE (x);
 
       if (GET_MODE_SIZE (original_mode) <= GET_MODE_SIZE (truncated_mode))
-	return -1;
+	return true;
 
       if (TRULY_NOOP_TRUNCATION_MODES_P (truncated_mode, original_mode))
-	return -1;
+	return true;
 
       x = SUBREG_REG (x);
     }
@@ -12523,7 +12521,7 @@ record_truncated_value (rtx *p, void *da
   else if (REG_P (x) && REGNO (x) < FIRST_PSEUDO_REGISTER)
     truncated_mode = GET_MODE (x);
   else
-    return 0;
+    return false;
 
   rsp = &reg_stat[REGNO (x)];
   if (rsp->truncated_to_mode == 0
@@ -12535,7 +12533,7 @@ record_truncated_value (rtx *p, void *da
       rsp->truncation_label = label_tick;
     }
 
-  return -1;
+  return true;
 }
 
 /* Callback for note_uses.  Find hardregs and subregs of pseudos and
@@ -12543,9 +12541,12 @@ record_truncated_value (rtx *p, void *da
    SUBREGs.  */
 
 static void
-record_truncated_values (rtx *x, void *data ATTRIBUTE_UNUSED)
+record_truncated_values (rtx *loc, void *data ATTRIBUTE_UNUSED)
 {
-  for_each_rtx (x, record_truncated_value, NULL);
+  subrtx_var_iterator::array_type array;
+  FOR_EACH_SUBRTX_VAR (iter, array, *loc, NONCONST)
+    if (record_truncated_value (*iter))
+      iter.skip_subrtxes ();
 }
 
 /* Scan X for promoted SUBREGs.  For each one found,

Re: [PATCH 07/50] combine.c:record_truncated_values

[Jeff Law]

On 08/03/14 07:50, Richard Sandiford wrote:
> gcc/
> 	* combine.c (record_truncated_value): Turn from being a for_each_rtx
> 	callback to a function that takes an rtx and returns a bool
> 	(record_truncated_values): Use FOR_EACH_SUBRTX_VAR instead of
> 	for_each_rtx.
OK.
jeff

[PATCH 08/50] Faster for_each_rtx-like iterators

[Richard Sandiford]

gcc/
	* combine-stack-adj.c: Include rtl-iter.h.
	(record_stack_refs_data): Delete.
	(record_stack_refs): Turn from being a for_each_rtx callback
	to being a function that examines each subrtx itself.
	Take a pointer to the reflist.  Invert sense of return value
	so that true means success and false means failure.  Don't
	handle null rtxes.
	(combine_stack_adjustments_for_block): Update accordingly.

Index: gcc/combine-stack-adj.c
===================================================================
--- gcc/combine-stack-adj.c	2014-08-03 11:25:10.425959254 +0100
+++ gcc/combine-stack-adj.c	2014-08-03 11:25:22.308076727 +0100
@@ -56,6 +56,7 @@ Software Foundation; either version 3, o
 #include "except.h"
 #include "reload.h"
 #include "tree-pass.h"
+#include "rtl-iter.h"
 
 \f
 /* Turn STACK_GROWS_DOWNWARD into a boolean.  */
@@ -85,7 +86,6 @@ static struct csa_reflist *record_one_st
 static int try_apply_stack_adjustment (rtx, struct csa_reflist *,
 				       HOST_WIDE_INT, HOST_WIDE_INT);
 static void combine_stack_adjustments_for_block (basic_block);
-static int record_stack_refs (rtx *, void *);
 
 
 /* Main entry point for stack adjustment combination.  */
@@ -236,61 +236,63 @@ try_apply_stack_adjustment (rtx insn, st
     return 0;
 }
 
-/* Called via for_each_rtx and used to record all stack memory and other
-   references in the insn and discard all other stack pointer references.  */
-struct record_stack_refs_data
-{
-  rtx insn;
-  struct csa_reflist *reflist;
-};
-
-static int
-record_stack_refs (rtx *xp, void *data)
-{
-  rtx x = *xp;
-  struct record_stack_refs_data *d =
-    (struct record_stack_refs_data *) data;
-  if (!x)
-    return 0;
-  switch (GET_CODE (x))
-    {
-    case MEM:
-      if (!reg_mentioned_p (stack_pointer_rtx, x))
-	return -1;
-      /* We are not able to handle correctly all possible memrefs containing
-         stack pointer, so this check is necessary.  */
-      if (stack_memref_p (x))
-	{
-	  d->reflist = record_one_stack_ref (d->insn, xp, d->reflist);
-	  return -1;
-	}
-      /* Try harder for DEBUG_INSNs, handle e.g. (mem (mem (sp + 16) + 4).  */
-      return !DEBUG_INSN_P (d->insn);
-    case REG:
-      /* ??? We want be able to handle non-memory stack pointer
-	 references later.  For now just discard all insns referring to
-	 stack pointer outside mem expressions.  We would probably
-	 want to teach validate_replace to simplify expressions first.
-
-	 We can't just compare with STACK_POINTER_RTX because the
-	 reference to the stack pointer might be in some other mode.
-	 In particular, an explicit clobber in an asm statement will
-	 result in a QImode clobber.
-
-	 In DEBUG_INSNs, we want to replace all occurrences, otherwise
-	 they will cause -fcompare-debug failures.  */
-      if (REGNO (x) == STACK_POINTER_REGNUM)
+/* For non-debug insns, record all stack memory references in INSN
+   and return true if there were no other (unrecorded) references to the
+   stack pointer.  For debug insns, record all stack references regardless
+   of context and unconditionally return true.  */
+
+static bool
+record_stack_refs (rtx insn, struct csa_reflist **reflist)
+{
+  subrtx_ptr_iterator::array_type array;
+  FOR_EACH_SUBRTX_PTR (iter, array, &PATTERN (insn), NONCONST)
+    {
+      rtx *loc = *iter;
+      rtx x = *loc;
+      switch (GET_CODE (x))
 	{
-	  if (!DEBUG_INSN_P (d->insn))
-	    return 1;
-	  d->reflist = record_one_stack_ref (d->insn, xp, d->reflist);
-	  return -1;
+	case MEM:
+	  if (!reg_mentioned_p (stack_pointer_rtx, x))
+	    iter.skip_subrtxes ();
+	  /* We are not able to handle correctly all possible memrefs
+	     containing stack pointer, so this check is necessary.  */
+	  else if (stack_memref_p (x))
+	    {
+	      *reflist = record_one_stack_ref (insn, loc, *reflist);
+	      iter.skip_subrtxes ();
+	    }
+	  /* Try harder for DEBUG_INSNs, handle e.g.
+	     (mem (mem (sp + 16) + 4).  */
+	  else if (!DEBUG_INSN_P (insn))
+	    return false;
+	  break;
+
+	case REG:
+	  /* ??? We want be able to handle non-memory stack pointer
+	     references later.  For now just discard all insns referring to
+	     stack pointer outside mem expressions.  We would probably
+	     want to teach validate_replace to simplify expressions first.
+
+	     We can't just compare with STACK_POINTER_RTX because the
+	     reference to the stack pointer might be in some other mode.
+	     In particular, an explicit clobber in an asm statement will
+	     result in a QImode clobber.
+
+	     In DEBUG_INSNs, we want to replace all occurrences, otherwise
+	     they will cause -fcompare-debug failures.  */
+	  if (REGNO (x) == STACK_POINTER_REGNUM)
+	    {
+	      if (!DEBUG_INSN_P (insn))
+		return false;
+	      *reflist = record_one_stack_ref (insn, loc, *reflist);
+	    }
+	  break;
+
+	default:
+	  break;
 	}
-      break;
-    default:
-      break;
     }
-  return 0;
+  return true;
 }
 
 /* If INSN has a REG_ARGS_SIZE note, move it to LAST.
@@ -429,7 +431,6 @@ combine_stack_adjustments_for_block (bas
   rtx last2_sp_set = NULL_RTX;
   struct csa_reflist *reflist = NULL;
   rtx insn, next, set;
-  struct record_stack_refs_data data;
   bool end_of_block = false;
 
   for (insn = BB_HEAD (bb); !end_of_block ; insn = next)
@@ -580,15 +581,9 @@ combine_stack_adjustments_for_block (bas
 	    }
 	}
 
-      data.insn = insn;
-      data.reflist = reflist;
       if (!CALL_P (insn) && last_sp_set
-	  && !for_each_rtx (&PATTERN (insn), record_stack_refs, &data))
-	{
-	   reflist = data.reflist;
-	   continue;
-	}
-      reflist = data.reflist;
+	  && record_stack_refs (insn, &reflist))
+	continue;
 
       /* Otherwise, we were not able to process the instruction.
 	 Do not continue collecting data across such a one.  */

Re: [PATCH 08/50] Faster for_each_rtx-like iterators

[Jeff Law]

On 08/03/14 07:52, Richard Sandiford wrote:
> gcc/
> 	* combine-stack-adj.c: Include rtl-iter.h.
> 	(record_stack_refs_data): Delete.
> 	(record_stack_refs): Turn from being a for_each_rtx callback
> 	to being a function that examines each subrtx itself.
> 	Take a pointer to the reflist.  Invert sense of return value
> 	so that true means success and false means failure.  Don't
> 	handle null rtxes.
> 	(combine_stack_adjustments_for_block): Update accordingly.
OK.
Jeff

[PATCH 09/50] cfgcleanup.c:mentions_nonequal_regs

[Richard Sandiford]

gcc/
	* cfgcleanup.c: Include rtl-iter.h.
	(mentions_nonequal_regs): Turn from being a for_each_rtx callback
	to being a function that examines each subrtx itself.
	(thread_jump): Update accordingly.

Index: gcc/cfgcleanup.c
===================================================================
--- gcc/cfgcleanup.c	2014-08-03 11:25:10.407959076 +0100
+++ gcc/cfgcleanup.c	2014-08-03 11:25:22.602079634 +0100
@@ -53,6 +53,7 @@ Software Foundation; either version 3, o
 #include "df.h"
 #include "dce.h"
 #include "dbgcnt.h"
+#include "rtl-iter.h"
 
 #define FORWARDER_BLOCK_P(BB) ((BB)->flags & BB_FORWARDER_BLOCK)
 
@@ -80,7 +81,6 @@ static edge thread_jump (edge, basic_blo
 static bool mark_effect (rtx, bitmap);
 static void notice_new_block (basic_block);
 static void update_forwarder_flag (basic_block);
-static int mentions_nonequal_regs (rtx *, void *);
 static void merge_memattrs (rtx, rtx);
 \f
 /* Set flags for newly created block.  */
@@ -234,29 +234,31 @@ mark_effect (rtx exp, regset nonequal)
     }
 }
 
-/* Return nonzero if X is a register set in regset DATA.
-   Called via for_each_rtx.  */
-static int
-mentions_nonequal_regs (rtx *x, void *data)
+/* Return true if X contains a register in NONEQUAL.  */
+static bool
+mentions_nonequal_regs (const_rtx x, regset nonequal)
 {
-  regset nonequal = (regset) data;
-  if (REG_P (*x))
+  subrtx_iterator::array_type array;
+  FOR_EACH_SUBRTX (iter, array, x, NONCONST)
     {
-      int regno;
-
-      regno = REGNO (*x);
-      if (REGNO_REG_SET_P (nonequal, regno))
-	return 1;
-      if (regno < FIRST_PSEUDO_REGISTER)
+      const_rtx x = *iter;
+      if (REG_P (x))
 	{
-	  int n = hard_regno_nregs[regno][GET_MODE (*x)];
-	  while (--n > 0)
-	    if (REGNO_REG_SET_P (nonequal, regno + n))
-	      return 1;
+	  unsigned int regno = REGNO (x);
+	  if (REGNO_REG_SET_P (nonequal, regno))
+	    return true;
+	  if (regno < FIRST_PSEUDO_REGISTER)
+	    {
+	      int n = hard_regno_nregs[regno][GET_MODE (x)];
+	      while (--n > 0)
+		if (REGNO_REG_SET_P (nonequal, regno + n))
+		  return true;
+	    }
 	}
     }
-  return 0;
+  return false;
 }
+
 /* Attempt to prove that the basic block B will have no side effects and
    always continues in the same edge if reached via E.  Return the edge
    if exist, NULL otherwise.  */
@@ -379,7 +381,7 @@ thread_jump (edge e, basic_block b)
 
   /* cond2 must not mention any register that is not equal to the
      former block.  */
-  if (for_each_rtx (&cond2, mentions_nonequal_regs, nonequal))
+  if (mentions_nonequal_regs (cond2, nonequal))
     goto failed_exit;
 
   EXECUTE_IF_SET_IN_REG_SET (nonequal, 0, i, rsi)

Re: [PATCH 09/50] cfgcleanup.c:mentions_nonequal_regs

[Jeff Law]

On 08/03/14 07:53, Richard Sandiford wrote:
> gcc/
> 	* cfgcleanup.c: Include rtl-iter.h.
> 	(mentions_nonequal_regs): Turn from being a for_each_rtx callback
> 	to being a function that examines each subrtx itself.
> 	(thread_jump): Update accordingly.
OK.
jeff

[PATCH 10/50] cse.c:approx_reg_cost

[Richard Sandiford]

gcc/
	* cse.c: Include rtl-iter.h.
	(approx_reg_cost_1): Delete.
	(approx_reg_cost): Use FOR_EACH_SUBRTX instead of for_each_rtx.
	Don't handle null rtxes.

Index: gcc/cse.c
===================================================================
--- gcc/cse.c	2014-08-03 11:25:10.373958740 +0100
+++ gcc/cse.c	2014-08-03 11:25:22.876082343 +0100
@@ -42,6 +42,7 @@ Software Foundation; either version 3, o
 #include "df.h"
 #include "dbgcnt.h"
 #include "hash-set.h"
+#include "rtl-iter.h"
 
 /* The basic idea of common subexpression elimination is to go
    through the code, keeping a record of expressions that would
@@ -550,8 +551,6 @@ struct cse_basic_block_data
 
 static bool fixed_base_plus_p (rtx x);
 static int notreg_cost (rtx, enum rtx_code, int);
-static int approx_reg_cost_1 (rtx *, void *);
-static int approx_reg_cost (rtx);
 static int preferable (int, int, int, int);
 static void new_basic_block (void);
 static void make_new_qty (unsigned int, enum machine_mode);
@@ -660,47 +659,35 @@ dump_class (struct table_elt *classp)
     }
 }
 
-/* Subroutine of approx_reg_cost; called through for_each_rtx.  */
+/* Return an estimate of the cost of the registers used in an rtx.
+   This is mostly the number of different REG expressions in the rtx;
+   however for some exceptions like fixed registers we use a cost of
+   0.  If any other hard register reference occurs, return MAX_COST.  */
 
 static int
-approx_reg_cost_1 (rtx *xp, void *data)
+approx_reg_cost (const_rtx x)
 {
-  rtx x = *xp;
-  int *cost_p = (int *) data;
-
-  if (x && REG_P (x))
+  int cost = 0;
+  subrtx_iterator::array_type array;
+  FOR_EACH_SUBRTX (iter, array, x, NONCONST)
     {
-      unsigned int regno = REGNO (x);
-
-      if (! CHEAP_REGNO (regno))
+      const_rtx x = *iter;
+      if (REG_P (x))
 	{
-	  if (regno < FIRST_PSEUDO_REGISTER)
+	  unsigned int regno = REGNO (x);
+	  if (!CHEAP_REGNO (regno))
 	    {
-	      if (targetm.small_register_classes_for_mode_p (GET_MODE (x)))
-		return 1;
-	      *cost_p += 2;
+	      if (regno < FIRST_PSEUDO_REGISTER)
+		{
+		  if (targetm.small_register_classes_for_mode_p (GET_MODE (x)))
+		    return MAX_COST;
+		  cost += 2;
+		}
+	      else
+		cost += 1;
 	    }
-	  else
-	    *cost_p += 1;
 	}
     }
-
-  return 0;
-}
-
-/* Return an estimate of the cost of the registers used in an rtx.
-   This is mostly the number of different REG expressions in the rtx;
-   however for some exceptions like fixed registers we use a cost of
-   0.  If any other hard register reference occurs, return MAX_COST.  */
-
-static int
-approx_reg_cost (rtx x)
-{
-  int cost = 0;
-
-  if (for_each_rtx (&x, approx_reg_cost_1, (void *) &cost))
-    return MAX_COST;
-
   return cost;
 }
 

Re: [PATCH 10/50] cse.c:approx_reg_cost

[Jeff Law]

On 08/03/14 07:54, Richard Sandiford wrote:
> gcc/
> 	* cse.c: Include rtl-iter.h.
> 	(approx_reg_cost_1): Delete.
> 	(approx_reg_cost): Use FOR_EACH_SUBRTX instead of for_each_rtx.
> 	Don't handle null rtxes.
OK.
jeff

[PATCH 11/50] Faster for_each_rtx-like iterators

[Richard Sandiford]

The order of the arguments matches canon_anti_dependence rather than
the order of the old structure fields.


gcc/
	* cse.c (check_dependence_data): Delete.
	(check_dependence): Change from being a for_each_rtx callback to being
	a function that examines all subrtxes itself.  Don't handle null rtxes.
	(invalidate): Update call accordingly.

Index: gcc/cse.c
===================================================================
--- gcc/cse.c	2014-08-03 11:25:22.876082343 +0100
+++ gcc/cse.c	2014-08-03 11:25:23.180085349 +0100
@@ -600,7 +600,6 @@ static int check_for_label_ref (rtx *, v
 extern void dump_class (struct table_elt*);
 static void get_cse_reg_info_1 (unsigned int regno);
 static struct cse_reg_info * get_cse_reg_info (unsigned int regno);
-static int check_dependence (rtx *, void *);
 
 static void flush_hash_table (void);
 static bool insn_live_p (rtx, int *);
@@ -1816,22 +1815,20 @@ flush_hash_table (void)
       }
 }
 \f
-/* Function called for each rtx to check whether an anti dependence exist.  */
-struct check_dependence_data
-{
-  enum machine_mode mode;
-  rtx exp;
-  rtx addr;
-};
+/* Check whether an anti dependence exists between X and EXP.  MODE and
+   ADDR are as for canon_anti_dependence.  */
 
-static int
-check_dependence (rtx *x, void *data)
+static bool
+check_dependence (const_rtx x, rtx exp, enum machine_mode mode, rtx addr)
 {
-  struct check_dependence_data *d = (struct check_dependence_data *) data;
-  if (*x && MEM_P (*x))
-    return canon_anti_dependence (*x, true, d->exp, d->mode, d->addr);
-  else
-    return 0;
+  subrtx_iterator::array_type array;
+  FOR_EACH_SUBRTX (iter, array, x, NONCONST)
+    {
+      const_rtx x = *iter;
+      if (MEM_P (x) && canon_anti_dependence (x, true, exp, mode, addr))
+	return true;
+    }
+  return false;
 }
 \f
 /* Remove from the hash table, or mark as invalid, all expressions whose
@@ -1952,18 +1949,13 @@ invalidate (rtx x, enum machine_mode ful
 	      next = p->next_same_hash;
 	      if (p->in_memory)
 		{
-		  struct check_dependence_data d;
-
 		  /* Just canonicalize the expression once;
 		     otherwise each time we call invalidate
 		     true_dependence will canonicalize the
 		     expression again.  */
 		  if (!p->canon_exp)
 		    p->canon_exp = canon_rtx (p->exp);
-		  d.exp = x;
-		  d.addr = addr;
-		  d.mode = full_mode;
-		  if (for_each_rtx (&p->canon_exp, check_dependence, &d))
+		  if (check_dependence (p->canon_exp, x, full_mode, addr))
 		    remove_from_table (p, i);
 		}
 	    }

Re: [PATCH 11/50] Faster for_each_rtx-like iterators

[Jeff Law]

On 08/03/14 07:55, Richard Sandiford wrote:
> The order of the arguments matches canon_anti_dependence rather than
> the order of the old structure fields.
>
>
> gcc/
> 	* cse.c (check_dependence_data): Delete.
> 	(check_dependence): Change from being a for_each_rtx callback to being
> 	a function that examines all subrtxes itself.  Don't handle null rtxes.
> 	(invalidate): Update call accordingly.
OK.
jeff

[PATCH 12/50] cse.c:check_for_label_ref

[Richard Sandiford]

gcc/
	* cse.c (check_for_label_ref): Move earlier in file.  Turn from
	being a for_each_rtx callback to being a function that examines
	each subrtx itself.
	(cse_extended_basic_block): Update call accordingly.

Index: gcc/cse.c
===================================================================
--- gcc/cse.c	2014-08-03 11:25:23.180085349 +0100
+++ gcc/cse.c	2014-08-03 11:25:23.456088077 +0100
@@ -596,7 +596,6 @@ static void invalidate_from_clobbers (rt
 static void invalidate_from_sets_and_clobbers (rtx);
 static rtx cse_process_notes (rtx, rtx, bool *);
 static void cse_extended_basic_block (struct cse_basic_block_data *);
-static int check_for_label_ref (rtx *, void *);
 extern void dump_class (struct table_elt*);
 static void get_cse_reg_info_1 (unsigned int regno);
 static struct cse_reg_info * get_cse_reg_info (unsigned int regno);
@@ -6366,6 +6365,32 @@ cse_prescan_path (struct cse_basic_block
   data->nsets = nsets;
 }
 \f
+/* Return true if the pattern of INSN uses a LABEL_REF for which
+   there isn't a REG_LABEL_OPERAND note.  */
+
+static bool
+check_for_label_ref (rtx insn)
+{
+  /* If this insn uses a LABEL_REF and there isn't a REG_LABEL_OPERAND
+     note for it, we must rerun jump since it needs to place the note.  If
+     this is a LABEL_REF for a CODE_LABEL that isn't in the insn chain,
+     don't do this since no REG_LABEL_OPERAND will be added.  */
+  subrtx_iterator::array_type array;
+  FOR_EACH_SUBRTX (iter, array, PATTERN (insn), ALL)
+    {
+      const_rtx x = *iter;
+      if (GET_CODE (x) == LABEL_REF
+	  && !LABEL_REF_NONLOCAL_P (x)
+	  && (!JUMP_P (insn)
+	      || !label_is_jump_target_p (XEXP (x, 0), insn))
+	  && LABEL_P (XEXP (x, 0))
+	  && INSN_UID (XEXP (x, 0)) != 0
+	  && !find_reg_note (insn, REG_LABEL_OPERAND, XEXP (x, 0)))
+	return true;
+    }
+  return false;
+}
+
 /* Process a single extended basic block described by EBB_DATA.  */
 
 static void
@@ -6436,8 +6461,7 @@ cse_extended_basic_block (struct cse_bas
 	      /* If we haven't already found an insn where we added a LABEL_REF,
 		 check this one.  */
 	      if (INSN_P (insn) && !recorded_label_ref
-		  && for_each_rtx (&PATTERN (insn), check_for_label_ref,
-				   (void *) insn))
+		  && check_for_label_ref (insn))
 		recorded_label_ref = true;
 
 #ifdef HAVE_cc0
@@ -6629,28 +6653,6 @@ cse_main (rtx f ATTRIBUTE_UNUSED, int nr
     return 0;
 }
 \f
-/* Called via for_each_rtx to see if an insn is using a LABEL_REF for
-   which there isn't a REG_LABEL_OPERAND note.
-   Return one if so.  DATA is the insn.  */
-
-static int
-check_for_label_ref (rtx *rtl, void *data)
-{
-  rtx insn = (rtx) data;
-
-  /* If this insn uses a LABEL_REF and there isn't a REG_LABEL_OPERAND
-     note for it, we must rerun jump since it needs to place the note.  If
-     this is a LABEL_REF for a CODE_LABEL that isn't in the insn chain,
-     don't do this since no REG_LABEL_OPERAND will be added.  */
-  return (GET_CODE (*rtl) == LABEL_REF
-	  && ! LABEL_REF_NONLOCAL_P (*rtl)
-	  && (!JUMP_P (insn)
-	      || !label_is_jump_target_p (XEXP (*rtl, 0), insn))
-	  && LABEL_P (XEXP (*rtl, 0))
-	  && INSN_UID (XEXP (*rtl, 0)) != 0
-	  && ! find_reg_note (insn, REG_LABEL_OPERAND, XEXP (*rtl, 0)));
-}
-\f
 /* Count the number of times registers are used (not set) in X.
    COUNTS is an array in which we accumulate the count, INCR is how much
    we count each register usage.

Re: [PATCH 12/50] cse.c:check_for_label_ref

[Jeff Law]

On 08/03/14 07:56, Richard Sandiford wrote:
> gcc/
> 	* cse.c (check_for_label_ref): Move earlier in file.  Turn from
> 	being a for_each_rtx callback to being a function that examines
> 	each subrtx itself.
> 	(cse_extended_basic_block): Update call accordingly.
OK.
Jeff

[PATCH 13/50] cse.c:is_dead_debug_insn

[Richard Sandiford]

gcc/
	* cse.c (is_dead_reg): Change argument to const_rtx.
	(dead_debug_insn_data): Delete.
	(is_dead_debug_insn): Expand commentary.  Turn from being a
	for_each_rtx callback to being a function that examines
	each subrtx itself.  Take the fields of dead_debug_insn_data
	as argument.
	(delete_trivially_dead_insns): Update call accordingly.

Index: gcc/cse.c
===================================================================
--- gcc/cse.c	2014-08-03 11:25:23.456088077 +0100
+++ gcc/cse.c	2014-08-03 11:25:23.760091083 +0100
@@ -6783,7 +6783,7 @@ count_reg_usage (rtx x, int *counts, rtx
 /* Return true if X is a dead register.  */
 
 static inline int
-is_dead_reg (rtx x, int *counts)
+is_dead_reg (const_rtx x, int *counts)
 {
   return (REG_P (x)
 	  && REGNO (x) >= FIRST_PSEUDO_REGISTER
@@ -6870,30 +6870,29 @@ count_stores (rtx x, const_rtx set ATTRI
     counts[REGNO (x)]++;
 }
 
-struct dead_debug_insn_data
-{
-  int *counts;
-  rtx *replacements;
-  bool seen_repl;
-};
-
-/* Return if a DEBUG_INSN needs to be reset because some dead
-   pseudo doesn't have a replacement.  Callback for for_each_rtx.  */
+/* Return if DEBUG_INSN pattern PAT needs to be reset because some dead
+   pseudo doesn't have a replacement.  COUNTS[X] is zero if register X
+   is dead and REPLACEMENTS[X] is null if it has no replacemenet.
+   Set *SEEN_REPL to true if we see a dead register that does have
+   a replacement.  */
 
-static int
-is_dead_debug_insn (rtx *loc, void *data)
+static bool
+is_dead_debug_insn (const_rtx pat, int *counts, rtx *replacements,
+		    bool *seen_repl)
 {
-  rtx x = *loc;
-  struct dead_debug_insn_data *ddid = (struct dead_debug_insn_data *) data;
-
-  if (is_dead_reg (x, ddid->counts))
+  subrtx_iterator::array_type array;
+  FOR_EACH_SUBRTX (iter, array, pat, NONCONST)
     {
-      if (ddid->replacements && ddid->replacements[REGNO (x)] != NULL_RTX)
-	ddid->seen_repl = true;
-      else
-	return 1;
+      const_rtx x = *iter;
+      if (is_dead_reg (x, counts))
+	{
+	  if (replacements && replacements[REGNO (x)] != NULL_RTX)
+	    *seen_repl = true;
+	  else
+	    return true;
+	}
     }
-  return 0;
+  return false;
 }
 
 /* Replace a dead pseudo in a DEBUG_INSN with replacement DEBUG_EXPR.
@@ -7035,22 +7034,19 @@ delete_trivially_dead_insns (rtx insns,
 
   if (MAY_HAVE_DEBUG_INSNS)
     {
-      struct dead_debug_insn_data ddid;
-      ddid.counts = counts;
-      ddid.replacements = replacements;
       for (insn = get_last_insn (); insn; insn = PREV_INSN (insn))
 	if (DEBUG_INSN_P (insn))
 	  {
 	    /* If this debug insn references a dead register that wasn't replaced
 	       with an DEBUG_EXPR, reset the DEBUG_INSN.  */
-	    ddid.seen_repl = false;
-	    if (for_each_rtx (&INSN_VAR_LOCATION_LOC (insn),
-			      is_dead_debug_insn, &ddid))
+	    bool seen_repl = false;
+	    if (is_dead_debug_insn (INSN_VAR_LOCATION_LOC (insn),
+				    counts, replacements, &seen_repl))
 	      {
 		INSN_VAR_LOCATION_LOC (insn) = gen_rtx_UNKNOWN_VAR_LOC ();
 		df_insn_rescan (insn);
 	      }
-	    else if (ddid.seen_repl)
+	    else if (seen_repl)
 	      {
 		INSN_VAR_LOCATION_LOC (insn)
 		  = simplify_replace_fn_rtx (INSN_VAR_LOCATION_LOC (insn),

Re: [PATCH 13/50] cse.c:is_dead_debug_insn

[Jeff Law]

On 08/03/14 07:57, Richard Sandiford wrote:
> gcc/
> 	* cse.c (is_dead_reg): Change argument to const_rtx.
> 	(dead_debug_insn_data): Delete.
> 	(is_dead_debug_insn): Expand commentary.  Turn from being a
> 	for_each_rtx callback to being a function that examines
> 	each subrtx itself.  Take the fields of dead_debug_insn_data
> 	as argument.
> 	(delete_trivially_dead_insns): Update call accordingly.
OK.
jeff

[PATCH 14/50] cse.c:cse_change_cc_mode

[Richard Sandiford]

gcc/
	* cse.c (change_cc_mode_args):  Delete.
	(cse_change_cc_mode): Turn from being a for_each_rtx callback to being
	a function that examines each subrtx itself.  Take the fields of
	change_cc_mode_args as argument and return void.
	(cse_change_cc_mode_insn): Update calls accordingly.

Index: gcc/cse.c
===================================================================
--- gcc/cse.c	2014-08-03 11:25:23.760091083 +0100
+++ gcc/cse.c	2014-08-03 11:25:24.059094039 +0100
@@ -255,14 +255,6 @@ struct qty_table_elem
 /* The table of all qtys, indexed by qty number.  */
 static struct qty_table_elem *qty_table;
 
-/* Structure used to pass arguments via for_each_rtx to function
-   cse_change_cc_mode.  */
-struct change_cc_mode_args
-{
-  rtx insn;
-  rtx newreg;
-};
-
 #ifdef HAVE_cc0
 /* For machines that have a CC0, we do not record its value in the hash
    table since its use is guaranteed to be the insn immediately following
@@ -603,7 +595,6 @@ static struct cse_reg_info * get_cse_reg
 static void flush_hash_table (void);
 static bool insn_live_p (rtx, int *);
 static bool set_live_p (rtx, rtx, int *);
-static int cse_change_cc_mode (rtx *, void *);
 static void cse_change_cc_mode_insn (rtx, rtx);
 static void cse_change_cc_mode_insns (rtx, rtx, rtx);
 static enum machine_mode cse_cc_succs (basic_block, basic_block, rtx, rtx,
@@ -7067,26 +7058,26 @@ delete_trivially_dead_insns (rtx insns,
   return ndead;
 }
 
-/* This function is called via for_each_rtx.  The argument, NEWREG, is
-   a condition code register with the desired mode.  If we are looking
-   at the same register in a different mode, replace it with
-   NEWREG.  */
+/* If LOC contains references to NEWREG in a different mode, change them
+   to use NEWREG instead.  */
 
-static int
-cse_change_cc_mode (rtx *loc, void *data)
+static void
+cse_change_cc_mode (subrtx_ptr_iterator::array_type &array,
+		    rtx *loc, rtx insn, rtx newreg)
 {
-  struct change_cc_mode_args* args = (struct change_cc_mode_args*)data;
-
-  if (*loc
-      && REG_P (*loc)
-      && REGNO (*loc) == REGNO (args->newreg)
-      && GET_MODE (*loc) != GET_MODE (args->newreg))
+  FOR_EACH_SUBRTX_PTR (iter, array, loc, NONCONST)
     {
-      validate_change (args->insn, loc, args->newreg, 1);
-
-      return -1;
+      rtx *loc = *iter;
+      rtx x = *loc;
+      if (x
+	  && REG_P (x)
+	  && REGNO (x) == REGNO (newreg)
+	  && GET_MODE (x) != GET_MODE (newreg))
+	{
+	  validate_change (insn, loc, newreg, 1);
+	  iter.skip_subrtxes ();
+	}
     }
-  return 0;
 }
 
 /* Change the mode of any reference to the register REGNO (NEWREG) to
@@ -7095,17 +7086,14 @@ cse_change_cc_mode (rtx *loc, void *data
 static void
 cse_change_cc_mode_insn (rtx insn, rtx newreg)
 {
-  struct change_cc_mode_args args;
   int success;
 
   if (!INSN_P (insn))
     return;
 
-  args.insn = insn;
-  args.newreg = newreg;
-
-  for_each_rtx (&PATTERN (insn), cse_change_cc_mode, &args);
-  for_each_rtx (&REG_NOTES (insn), cse_change_cc_mode, &args);
+  subrtx_ptr_iterator::array_type array;
+  cse_change_cc_mode (array, &PATTERN (insn), insn, newreg);
+  cse_change_cc_mode (array, &REG_NOTES (insn), insn, newreg);
 
   /* If the following assertion was triggered, there is most probably
      something wrong with the cc_modes_compatible back end function.

Re: [PATCH 14/50] cse.c:cse_change_cc_mode

[Jeff Law]

On 08/03/14 07:58, Richard Sandiford wrote:
> gcc/
> 	* cse.c (change_cc_mode_args):  Delete.
> 	(cse_change_cc_mode): Turn from being a for_each_rtx callback to being
> 	a function that examines each subrtx itself.  Take the fields of
> 	change_cc_mode_args as argument and return void.
> 	(cse_change_cc_mode_insn): Update calls accordingly.
OK.
jeff

[PATCH 15/50] ddg.c:mark_mem_use

[Richard Sandiford]

gcc/
	* ddg.c: Include rtl-iter.h.
	(mark_mem_use_1): Rename to...
	(mark_mem_use): ...deleting old mark_mem_use.  Use FOR_EACH_SUBRTX
	instead of for_each_rtx.
	(mem_read_insn_p): Update accordingly.

Index: gcc/ddg.c
===================================================================
--- gcc/ddg.c	2014-08-03 11:25:10.216957187 +0100
+++ gcc/ddg.c	2014-08-03 11:25:24.327096689 +0100
@@ -41,6 +41,7 @@ Software Foundation; either version 3, o
 #include "expr.h"
 #include "bitmap.h"
 #include "ddg.h"
+#include "rtl-iter.h"
 
 #ifdef INSN_SCHEDULING
 
@@ -63,19 +64,16 @@ static void add_edge_to_ddg (ddg_ptr g,
 static bool mem_ref_p;
 
 /* Auxiliary function for mem_read_insn_p.  */
-static int
-mark_mem_use (rtx *x, void *data ATTRIBUTE_UNUSED)
-{
-  if (MEM_P (*x))
-    mem_ref_p = true;
-  return 0;
-}
-
-/* Auxiliary function for mem_read_insn_p.  */
 static void
-mark_mem_use_1 (rtx *x, void *data)
+mark_mem_use (rtx *x, void *)
 {
-  for_each_rtx (x, mark_mem_use, data);
+  subrtx_iterator::array_type array;
+  FOR_EACH_SUBRTX (iter, array, *x, NONCONST)
+    if (MEM_P (*x))
+      {
+	mem_ref_p = true;
+	break;
+      }
 }
 
 /* Returns nonzero if INSN reads from memory.  */
@@ -83,7 +81,7 @@ mark_mem_use_1 (rtx *x, void *data)
 mem_read_insn_p (rtx insn)
 {
   mem_ref_p = false;
-  note_uses (&PATTERN (insn), mark_mem_use_1, NULL);
+  note_uses (&PATTERN (insn), mark_mem_use, NULL);
   return mem_ref_p;
 }
 

Re: [PATCH 15/50] ddg.c:mark_mem_use

[Jeff Law]

On 08/03/14 07:58, Richard Sandiford wrote:
> gcc/
> 	* ddg.c: Include rtl-iter.h.
> 	(mark_mem_use_1): Rename to...
> 	(mark_mem_use): ...deleting old mark_mem_use.  Use FOR_EACH_SUBRTX
> 	instead of for_each_rtx.
> 	(mem_read_insn_p): Update accordingly.
OK.
jeff

[PATCH 16/50] ddg.c:insns_may_alias_p

[Richard Sandiford]

gcc/
	* ddg.c (walk_mems_2, walk_mems_1): Delete.
	(insns_may_alias_p): Use FOR_EACH_SUBRTX rather than for_each_rtx
	to iterate over subrtxes.  Return a bool rather than an int.

Index: gcc/ddg.c
===================================================================
--- gcc/ddg.c	2014-08-03 11:25:24.327096689 +0100
+++ gcc/ddg.c	2014-08-03 11:25:24.650099882 +0100
@@ -397,41 +397,25 @@ build_inter_loop_deps (ddg_ptr g)
 }
 
 
-static int
-walk_mems_2 (rtx *x, rtx mem)
-{
-  if (MEM_P (*x))
-    {
-      if (may_alias_p (*x, mem))
-        return 1;
-
-      return -1;
-    }
-  return 0;
-}
-
-static int
-walk_mems_1 (rtx *x, rtx *pat)
+/* Return true if two specified instructions have mem expr with conflict
+   alias sets.  */
+static bool
+insns_may_alias_p (rtx insn1, rtx insn2)
 {
-  if (MEM_P (*x))
+  subrtx_iterator::array_type array1;
+  subrtx_iterator::array_type array2;
+  FOR_EACH_SUBRTX (iter1, array1, PATTERN (insn1), NONCONST)
     {
-      /* Visit all MEMs in *PAT and check independence.  */
-      if (for_each_rtx (pat, (rtx_function) walk_mems_2, *x))
-        /* Indicate that dependence was determined and stop traversal.  */
-        return 1;
-
-      return -1;
+      const_rtx x1 = *iter1;
+      if (MEM_P (x1))
+	FOR_EACH_SUBRTX (iter2, array2, PATTERN (insn2), NONCONST)
+	  {
+	    const_rtx x2 = *iter2;
+	    if (MEM_P (x2) && may_alias_p (x2, x1))
+	      return true;
+	  }
     }
-  return 0;
-}
-
-/* Return 1 if two specified instructions have mem expr with conflict alias sets*/
-static int
-insns_may_alias_p (rtx insn1, rtx insn2)
-{
-  /* For each pair of MEMs in INSN1 and INSN2 check their independence.  */
-  return  for_each_rtx (&PATTERN (insn1), (rtx_function) walk_mems_1,
-			 &PATTERN (insn2));
+  return false;
 }
 
 /* Given two nodes, analyze their RTL insns and add intra-loop mem deps

Re: [PATCH 16/50] ddg.c:insns_may_alias_p

[Jeff Law]

On 08/03/14 07:59, Richard Sandiford wrote:
> gcc/
> 	* ddg.c (walk_mems_2, walk_mems_1): Delete.
> 	(insns_may_alias_p): Use FOR_EACH_SUBRTX rather than for_each_rtx
> 	to iterate over subrtxes.  Return a bool rather than an int.
OK.
Jeff

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

[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;
 

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

[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

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

[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.

[PATCH 18/50] dse.c:check_mem_read_use

[Richard Sandiford]

gcc/
	* dse.c: Include rtl-iter.h.
	(check_mem_read_rtx): Change void * parameter to real type.
	Remove return value.
	(check_mem_read_use): Fix comment.  Use FOR_EACH_SUBRTX_PTR instead of
	for_each_rtx.  Don't handle null rtxes.

Index: gcc/dse.c
===================================================================
--- gcc/dse.c	2014-08-03 11:25:10.142956456 +0100
+++ gcc/dse.c	2014-08-03 11:25:25.192105241 +0100
@@ -54,6 +54,7 @@ Software Foundation; either version 3, o
 #include "is-a.h"
 #include "gimple.h"
 #include "gimple-ssa.h"
+#include "rtl-iter.h"
 
 /* This file contains three techniques for performing Dead Store
    Elimination (dse).
@@ -2089,15 +2090,13 @@ replace_read (store_info_t store_info, i
     }
 }
 
-/* A for_each_rtx callback in which DATA is the bb_info.  Check to see
-   if LOC is a mem and if it is look at the address and kill any
-   appropriate stores that may be active.  */
+/* Check the address of MEM *LOC and kill any appropriate stores that may
+   be active.  */
 
-static int
-check_mem_read_rtx (rtx *loc, void *data)
+static void
+check_mem_read_rtx (rtx *loc, bb_info_t bb_info)
 {
   rtx mem = *loc, mem_addr;
-  bb_info_t bb_info;
   insn_info_t insn_info;
   HOST_WIDE_INT offset = 0;
   HOST_WIDE_INT width = 0;
@@ -2106,10 +2105,6 @@ check_mem_read_rtx (rtx *loc, void *data
   int group_id;
   read_info_t read_info;
 
-  if (!mem || !MEM_P (mem))
-    return 0;
-
-  bb_info = (bb_info_t) data;
   insn_info = bb_info->last_insn;
 
   if ((MEM_ALIAS_SET (mem) == ALIAS_SET_MEMORY_BARRIER)
@@ -2119,20 +2114,20 @@ check_mem_read_rtx (rtx *loc, void *data
 	fprintf (dump_file, " adding wild read, volatile or barrier.\n");
       add_wild_read (bb_info);
       insn_info->cannot_delete = true;
-      return 0;
+      return;
     }
 
   /* If it is reading readonly mem, then there can be no conflict with
      another write. */
   if (MEM_READONLY_P (mem))
-    return 0;
+    return;
 
   if (!canon_address (mem, &spill_alias_set, &group_id, &offset, &base))
     {
       if (dump_file && (dump_flags & TDF_DETAILS))
 	fprintf (dump_file, " adding wild read, canon_address failure.\n");
       add_wild_read (bb_info);
-      return 0;
+      return;
     }
 
   if (GET_MODE (mem) == BLKmode)
@@ -2260,7 +2255,7 @@ check_mem_read_rtx (rtx *loc, void *data
 						 width)
 		      && replace_read (store_info, i_ptr, read_info,
 				       insn_info, loc, bb_info->regs_live))
-		    return 0;
+		    return;
 
 		  /* The bases are the same, just see if the offsets
 		     overlap.  */
@@ -2327,7 +2322,7 @@ check_mem_read_rtx (rtx *loc, void *data
 					 offset - store_info->begin, width)
 	      && replace_read (store_info, i_ptr,  read_info, insn_info, loc,
 			       bb_info->regs_live))
-	    return 0;
+	    return;
 
 	  if (!store_info->alias_set)
 	    remove = canon_true_dependence (store_info->mem,
@@ -2351,17 +2346,22 @@ check_mem_read_rtx (rtx *loc, void *data
 	  i_ptr = i_ptr->next_local_store;
 	}
     }
-  return 0;
 }
 
-/* A for_each_rtx callback in which DATA points the INSN_INFO for
+/* A note_uses callback in which DATA points the INSN_INFO for
    as check_mem_read_rtx.  Nullify the pointer if i_m_r_m_r returns
    true for any part of *LOC.  */
 
 static void
 check_mem_read_use (rtx *loc, void *data)
 {
-  for_each_rtx (loc, check_mem_read_rtx, data);
+  subrtx_ptr_iterator::array_type array;
+  FOR_EACH_SUBRTX_PTR (iter, array, loc, NONCONST)
+    {
+      rtx *loc = *iter;
+      if (MEM_P (*loc))
+	check_mem_read_rtx (loc, (bb_info_t) data);
+    }
 }
 
 

Re: [PATCH 18/50] dse.c:check_mem_read_use

[Jeff Law]

On 08/03/14 08:02, Richard Sandiford wrote:
> gcc/
> 	* dse.c: Include rtl-iter.h.
> 	(check_mem_read_rtx): Change void * parameter to real type.
> 	Remove return value.
> 	(check_mem_read_use): Fix comment.  Use FOR_EACH_SUBRTX_PTR instead of
> 	for_each_rtx.  Don't handle null rtxes.
OK.
Jeff

[PATCH 19/50] dwarf2out.c:const_ok_for_output

[Richard Sandiford]

gcc/
	* dwarf2out.c: Include rtl-iter.h.
	(const_ok_for_output_1): Take the rtx instead of a pointer to it.
	Remove unused data parameter.  Return a bool, inverting the result
	so that 0/false means "not ok".
	(const_ok_for_output): Update accordingly.  Use FOR_EACH_SUBRTX_VAR
	instead of for_each_rtx.

Index: gcc/dwarf2out.c
===================================================================
--- gcc/dwarf2out.c	2014-08-03 11:25:10.105956090 +0100
+++ gcc/dwarf2out.c	2014-08-03 11:25:25.454107831 +0100
@@ -98,6 +98,7 @@ Software Foundation; either version 3, o
 #include "opts.h"
 #include "tree-dfa.h"
 #include "gdb/gdb-index.h"
+#include "rtl-iter.h"
 
 static void dwarf2out_source_line (unsigned int, const char *, int, bool);
 static rtx last_var_location_insn;
@@ -11433,14 +11434,11 @@ expansion_failed (tree expr, rtx rtl, ch
     }
 }
 
-/* Helper function for const_ok_for_output, called either directly
-   or via for_each_rtx.  */
+/* Helper function for const_ok_for_output.  */
 
-static int
-const_ok_for_output_1 (rtx *rtlp, void *data ATTRIBUTE_UNUSED)
+static bool
+const_ok_for_output_1 (rtx rtl)
 {
-  rtx rtl = *rtlp;
-
   if (GET_CODE (rtl) == UNSPEC)
     {
       /* If delegitimize_address couldn't do anything with the UNSPEC, assume
@@ -11468,14 +11466,14 @@ const_ok_for_output_1 (rtx *rtlp, void *
 #endif
       expansion_failed (NULL_TREE, rtl,
 			"UNSPEC hasn't been delegitimized.\n");
-      return 1;
+      return false;
     }
 
   if (targetm.const_not_ok_for_debug_p (rtl))
     {
       expansion_failed (NULL_TREE, rtl,
 			"Expression rejected for debug by the backend.\n");
-      return 1;
+      return false;
     }
 
   /* FIXME: Refer to PR60655. It is possible for simplification
@@ -11486,9 +11484,8 @@ const_ok_for_output_1 (rtx *rtlp, void *
   if (GET_CODE (rtl) != SYMBOL_REF)
     {
       if (GET_CODE (rtl) == NOT)
-	  return 1;
-
-      return 0;
+	return false;
+      return true;
     }
 
   if (CONSTANT_POOL_ADDRESS_P (rtl))
@@ -11501,12 +11498,12 @@ const_ok_for_output_1 (rtx *rtlp, void *
 	{
 	  expansion_failed (NULL_TREE, rtl,
 			    "Constant was removed from constant pool.\n");
-	  return 1;
+	  return false;
 	}
     }
 
   if (SYMBOL_REF_TLS_MODEL (rtl) != TLS_MODEL_NONE)
-    return 1;
+    return false;
 
   /* Avoid references to external symbols in debug info, on several targets
      the linker might even refuse to link when linking a shared library,
@@ -11521,11 +11518,11 @@ const_ok_for_output_1 (rtx *rtlp, void *
 	{
 	  expansion_failed (NULL_TREE, rtl,
 			    "Symbol not defined in current TU.\n");
-	  return 1;
+	  return false;
 	}
     }
 
-  return 0;
+  return true;
 }
 
 /* Return true if constant RTL can be emitted in DW_OP_addr or
@@ -11536,10 +11533,16 @@ const_ok_for_output_1 (rtx *rtlp, void *
 const_ok_for_output (rtx rtl)
 {
   if (GET_CODE (rtl) == SYMBOL_REF)
-    return const_ok_for_output_1 (&rtl, NULL) == 0;
+    return const_ok_for_output_1 (rtl);
 
   if (GET_CODE (rtl) == CONST)
-    return for_each_rtx (&XEXP (rtl, 0), const_ok_for_output_1, NULL) == 0;
+    {
+      subrtx_var_iterator::array_type array;
+      FOR_EACH_SUBRTX_VAR (iter, array, XEXP (rtl, 0), ALL)
+	if (!const_ok_for_output_1 (*iter))
+	  return false;
+      return true;
+    }
 
   return true;
 }

Re: [PATCH 19/50] dwarf2out.c:const_ok_for_output

[Jeff Law]

On 08/03/14 08:03, Richard Sandiford wrote:
> gcc/
> 	* dwarf2out.c: Include rtl-iter.h.
> 	(const_ok_for_output_1): Take the rtx instead of a pointer to it.
> 	Remove unused data parameter.  Return a bool, inverting the result
> 	so that 0/false means "not ok".
> 	(const_ok_for_output): Update accordingly.  Use FOR_EACH_SUBRTX_VAR
> 	instead of for_each_rtx.
OK.
jeff

[PATCH 20/50] dwarf2out.c:resolve_one_addr

[Richard Sandiford]

gcc/
	* dwarf2out.c (resolve_one_addr): Remove unused data parameter.
	Return a bool, inverting the result so that 0/false means "not ok".
	Use FOR_EACH_SUBRTX_PTR instead of for_each_rtx to iterate over
	subrtxes of a CONST.
	(mem_loc_descriptor, add_const_value_attribute)
	(resolve_addr_in_expr): Update calls accordingly.

Index: gcc/dwarf2out.c
===================================================================
--- gcc/dwarf2out.c	2014-08-03 11:25:25.454107831 +0100
+++ gcc/dwarf2out.c	2014-08-03 11:25:25.747110728 +0100
@@ -3156,7 +3156,7 @@ static dw_loc_descr_ref multiple_reg_loc
 static dw_loc_descr_ref based_loc_descr (rtx, HOST_WIDE_INT,
 					 enum var_init_status);
 static int is_based_loc (const_rtx);
-static int resolve_one_addr (rtx *, void *);
+static bool resolve_one_addr (rtx *);
 static dw_loc_descr_ref concat_loc_descriptor (rtx, rtx,
 					       enum var_init_status);
 static dw_loc_descr_ref loc_descriptor (rtx, enum machine_mode mode,
@@ -13240,7 +13240,7 @@ mem_loc_descriptor (rtx rtl, enum machin
       break;
 
     case CONST_STRING:
-      resolve_one_addr (&rtl, NULL);
+      resolve_one_addr (&rtl);
       goto symref;
 
     default:
@@ -15295,7 +15295,7 @@ add_const_value_attribute (dw_die_ref di
       if (dwarf_version >= 4 || !dwarf_strict)
 	{
 	  dw_loc_descr_ref loc_result;
-	  resolve_one_addr (&rtl, NULL);
+	  resolve_one_addr (&rtl);
 	rtl_addr:
           loc_result = new_addr_loc_descr (rtl, dtprel_false);
 	  add_loc_descr (&loc_result, new_loc_descr (DW_OP_stack_value, 0, 0));
@@ -23127,11 +23127,11 @@ move_marked_base_types (void)
 }
 
 /* Helper function for resolve_addr, attempt to resolve
-   one CONST_STRING, return non-zero if not successful.  Similarly verify that
+   one CONST_STRING, return true if successful.  Similarly verify that
    SYMBOL_REFs refer to variables emitted in the current CU.  */
 
-static int
-resolve_one_addr (rtx *addr, void *data ATTRIBUTE_UNUSED)
+static bool
+resolve_one_addr (rtx *addr)
 {
   rtx rtl = *addr;
 
@@ -23144,15 +23144,15 @@ resolve_one_addr (rtx *addr, void *data
 	= build_array_type (char_type_node, build_index_type (tlen));
       rtl = lookup_constant_def (t);
       if (!rtl || !MEM_P (rtl))
-	return 1;
+	return false;
       rtl = XEXP (rtl, 0);
       if (GET_CODE (rtl) == SYMBOL_REF
 	  && SYMBOL_REF_DECL (rtl)
 	  && !TREE_ASM_WRITTEN (SYMBOL_REF_DECL (rtl)))
-	return 1;
+	return false;
       vec_safe_push (used_rtx_array, rtl);
       *addr = rtl;
-      return 0;
+      return true;
     }
 
   if (GET_CODE (rtl) == SYMBOL_REF
@@ -23161,17 +23161,21 @@ resolve_one_addr (rtx *addr, void *data
       if (TREE_CONSTANT_POOL_ADDRESS_P (rtl))
 	{
 	  if (!TREE_ASM_WRITTEN (DECL_INITIAL (SYMBOL_REF_DECL (rtl))))
-	    return 1;
+	    return false;
 	}
       else if (!TREE_ASM_WRITTEN (SYMBOL_REF_DECL (rtl)))
-	return 1;
+	return false;
     }
 
-  if (GET_CODE (rtl) == CONST
-      && for_each_rtx (&XEXP (rtl, 0), resolve_one_addr, NULL))
-    return 1;
+  if (GET_CODE (rtl) == CONST)
+    {
+      subrtx_ptr_iterator::array_type array;
+      FOR_EACH_SUBRTX_PTR (iter, array, &XEXP (rtl, 0), ALL)
+	if (!resolve_one_addr (*iter))
+	  return false;
+    }
 
-  return 0;
+  return true;
 }
 
 /* For STRING_CST, return SYMBOL_REF of its constant pool entry,
@@ -23283,7 +23287,7 @@ resolve_addr_in_expr (dw_loc_descr_ref l
     switch (loc->dw_loc_opc)
       {
       case DW_OP_addr:
-	if (resolve_one_addr (&loc->dw_loc_oprnd1.v.val_addr, NULL))
+	if (!resolve_one_addr (&loc->dw_loc_oprnd1.v.val_addr))
 	  {
 	    if ((prev == NULL
 		 || prev->dw_loc_opc == DW_OP_piece
@@ -23302,7 +23306,7 @@ resolve_addr_in_expr (dw_loc_descr_ref l
             || (loc->dw_loc_opc == DW_OP_GNU_const_index && loc->dtprel))
           {
             rtx rtl = loc->dw_loc_oprnd1.val_entry->addr.rtl;
-            if (resolve_one_addr (&rtl, NULL))
+            if (!resolve_one_addr (&rtl))
               return false;
             remove_addr_table_entry (loc->dw_loc_oprnd1.val_entry);
             loc->dw_loc_oprnd1.val_entry =
@@ -23312,7 +23316,7 @@ resolve_addr_in_expr (dw_loc_descr_ref l
       case DW_OP_const4u:
       case DW_OP_const8u:
 	if (loc->dtprel
-	    && resolve_one_addr (&loc->dw_loc_oprnd1.v.val_addr, NULL))
+	    && !resolve_one_addr (&loc->dw_loc_oprnd1.v.val_addr))
 	  return false;
 	break;
       case DW_OP_plus_uconst:
@@ -23330,7 +23334,7 @@ resolve_addr_in_expr (dw_loc_descr_ref l
 	break;
       case DW_OP_implicit_value:
 	if (loc->dw_loc_oprnd2.val_class == dw_val_class_addr
-	    && resolve_one_addr (&loc->dw_loc_oprnd2.v.val_addr, NULL))
+	    && !resolve_one_addr (&loc->dw_loc_oprnd2.v.val_addr))
 	  return false;
 	break;
       case DW_OP_GNU_implicit_pointer:
@@ -23587,7 +23591,7 @@ resolve_addr (dw_die_ref die)
 	break;
       case dw_val_class_addr:
 	if (a->dw_attr == DW_AT_const_value
-	    && resolve_one_addr (&a->dw_attr_val.v.val_addr, NULL))
+	    && !resolve_one_addr (&a->dw_attr_val.v.val_addr))
 	  {
             if (AT_index (a) != NOT_INDEXED)
               remove_addr_table_entry (a->dw_attr_val.val_entry);

Re: [PATCH 20/50] dwarf2out.c:resolve_one_addr

[Jeff Law]

On 08/03/14 08:04, Richard Sandiford wrote:
> gcc/
> 	* dwarf2out.c (resolve_one_addr): Remove unused data parameter.
> 	Return a bool, inverting the result so that 0/false means "not ok".
> 	Use FOR_EACH_SUBRTX_PTR instead of for_each_rtx to iterate over
> 	subrtxes of a CONST.
> 	(mem_loc_descriptor, add_const_value_attribute)
> 	(resolve_addr_in_expr): Update calls accordingly.
OK.
jeff

[PATCH 21/50] Faster for_each_rtx-like iterators

[Richard Sandiford]

The switch statement in the old code seemed overly cautious.  It's well
established elsewhere that the first operand of an RTX_AUTOINC is the
automodified register.  If anyone wanted to add a new code for which that
wasn't true they should (a) reconsider or (b) go through all RTX_AUTOINCs
as a precaution.


gcc/
	* emit-rtl.c: Include rtl-iter.h.
	(find_auto_inc): Turn from being a for_each_rtx callback to being
	a function that examines each subrtx itself.  Assume the first operand
	to an RTX_AUTOINC is the automodified register.
	(try_split): Update call accordingly.

Index: gcc/emit-rtl.c
===================================================================
--- gcc/emit-rtl.c	2014-08-03 11:25:10.047955517 +0100
+++ gcc/emit-rtl.c	2014-08-03 11:25:26.062113842 +0100
@@ -58,6 +58,7 @@ Software Foundation; either version 3, o
 #include "params.h"
 #include "target.h"
 #include "builtins.h"
+#include "rtl-iter.h"
 
 struct target_rtl default_target_rtl;
 #if SWITCHABLE_TARGET
@@ -3485,30 +3486,17 @@ prev_cc0_setter (rtx insn)
 /* Find a RTX_AUTOINC class rtx which matches DATA.  */
 
 static int
-find_auto_inc (rtx *xp, void *data)
+find_auto_inc (const_rtx x, const_rtx reg)
 {
-  rtx x = *xp;
-  rtx reg = (rtx) data;
-
-  if (GET_RTX_CLASS (GET_CODE (x)) != RTX_AUTOINC)
-    return 0;
-
-  switch (GET_CODE (x))
+  subrtx_iterator::array_type array;
+  FOR_EACH_SUBRTX (iter, array, x, NONCONST)
     {
-      case PRE_DEC:
-      case PRE_INC:
-      case POST_DEC:
-      case POST_INC:
-      case PRE_MODIFY:
-      case POST_MODIFY:
-	if (rtx_equal_p (reg, XEXP (x, 0)))
-	  return 1;
-	break;
-
-      default:
-	gcc_unreachable ();
+      const_rtx x = *iter;
+      if (GET_RTX_CLASS (GET_CODE (x)) == RTX_AUTOINC
+	  && rtx_equal_p (reg, XEXP (x, 0)))
+	return true;
     }
-  return -1;
+  return false;
 }
 #endif
 
@@ -3695,7 +3683,7 @@ try_split (rtx pat, rtx trial, int last)
 	    {
 	      rtx reg = XEXP (note, 0);
 	      if (!FIND_REG_INC_NOTE (insn, reg)
-		  && for_each_rtx (&PATTERN (insn), find_auto_inc, reg) > 0)
+		  && find_auto_inc (PATTERN (insn), reg))
 		add_reg_note (insn, REG_INC, reg);
 	    }
 	  break;

Re: [PATCH 21/50] Faster for_each_rtx-like iterators

[Jeff Law]

On 08/03/14 08:07, Richard Sandiford wrote:
> The switch statement in the old code seemed overly cautious.  It's well
> established elsewhere that the first operand of an RTX_AUTOINC is the
> automodified register.  If anyone wanted to add a new code for which that
> wasn't true they should (a) reconsider or (b) go through all RTX_AUTOINCs
> as a precaution.
>
>
> gcc/
> 	* emit-rtl.c: Include rtl-iter.h.
> 	(find_auto_inc): Turn from being a for_each_rtx callback to being
> 	a function that examines each subrtx itself.  Assume the first operand
> 	to an RTX_AUTOINC is the automodified register.
> 	(try_split): Update call accordingly.
OK.

It seems like we ought to document somewhere that all autoincrement RTXs 
increment their first operand.  If you can find a goodplace, please add 
that little doc update as a pre-approved patch.

jeff

[PATCH 22/50] final.c:mark_symbol_refs_as_used

[Richard Sandiford]

gcc/
	* final.c: Include rtl-iter.h.
	(mark_symbol_ref_as_used): Delete.
	(mark_symbol_refs_as_used): Use FOR_EACH_SUBRTX instead of
	for_each_rtx.

Index: gcc/final.c
===================================================================
--- gcc/final.c	2014-08-03 11:25:10.029955339 +0100
+++ gcc/final.c	2014-08-03 11:25:26.329116482 +0100
@@ -81,6 +81,7 @@ Software Foundation; either version 3, o
 #include "tree-pretty-print.h" /* for dump_function_header */
 #include "asan.h"
 #include "wide-int-print.h"
+#include "rtl-iter.h"
 
 #ifdef XCOFF_DEBUGGING_INFO
 #include "xcoffout.h"		/* Needed for external data
@@ -3774,38 +3775,19 @@ output_asm_label (rtx x)
   assemble_name (asm_out_file, buf);
 }
 
-/* Helper rtx-iteration-function for mark_symbol_refs_as_used and
-   output_operand.  Marks SYMBOL_REFs as referenced through use of
-   assemble_external.  */
-
-static int
-mark_symbol_ref_as_used (rtx *xp, void *dummy ATTRIBUTE_UNUSED)
-{
-  rtx x = *xp;
-
-  /* If we have a used symbol, we may have to emit assembly
-     annotations corresponding to whether the symbol is external, weak
-     or has non-default visibility.  */
-  if (GET_CODE (x) == SYMBOL_REF)
-    {
-      tree t;
-
-      t = SYMBOL_REF_DECL (x);
-      if (t)
-	assemble_external (t);
-
-      return -1;
-    }
-
-  return 0;
-}
-
 /* Marks SYMBOL_REFs in x as referenced through use of assemble_external.  */
 
 void
 mark_symbol_refs_as_used (rtx x)
 {
-  for_each_rtx (&x, mark_symbol_ref_as_used, NULL);
+  subrtx_iterator::array_type array;
+  FOR_EACH_SUBRTX (iter, array, x, ALL)
+    {
+      const_rtx x = *iter;
+      if (GET_CODE (x) == SYMBOL_REF)
+	if (tree t = SYMBOL_REF_DECL (x))
+	  assemble_external (t);
+    }
 }
 
 /* Print operand X using machine-dependent assembler syntax.
@@ -3831,7 +3813,7 @@ output_operand (rtx x, int code ATTRIBUT
   if (x == NULL_RTX)
     return;
 
-  for_each_rtx (&x, mark_symbol_ref_as_used, NULL);
+  mark_symbol_refs_as_used (x);
 }
 
 /* Print a memory reference operand for address X using

Re: [PATCH 22/50] final.c:mark_symbol_refs_as_used

[Jeff Law]

On 08/03/14 08:08, Richard Sandiford wrote:
> gcc/
> 	* final.c: Include rtl-iter.h.
> 	(mark_symbol_ref_as_used): Delete.
> 	(mark_symbol_refs_as_used): Use FOR_EACH_SUBRTX instead of
> 	for_each_rtx.
OK.
jeff

[PATCH 23/50] function.c:instantiate_virtual_regs_in_rtx

[Richard Sandiford]

gcc/
	* function.c: Include rtl-iter.h
	(instantiate_virtual_regs_in_rtx): Turn from being a for_each_rtx
	callback to being a function that examines each subrtx itself.
	Return the changed flag.
	(instantiate_virtual_regs_in_insn, instantiate_decl_rtl)
	(instantiate_virtual_regs): Update calls accordingly.

Index: gcc/function.c
===================================================================
--- gcc/function.c	2014-08-03 11:25:10.011955161 +0100
+++ gcc/function.c	2014-08-03 11:25:26.606119220 +0100
@@ -65,6 +65,7 @@ Software Foundation; either version 3, o
 #include "bb-reorder.h"
 #include "shrink-wrap.h"
 #include "toplev.h"
+#include "rtl-iter.h"
 
 /* So we can assign to cfun in this file.  */
 #undef cfun
@@ -1431,57 +1432,60 @@ instantiate_new_reg (rtx x, HOST_WIDE_IN
   return new_rtx;
 }
 
-/* A subroutine of instantiate_virtual_regs, called via for_each_rtx.
-   Instantiate any virtual registers present inside of *LOC.  The expression
-   is simplified, as much as possible, but is not to be considered "valid"
-   in any sense implied by the target.  If any change is made, set CHANGED
-   to true.  */
+/* A subroutine of instantiate_virtual_regs.  Instantiate any virtual
+   registers present inside of *LOC.  The expression is simplified,
+   as much as possible, but is not to be considered "valid" in any sense
+   implied by the target.  Return true if any change is made.  */
 
-static int
-instantiate_virtual_regs_in_rtx (rtx *loc, void *data)
+static bool
+instantiate_virtual_regs_in_rtx (rtx *loc)
 {
-  HOST_WIDE_INT offset;
-  bool *changed = (bool *) data;
-  rtx x, new_rtx;
+  if (!*loc)
+    return false;
+  bool changed = false;
+  subrtx_ptr_iterator::array_type array;
+  FOR_EACH_SUBRTX_PTR (iter, array, loc, NONCONST)
+    {
+      rtx *loc = *iter;
+      if (rtx x = *loc)
+	{
+	  rtx new_rtx;
+	  HOST_WIDE_INT offset;
+	  switch (GET_CODE (x))
+	    {
+	    case REG:
+	      new_rtx = instantiate_new_reg (x, &offset);
+	      if (new_rtx)
+		{
+		  *loc = plus_constant (GET_MODE (x), new_rtx, offset);
+		  changed = true;
+		}
+	      iter.skip_subrtxes ();
+	      break;
 
-  x = *loc;
-  if (x == 0)
-    return 0;
+	    case PLUS:
+	      new_rtx = instantiate_new_reg (XEXP (x, 0), &offset);
+	      if (new_rtx)
+		{
+		  XEXP (x, 0) = new_rtx;
+		  *loc = plus_constant (GET_MODE (x), x, offset, true);
+		  changed = true;
+		  iter.skip_subrtxes ();
+		  break;
+		}
 
-  switch (GET_CODE (x))
-    {
-    case REG:
-      new_rtx = instantiate_new_reg (x, &offset);
-      if (new_rtx)
-	{
-	  *loc = plus_constant (GET_MODE (x), new_rtx, offset);
-	  if (changed)
-	    *changed = true;
-	}
-      return -1;
+	      /* FIXME -- from old code */
+	      /* If we have (plus (subreg (virtual-reg)) (const_int)), we know
+		 we can commute the PLUS and SUBREG because pointers into the
+		 frame are well-behaved.  */
+	      break;
 
-    case PLUS:
-      new_rtx = instantiate_new_reg (XEXP (x, 0), &offset);
-      if (new_rtx)
-	{
-	  XEXP (x, 0) = new_rtx;
-	  *loc = plus_constant (GET_MODE (x), x, offset, true);
-	  if (changed)
-	    *changed = true;
-	  return -1;
+	    default:
+	      break;
+	    }
 	}
-
-      /* FIXME -- from old code */
-	  /* If we have (plus (subreg (virtual-reg)) (const_int)), we know
-	     we can commute the PLUS and SUBREG because pointers into the
-	     frame are well-behaved.  */
-      break;
-
-    default:
-      break;
     }
-
-  return 0;
+  return changed;
 }
 
 /* A subroutine of instantiate_virtual_regs_in_insn.  Return true if X
@@ -1517,7 +1521,7 @@ instantiate_virtual_regs_in_insn (rtx in
 	{
 	  start_sequence ();
 
-	  for_each_rtx (&SET_SRC (set), instantiate_virtual_regs_in_rtx, NULL);
+	  instantiate_virtual_regs_in_rtx (&SET_SRC (set));
 	  x = simplify_gen_binary (PLUS, GET_MODE (new_rtx), SET_SRC (set),
 				   gen_int_mode (-offset, GET_MODE (new_rtx)));
 	  x = force_operand (x, new_rtx);
@@ -1620,10 +1624,8 @@ instantiate_virtual_regs_in_insn (rtx in
 	case MEM:
 	  {
 	    rtx addr = XEXP (x, 0);
-	    bool changed = false;
 
-	    for_each_rtx (&addr, instantiate_virtual_regs_in_rtx, &changed);
-	    if (!changed)
+	    if (!instantiate_virtual_regs_in_rtx (&addr))
 	      continue;
 
 	    start_sequence ();
@@ -1789,7 +1791,7 @@ instantiate_decl_rtl (rtx x)
 	      || REGNO (addr) > LAST_VIRTUAL_REGISTER)))
     return;
 
-  for_each_rtx (&XEXP (x, 0), instantiate_virtual_regs_in_rtx, NULL);
+  instantiate_virtual_regs_in_rtx (&XEXP (x, 0));
 }
 
 /* Helper for instantiate_decls called via walk_tree: Process all decls
@@ -1926,20 +1928,18 @@ instantiate_virtual_regs (void)
 	    || GET_CODE (PATTERN (insn)) == ASM_INPUT)
 	  continue;
 	else if (DEBUG_INSN_P (insn))
-	  for_each_rtx (&INSN_VAR_LOCATION (insn),
-			instantiate_virtual_regs_in_rtx, NULL);
+	  instantiate_virtual_regs_in_rtx (&INSN_VAR_LOCATION (insn));
 	else
 	  instantiate_virtual_regs_in_insn (insn);
 
 	if (INSN_DELETED_P (insn))
 	  continue;
 
-	for_each_rtx (&REG_NOTES (insn), instantiate_virtual_regs_in_rtx, NULL);
+	instantiate_virtual_regs_in_rtx (&REG_NOTES (insn));
 
 	/* Instantiate any virtual registers in CALL_INSN_FUNCTION_USAGE.  */
 	if (CALL_P (insn))
-	  for_each_rtx (&CALL_INSN_FUNCTION_USAGE (insn),
-			instantiate_virtual_regs_in_rtx, NULL);
+	  instantiate_virtual_regs_in_rtx (&CALL_INSN_FUNCTION_USAGE (insn));
       }
 
   /* Instantiate the virtual registers in the DECLs for debugging purposes.  */

Re: [PATCH 23/50] function.c:instantiate_virtual_regs_in_rtx

[Jeff Law]

On 08/03/14 08:09, Richard Sandiford wrote:
> gcc/
> 	* function.c: Include rtl-iter.h
> 	(instantiate_virtual_regs_in_rtx): Turn from being a for_each_rtx
> 	callback to being a function that examines each subrtx itself.
> 	Return the changed flag.
> 	(instantiate_virtual_regs_in_insn, instantiate_decl_rtl)
> 	(instantiate_virtual_regs): Update calls accordingly.
OK.
Jeff

[PATCH 24/50] fwprop.c:varying_mem_p

[Richard Sandiford]

gcc/
	* fwprop.c: Include rtl-iter.h.
	(varying_mem_p): Turn from being a for_each_rtx callback to being
	a function that examines each subrtx itself.
	(propagate_rtx): Update accordingly.

Index: gcc/fwprop.c
===================================================================
--- gcc/fwprop.c	2014-08-03 11:25:09.992954973 +0100
+++ gcc/fwprop.c	2014-08-03 11:25:26.890122028 +0100
@@ -38,6 +38,7 @@ Software Foundation; either version 3, o
 #include "tree-pass.h"
 #include "domwalk.h"
 #include "emit-rtl.h"
+#include "rtl-iter.h"
 
 
 /* This pass does simple forward propagation and simplification when an
@@ -623,14 +624,16 @@ propagate_rtx_1 (rtx *px, rtx old_rtx, r
 }
 
 
-/* for_each_rtx traversal function that returns 1 if BODY points to
-   a non-constant mem.  */
+/* Return true if X constains a non-constant mem.  */
 
-static int
-varying_mem_p (rtx *body, void *data ATTRIBUTE_UNUSED)
+static bool
+varying_mem_p (const_rtx x)
 {
-  rtx x = *body;
-  return MEM_P (x) && !MEM_READONLY_P (x);
+  subrtx_iterator::array_type array;
+  FOR_EACH_SUBRTX (iter, array, x, NONCONST)
+    if (MEM_P (*iter) && !MEM_READONLY_P (*iter))
+      return true;
+  return false;
 }
 
 
@@ -661,7 +664,7 @@ propagate_rtx (rtx x, enum machine_mode
 	  && (GET_MODE_SIZE (mode)
 	      <= GET_MODE_SIZE (GET_MODE (SUBREG_REG (new_rtx))))))
     flags |= PR_CAN_APPEAR;
-  if (!for_each_rtx (&new_rtx, varying_mem_p, NULL))
+  if (!varying_mem_p (new_rtx))
     flags |= PR_HANDLE_MEM;
 
   if (speed)

Re: [PATCH 24/50] fwprop.c:varying_mem_p

[Jeff Law]

On 08/03/14 08:10, Richard Sandiford wrote:
> gcc/
> 	* fwprop.c: Include rtl-iter.h.
> 	(varying_mem_p): Turn from being a for_each_rtx callback to being
> 	a function that examines each subrtx itself.
> 	(propagate_rtx): Update accordingly.
OK.
jeff

[PATCH 25/50] ira.c:set_paradoxical_subreg

[Richard Sandiford]

gcc/
	* ira.c: Include rtl-iter.h.
	(set_paradoxical_subreg): Turn from being a for_each_rtx callback
	to being a function that examines each subrtx itself.  Remove
	handling of null rtxes.
	(update_equiv_regs): Update call accordingly.

Index: gcc/ira.c
===================================================================
--- gcc/ira.c	2014-08-03 11:25:09.975954805 +0100
+++ gcc/ira.c	2014-08-03 11:25:27.162124717 +0100
@@ -392,6 +392,7 @@ Software Foundation; either version 3, o
 #include "lra.h"
 #include "dce.h"
 #include "dbgcnt.h"
+#include "rtl-iter.h"
 
 struct target_ira default_target_ira;
 struct target_ira_int default_target_ira_int;
@@ -3265,23 +3266,20 @@ no_equiv (rtx reg, const_rtx store ATTRI
 /* Check whether the SUBREG is a paradoxical subreg and set the result
    in PDX_SUBREGS.  */
 
-static int
-set_paradoxical_subreg (rtx *subreg, void *pdx_subregs)
+static void
+set_paradoxical_subreg (rtx insn, bool *pdx_subregs)
 {
-  rtx reg;
-
-  if ((*subreg) == NULL_RTX)
-    return 1;
-  if (GET_CODE (*subreg) != SUBREG)
-    return 0;
-  reg = SUBREG_REG (*subreg);
-  if (!REG_P (reg))
-    return 0;
-
-  if (paradoxical_subreg_p (*subreg))
-    ((bool *)pdx_subregs)[REGNO (reg)] = true;
-
-  return 0;
+  subrtx_iterator::array_type array;
+  FOR_EACH_SUBRTX (iter, array, PATTERN (insn), NONCONST)
+    {
+      const_rtx subreg = *iter;
+      if (GET_CODE (subreg) == SUBREG)
+	{
+	  const_rtx reg = SUBREG_REG (subreg);
+	  if (REG_P (reg) && paradoxical_subreg_p (subreg))
+	    pdx_subregs[REGNO (reg)] = true;
+	}
+    }
 }
 
 /* In DEBUG_INSN location adjust REGs from CLEARED_REGS bitmap to the
@@ -3344,7 +3342,7 @@ update_equiv_regs (void)
   FOR_EACH_BB_FN (bb, cfun)
     FOR_BB_INSNS (bb, insn)
       if (NONDEBUG_INSN_P (insn))
-	for_each_rtx (&insn, set_paradoxical_subreg, (void *) pdx_subregs);
+	set_paradoxical_subreg (insn, pdx_subregs);
 
   /* Scan the insns and find which registers have equivalences.  Do this
      in a separate scan of the insns because (due to -fcse-follow-jumps)

Re: [PATCH 25/50] ira.c:set_paradoxical_subreg

[Jeff Law]

On 08/03/14 08:10, Richard Sandiford wrote:
> gcc/
> 	* ira.c: Include rtl-iter.h.
> 	(set_paradoxical_subreg): Turn from being a for_each_rtx callback
> 	to being a function that examines each subrtx itself.  Remove
> 	handling of null rtxes.
> 	(update_equiv_regs): Update call accordingly.
OK.
jeff

[PATCH 26/50] jump.c:returnjump_p

[Richard Sandiford]

gcc/
	* jump.c: Include rtl-iter.h.
	(returnjump_p_1): Delete.
	(returnjump_p): Use FOR_EACH_SUBRTX rather than for_each_rtx.
	Remove handling of null rtxes.

Index: gcc/jump.c
===================================================================
--- gcc/jump.c	2014-08-03 11:25:09.957954627 +0100
+++ gcc/jump.c	2014-08-03 11:25:27.442127486 +0100
@@ -54,6 +54,7 @@ Software Foundation; either version 3, o
 #include "predict.h"
 #include "tree-pass.h"
 #include "target.h"
+#include "rtl-iter.h"
 
 /* Optimize jump y; x: ... y: jumpif... x?
    Don't know if it is worth bothering with.  */
@@ -68,7 +69,6 @@ static void mark_jump_label_1 (rtx, rtx,
 static void mark_jump_label_asm (rtx, rtx);
 static void redirect_exp_1 (rtx *, rtx, rtx, rtx);
 static int invert_exp_1 (rtx, rtx);
-static int returnjump_p_1 (rtx *, void *);
 \f
 /* Worker for rebuild_jump_labels and rebuild_jump_labels_chain.  */
 static void
@@ -920,39 +920,35 @@ condjump_label (const_rtx insn)
   return NULL_RTX;
 }
 
-/* Return true if INSN is a (possibly conditional) return insn.  */
-
-static int
-returnjump_p_1 (rtx *loc, void *data ATTRIBUTE_UNUSED)
-{
-  rtx x = *loc;
-
-  if (x == NULL)
-    return false;
-
-  switch (GET_CODE (x))
-    {
-    case RETURN:
-    case SIMPLE_RETURN:
-    case EH_RETURN:
-      return true;
-
-    case SET:
-      return SET_IS_RETURN_P (x);
-
-    default:
-      return false;
-    }
-}
-
 /* Return TRUE if INSN is a return jump.  */
 
 int
 returnjump_p (rtx insn)
 {
-  if (!JUMP_P (insn))
-    return 0;
-  return for_each_rtx (&PATTERN (insn), returnjump_p_1, NULL);
+  if (JUMP_P (insn))
+    {
+      subrtx_iterator::array_type array;
+      FOR_EACH_SUBRTX (iter, array, PATTERN (insn), NONCONST)
+	{
+	  const_rtx x = *iter;
+	  switch (GET_CODE (x))
+	    {
+	    case RETURN:
+	    case SIMPLE_RETURN:
+	    case EH_RETURN:
+	      return true;
+
+	    case SET:
+	      if (SET_IS_RETURN_P (x))
+		return true;
+	      break;
+
+	    default:
+	      break;
+	    }
+	}
+    }
+  return false;
 }
 
 /* Return true if INSN is a (possibly conditional) return insn.  */

Re: [PATCH 26/50] jump.c:returnjump_p

[Jeff Law]

On 08/03/14 08:11, Richard Sandiford wrote:
> gcc/
> 	* jump.c: Include rtl-iter.h.
> 	(returnjump_p_1): Delete.
> 	(returnjump_p): Use FOR_EACH_SUBRTX rather than for_each_rtx.
> 	Remove handling of null rtxes.
OK.
Jeff

[PATCH 27/50] jump.c:eh_returnjump_p

[Richard Sandiford]

gcc/
	* jump.c (eh_returnjump_p_1): Delete.
	(eh_returnjump_p): Use FOR_EACH_SUBRTX rather than for_each_rtx.
	Remove handling of null rtxes.

Index: gcc/jump.c
===================================================================
--- gcc/jump.c	2014-08-03 11:25:27.442127486 +0100
+++ gcc/jump.c	2014-08-03 11:25:27.722130254 +0100
@@ -953,18 +953,17 @@ returnjump_p (rtx insn)
 
 /* Return true if INSN is a (possibly conditional) return insn.  */
 
-static int
-eh_returnjump_p_1 (rtx *loc, void *data ATTRIBUTE_UNUSED)
-{
-  return *loc && GET_CODE (*loc) == EH_RETURN;
-}
-
 int
 eh_returnjump_p (rtx insn)
 {
-  if (!JUMP_P (insn))
-    return 0;
-  return for_each_rtx (&PATTERN (insn), eh_returnjump_p_1, NULL);
+  if (JUMP_P (insn))
+    {
+      subrtx_iterator::array_type array;
+      FOR_EACH_SUBRTX (iter, array, PATTERN (insn), NONCONST)
+	if (GET_CODE (*iter) == EH_RETURN)
+	  return true;
+    }
+  return false;
 }
 
 /* Return true if INSN is a jump that only transfers control and

Re: [PATCH 27/50] jump.c:eh_returnjump_p

[Jeff Law]

On 08/03/14 08:12, Richard Sandiford wrote:
> gcc/
> 	* jump.c (eh_returnjump_p_1): Delete.
> 	(eh_returnjump_p): Use FOR_EACH_SUBRTX rather than for_each_rtx.
> 	Remove handling of null rtxes.
OK.
jeff

[PATCH 28/50] loop-iv.c:replace_single_def_regs

[Richard Sandiford]

gcc/
	* loop-iv.c: Include rtl-iter.h.
	(find_single_def_src): New function.
	(replace_single_def_regs): Turn from being a for_each_rtx callback
	to being a function that examines each subrtx itself.
	(replace_in_expr, simplify_using_initial_values): Update accordingly.

Index: gcc/loop-iv.c
===================================================================
--- gcc/loop-iv.c	2014-08-03 11:25:09.905954113 +0100
+++ gcc/loop-iv.c	2014-08-03 11:25:27.978132785 +0100
@@ -62,6 +62,7 @@ Free Software Foundation; either version
 #include "df.h"
 #include "hash-table.h"
 #include "dumpfile.h"
+#include "rtl-iter.h"
 
 /* Possible return values of iv_get_reaching_def.  */
 
@@ -1397,33 +1398,27 @@ simple_rhs_p (rtx rhs)
     }
 }
 
-/* If REG has a single definition, replace it with its known value in EXPR.
-   Callback for for_each_rtx.  */
+/* If REGNO has a single definition, return its known value, otherwise return
+   null.  */
 
-static int
-replace_single_def_regs (rtx *reg, void *expr1)
+static rtx
+find_single_def_src (unsigned int regno)
 {
-  unsigned regno;
   df_ref adef;
   rtx set, src;
-  rtx *expr = (rtx *)expr1;
 
-  if (!REG_P (*reg))
-    return 0;
-
-  regno = REGNO (*reg);
   for (;;)
     {
       rtx note;
       adef = DF_REG_DEF_CHAIN (regno);
       if (adef == NULL || DF_REF_NEXT_REG (adef) != NULL
-	    || DF_REF_IS_ARTIFICIAL (adef))
-	return -1;
+	  || DF_REF_IS_ARTIFICIAL (adef))
+	return NULL_RTX;
 
       set = single_set (DF_REF_INSN (adef));
       if (set == NULL || !REG_P (SET_DEST (set))
 	  || REGNO (SET_DEST (set)) != regno)
-	return -1;
+	return NULL_RTX;
 
       note = find_reg_equal_equiv_note (DF_REF_INSN (adef));
 
@@ -1442,10 +1437,29 @@ replace_single_def_regs (rtx *reg, void
       break;
     }
   if (!function_invariant_p (src))
-    return -1;
+    return NULL_RTX;
+
+  return src;
+}
+
+/* If any registers in *EXPR that have a single definition, try to replace
+   them with the known-equivalent values.  */
 
-  *expr = simplify_replace_rtx (*expr, *reg, src);
-  return 1;
+static void
+replace_single_def_regs (rtx *expr)
+{
+  subrtx_var_iterator::array_type array;
+ repeat:
+  FOR_EACH_SUBRTX_VAR (iter, array, *expr, NONCONST)
+    {
+      rtx x = *iter;
+      if (REG_P (x))
+	if (rtx new_x = find_single_def_src (REGNO (x)))
+	  {
+	    *expr = simplify_replace_rtx (*expr, x, new_x);
+	    goto repeat;
+	  }
+    }
 }
 
 /* A subroutine of simplify_using_initial_values, this function examines INSN
@@ -1490,8 +1504,7 @@ replace_in_expr (rtx *expr, rtx dest, rt
   *expr = simplify_replace_rtx (*expr, dest, src);
   if (old == *expr)
     return;
-  while (for_each_rtx (expr, replace_single_def_regs, expr) != 0)
-    continue;
+  replace_single_def_regs (expr);
 }
 
 /* Checks whether A implies B.  */
@@ -1934,9 +1947,7 @@ simplify_using_initial_values (struct lo
 
   gcc_assert (op == UNKNOWN);
 
-  for (;;)
-    if (for_each_rtx (expr, replace_single_def_regs, expr) == 0)
-      break;
+  replace_single_def_regs (expr);
   if (CONSTANT_P (*expr))
     return;
 

Re: [PATCH 28/50] loop-iv.c:replace_single_def_regs

[Jeff Law]

On 08/03/14 08:13, Richard Sandiford wrote:
> gcc/
> 	* loop-iv.c: Include rtl-iter.h.
> 	(find_single_def_src): New function.
> 	(replace_single_def_regs): Turn from being a for_each_rtx callback
> 	to being a function that examines each subrtx itself.
> 	(replace_in_expr, simplify_using_initial_values): Update accordingly.
OK.
Jeff

[PATCH 29/50] loop-iv.c:altered_reg_used

[Richard Sandiford]

gcc/
	* loop-iv.c (altered_reg_used): Turn from being a for_each_rtx callback
	to being a function that examines each subrtx itself.
	(simplify_using_condition, simplify_using_initial_values): Update
	accordingly.

Index: gcc/loop-iv.c
===================================================================
--- gcc/loop-iv.c	2014-08-03 11:25:27.978132785 +0100
+++ gcc/loop-iv.c	2014-08-03 11:25:28.247135444 +0100
@@ -1327,15 +1327,19 @@ inverse (uint64_t x, int mod)
   return rslt;
 }
 
-/* Checks whether register *REG is in set ALT.  Callback for for_each_rtx.  */
+/* Checks whether any register in X is in set ALT.  */
 
-static int
-altered_reg_used (rtx *reg, void *alt)
+static bool
+altered_reg_used (const_rtx x, bitmap alt)
 {
-  if (!REG_P (*reg))
-    return 0;
-
-  return REGNO_REG_SET_P ((bitmap) alt, REGNO (*reg));
+  subrtx_iterator::array_type array;
+  FOR_EACH_SUBRTX (iter, array, x, NONCONST)
+    {
+      const_rtx x = *iter;
+      if (REG_P (x) && REGNO_REG_SET_P (alt, REGNO (x)))
+	return true;
+    }
+  return false;
 }
 
 /* Marks registers altered by EXPR in set ALT.  */
@@ -1771,8 +1775,7 @@ simplify_using_condition (rtx cond, rtx
 
   /* If some register gets altered later, we do not really speak about its
      value at the time of comparison.  */
-  if (altered
-      && for_each_rtx (&cond, altered_reg_used, altered))
+  if (altered && altered_reg_used (cond, altered))
     return;
 
   if (GET_CODE (cond) == EQ
@@ -2048,7 +2051,7 @@ simplify_using_initial_values (struct lo
 	      /* If we did not use this insn to make a replacement, any overlap
 		 between stores in this insn and our expression will cause the
 		 expression to become invalid.  */
-	      if (for_each_rtx (expr, altered_reg_used, this_altered))
+	      if (altered_reg_used (*expr, this_altered))
 		goto out;
 
 	      /* Likewise for the conditions.  */
@@ -2058,7 +2061,7 @@ simplify_using_initial_values (struct lo
 		  rtx old_cond = XEXP (note, 0);
 
 		  pnote_next = &XEXP (note, 1);
-		  if (for_each_rtx (&old_cond, altered_reg_used, this_altered))
+		  if (altered_reg_used (old_cond, this_altered))
 		    {
 		      *pnote = *pnote_next;
 		      pnote_next = pnote;
@@ -2076,7 +2079,7 @@ simplify_using_initial_values (struct lo
 	     can't return it to the caller.  However, it is still valid for
 	     further simplification, so keep searching to see if we can
 	     eventually turn it into a constant.  */
-	  if (for_each_rtx (expr, altered_reg_used, altered))
+	  if (altered_reg_used (*expr, altered))
 	    expression_valid = false;
 	  if (expression_valid)
 	    last_valid_expr = *expr;

Re: [PATCH 29/50] loop-iv.c:altered_reg_used

[Jeff Law]

On 08/03/14 08:14, Richard Sandiford wrote:
> gcc/
> 	* loop-iv.c (altered_reg_used): Turn from being a for_each_rtx callback
> 	to being a function that examines each subrtx itself.
> 	(simplify_using_condition, simplify_using_initial_values): Update
> 	accordingly.
Ok.
jeff

[PATCH 30/50] lower-subreg.c:resolve_subreg_use

[Richard Sandiford]

gcc/
	* lower-subreg.c: Include rtl-iter.h.
	(resolve_subreg_use): Turn from being a for_each_rtx callback
	to being a function that examines each subrtx itself.  Remove
	handling of null rtxes.
	(resolve_reg_notes, resolve_simple_move): Update accordingly.
	(decompose_multiword_subregs): Likewise.

Index: gcc/lower-subreg.c
===================================================================
--- gcc/lower-subreg.c	2014-08-03 11:25:09.853953599 +0100
+++ gcc/lower-subreg.c	2014-08-03 11:25:28.502137966 +0100
@@ -40,6 +40,7 @@ Software Foundation; either version 3, o
 #include "tree-pass.h"
 #include "df.h"
 #include "lower-subreg.h"
+#include "rtl-iter.h"
 
 #ifdef STACK_GROWS_DOWNWARD
 # undef STACK_GROWS_DOWNWARD
@@ -726,46 +727,42 @@ resolve_subreg_p (rtx x)
   return resolve_reg_p (SUBREG_REG (x));
 }
 
-/* This is called via for_each_rtx.  Look for SUBREGs which need to be
-   decomposed.  */
+/* Look for SUBREGs in *LOC which need to be decomposed.  */
 
-static int
-resolve_subreg_use (rtx *px, void *data)
+static bool
+resolve_subreg_use (rtx *loc, rtx insn)
 {
-  rtx insn = (rtx) data;
-  rtx x = *px;
-
-  if (x == NULL_RTX)
-    return 0;
-
-  if (resolve_subreg_p (x))
+  subrtx_ptr_iterator::array_type array;
+  FOR_EACH_SUBRTX_PTR (iter, array, loc, NONCONST)
     {
-      x = simplify_subreg_concatn (GET_MODE (x), SUBREG_REG (x),
-				   SUBREG_BYTE (x));
-
-      /* It is possible for a note to contain a reference which we can
-	 decompose.  In this case, return 1 to the caller to indicate
-	 that the note must be removed.  */
-      if (!x)
+      rtx *loc = *iter;
+      rtx x = *loc;
+      if (resolve_subreg_p (x))
 	{
-	  gcc_assert (!insn);
-	  return 1;
-	}
+	  x = simplify_subreg_concatn (GET_MODE (x), SUBREG_REG (x),
+				       SUBREG_BYTE (x));
 
-      validate_change (insn, px, x, 1);
-      return -1;
-    }
+	  /* It is possible for a note to contain a reference which we can
+	     decompose.  In this case, return 1 to the caller to indicate
+	     that the note must be removed.  */
+	  if (!x)
+	    {
+	      gcc_assert (!insn);
+	      return true;
+	    }
 
-  if (resolve_reg_p (x))
-    {
-      /* Return 1 to the caller to indicate that we found a direct
-	 reference to a register which is being decomposed.  This can
-	 happen inside notes, multiword shift or zero-extend
-	 instructions.  */
-      return 1;
+	  validate_change (insn, loc, x, 1);
+	  iter.skip_subrtxes ();
+	}
+      else if (resolve_reg_p (x))
+	/* Return 1 to the caller to indicate that we found a direct
+	   reference to a register which is being decomposed.  This can
+	   happen inside notes, multiword shift or zero-extend
+	   instructions.  */
+	return true;
     }
 
-  return 0;
+  return false;
 }
 
 /* This is called via for_each_rtx.  Look for SUBREGs which can be
@@ -808,7 +805,7 @@ resolve_reg_notes (rtx insn)
   if (note)
     {
       int old_count = num_validated_changes ();
-      if (for_each_rtx (&XEXP (note, 0), resolve_subreg_use, NULL))
+      if (resolve_subreg_use (&XEXP (note, 0), NULL_RTX))
 	remove_note (insn, note);
       else
 	if (old_count != num_validated_changes ())
@@ -950,9 +947,9 @@ resolve_simple_move (rtx set, rtx insn)
       int acg;
 
       if (MEM_P (src))
-	for_each_rtx (&XEXP (src, 0), resolve_subreg_use, NULL_RTX);
+	resolve_subreg_use (&XEXP (src, 0), NULL_RTX);
       if (MEM_P (dest))
-	for_each_rtx (&XEXP (dest, 0), resolve_subreg_use, NULL_RTX);
+	resolve_subreg_use (&XEXP (dest, 0), NULL_RTX);
       acg = apply_change_group ();
       gcc_assert (acg);
     }
@@ -1614,9 +1611,7 @@ decompose_multiword_subregs (bool decomp
 		    }
 
 		  for (i = recog_data.n_operands - 1; i >= 0; --i)
-		    for_each_rtx (recog_data.operand_loc[i],
-				  resolve_subreg_use,
-				  insn);
+		    resolve_subreg_use (recog_data.operand_loc[i], insn);
 
 		  resolve_reg_notes (insn);
 

Re: [PATCH 30/50] lower-subreg.c:resolve_subreg_use

[Jeff Law]

On 08/03/14 08:15, Richard Sandiford wrote:
> gcc/
> 	* lower-subreg.c: Include rtl-iter.h.
> 	(resolve_subreg_use): Turn from being a for_each_rtx callback
> 	to being a function that examines each subrtx itself.  Remove
> 	handling of null rtxes.
> 	(resolve_reg_notes, resolve_simple_move): Update accordingly.
> 	(decompose_multiword_subregs): Likewise.
OK.
Jeff

[PATCH 31/50] lower-subreg.c:resolve_debug

[Richard Sandiford]

gcc/
	* lower-subreg.c (adjust_decomposed_uses): Delete.
	(resolve_debug): Use FOR_EACH_SUBRTX_PTR rather than for_each_rtx.
	Remove handling of null rtxes.

Index: gcc/lower-subreg.c
===================================================================
--- gcc/lower-subreg.c	2014-08-03 11:25:28.502137966 +0100
+++ gcc/lower-subreg.c	2014-08-03 11:25:28.779140704 +0100
@@ -765,34 +765,6 @@ resolve_subreg_use (rtx *loc, rtx insn)
   return false;
 }
 
-/* This is called via for_each_rtx.  Look for SUBREGs which can be
-   decomposed and decomposed REGs that need copying.  */
-
-static int
-adjust_decomposed_uses (rtx *px, void *data ATTRIBUTE_UNUSED)
-{
-  rtx x = *px;
-
-  if (x == NULL_RTX)
-    return 0;
-
-  if (resolve_subreg_p (x))
-    {
-      x = simplify_subreg_concatn (GET_MODE (x), SUBREG_REG (x),
-				   SUBREG_BYTE (x));
-
-      if (x)
-	*px = x;
-      else
-	x = copy_rtx (*px);
-    }
-
-  if (resolve_reg_p (x))
-    *px = copy_rtx (x);
-
-  return 0;
-}
-
 /* Resolve any decomposed registers which appear in register notes on
    INSN.  */
 
@@ -1164,7 +1136,24 @@ resolve_use (rtx pat, rtx insn)
 static void
 resolve_debug (rtx insn)
 {
-  for_each_rtx (&PATTERN (insn), adjust_decomposed_uses, NULL_RTX);
+  subrtx_ptr_iterator::array_type array;
+  FOR_EACH_SUBRTX_PTR (iter, array, &PATTERN (insn), NONCONST)
+    {
+      rtx *loc = *iter;
+      rtx x = *loc;
+      if (resolve_subreg_p (x))
+	{
+	  x = simplify_subreg_concatn (GET_MODE (x), SUBREG_REG (x),
+				       SUBREG_BYTE (x));
+
+	  if (x)
+	    *loc = x;
+	  else
+	    x = copy_rtx (*loc);
+	}
+      if (resolve_reg_p (x))
+	*loc = copy_rtx (x);
+    }
 
   df_insn_rescan (insn);
 

Re: [PATCH 31/50] lower-subreg.c:resolve_debug

[Jeff Law]

On 08/03/14 08:15, Richard Sandiford wrote:
> gcc/
> 	* lower-subreg.c (adjust_decomposed_uses): Delete.
> 	(resolve_debug): Use FOR_EACH_SUBRTX_PTR rather than for_each_rtx.
> 	Remove handling of null rtxes.
OK.
jeff

[PATCH 32/50] lower-subreg.c:find_decomposable_subregs

[Richard Sandiford]

Mostly just reindentation.


gcc/
	* lower-subreg.c (find_decomposable_subregs): Turn from being
	a for_each_rtx callback to being a function that examines each
	subrtx itself.  Remove handling of null rtxes.
	(decompose_multiword_subregs): Update accordingly.

Index: gcc/lower-subreg.c
===================================================================
--- gcc/lower-subreg.c	2014-08-03 11:25:28.779140704 +0100
+++ gcc/lower-subreg.c	2014-08-03 11:25:29.052143403 +0100
@@ -443,7 +443,7 @@ propagate_pseudo_copies (void)
 }
 
 /* A pointer to one of these values is passed to
-   find_decomposable_subregs via for_each_rtx.  */
+   find_decomposable_subregs.  */
 
 enum classify_move_insn
 {
@@ -455,120 +455,121 @@ enum classify_move_insn
   SIMPLE_MOVE
 };
 
-/* This is called via for_each_rtx.  If we find a SUBREG which we
-   could use to decompose a pseudo-register, set a bit in
-   DECOMPOSABLE_CONTEXT.  If we find an unadorned register which is
-   not a simple pseudo-register copy, DATA will point at the type of
-   move, and we set a bit in DECOMPOSABLE_CONTEXT or
-   NON_DECOMPOSABLE_CONTEXT as appropriate.  */
+/* If we find a SUBREG in *LOC which we could use to decompose a
+   pseudo-register, set a bit in DECOMPOSABLE_CONTEXT.  If we find an
+   unadorned register which is not a simple pseudo-register copy,
+   DATA will point at the type of move, and we set a bit in
+   DECOMPOSABLE_CONTEXT or NON_DECOMPOSABLE_CONTEXT as appropriate.  */
 
-static int
-find_decomposable_subregs (rtx *px, void *data)
+static void
+find_decomposable_subregs (rtx *loc, enum classify_move_insn *pcmi)
 {
-  enum classify_move_insn *pcmi = (enum classify_move_insn *) data;
-  rtx x = *px;
-
-  if (x == NULL_RTX)
-    return 0;
-
-  if (GET_CODE (x) == SUBREG)
+  subrtx_var_iterator::array_type array;
+  FOR_EACH_SUBRTX_VAR (iter, array, *loc, NONCONST)
     {
-      rtx inner = SUBREG_REG (x);
-      unsigned int regno, outer_size, inner_size, outer_words, inner_words;
+      rtx x = *iter;
+      if (GET_CODE (x) == SUBREG)
+	{
+	  rtx inner = SUBREG_REG (x);
+	  unsigned int regno, outer_size, inner_size, outer_words, inner_words;
 
-      if (!REG_P (inner))
-	return 0;
+	  if (!REG_P (inner))
+	    continue;
 
-      regno = REGNO (inner);
-      if (HARD_REGISTER_NUM_P (regno))
-	return -1;
+	  regno = REGNO (inner);
+	  if (HARD_REGISTER_NUM_P (regno))
+	    {
+	      iter.skip_subrtxes ();
+	      continue;
+	    }
 
-      outer_size = GET_MODE_SIZE (GET_MODE (x));
-      inner_size = GET_MODE_SIZE (GET_MODE (inner));
-      outer_words = (outer_size + UNITS_PER_WORD - 1) / UNITS_PER_WORD;
-      inner_words = (inner_size + UNITS_PER_WORD - 1) / UNITS_PER_WORD;
-
-      /* We only try to decompose single word subregs of multi-word
-	 registers.  When we find one, we return -1 to avoid iterating
-	 over the inner register.
-
-	 ??? This doesn't allow, e.g., DImode subregs of TImode values
-	 on 32-bit targets.  We would need to record the way the
-	 pseudo-register was used, and only decompose if all the uses
-	 were the same number and size of pieces.  Hopefully this
-	 doesn't happen much.  */
+	  outer_size = GET_MODE_SIZE (GET_MODE (x));
+	  inner_size = GET_MODE_SIZE (GET_MODE (inner));
+	  outer_words = (outer_size + UNITS_PER_WORD - 1) / UNITS_PER_WORD;
+	  inner_words = (inner_size + UNITS_PER_WORD - 1) / UNITS_PER_WORD;
+
+	  /* We only try to decompose single word subregs of multi-word
+	     registers.  When we find one, we return -1 to avoid iterating
+	     over the inner register.
+
+	     ??? This doesn't allow, e.g., DImode subregs of TImode values
+	     on 32-bit targets.  We would need to record the way the
+	     pseudo-register was used, and only decompose if all the uses
+	     were the same number and size of pieces.  Hopefully this
+	     doesn't happen much.  */
 
-      if (outer_words == 1 && inner_words > 1)
-	{
-	  bitmap_set_bit (decomposable_context, regno);
-	  return -1;
-	}
+	  if (outer_words == 1 && inner_words > 1)
+	    {
+	      bitmap_set_bit (decomposable_context, regno);
+	      iter.skip_subrtxes ();
+	      continue;
+	    }
 
-      /* If this is a cast from one mode to another, where the modes
-	 have the same size, and they are not tieable, then mark this
-	 register as non-decomposable.  If we decompose it we are
-	 likely to mess up whatever the backend is trying to do.  */
-      if (outer_words > 1
-	  && outer_size == inner_size
-	  && !MODES_TIEABLE_P (GET_MODE (x), GET_MODE (inner)))
-	{
-	  bitmap_set_bit (non_decomposable_context, regno);
-	  bitmap_set_bit (subreg_context, regno);
-	  return -1;
+	  /* If this is a cast from one mode to another, where the modes
+	     have the same size, and they are not tieable, then mark this
+	     register as non-decomposable.  If we decompose it we are
+	     likely to mess up whatever the backend is trying to do.  */
+	  if (outer_words > 1
+	      && outer_size == inner_size
+	      && !MODES_TIEABLE_P (GET_MODE (x), GET_MODE (inner)))
+	    {
+	      bitmap_set_bit (non_decomposable_context, regno);
+	      bitmap_set_bit (subreg_context, regno);
+	      iter.skip_subrtxes ();
+	      continue;
+	    }
 	}
-    }
-  else if (REG_P (x))
-    {
-      unsigned int regno;
-
-      /* We will see an outer SUBREG before we see the inner REG, so
-	 when we see a plain REG here it means a direct reference to
-	 the register.
-
-	 If this is not a simple copy from one location to another,
-	 then we can not decompose this register.  If this is a simple
-	 copy we want to decompose, and the mode is right,
-	 then we mark the register as decomposable.
-	 Otherwise we don't say anything about this register --
-	 it could be decomposed, but whether that would be
-	 profitable depends upon how it is used elsewhere.
-
-	 We only set bits in the bitmap for multi-word
-	 pseudo-registers, since those are the only ones we care about
-	 and it keeps the size of the bitmaps down.  */
-
-      regno = REGNO (x);
-      if (!HARD_REGISTER_NUM_P (regno)
-	  && GET_MODE_SIZE (GET_MODE (x)) > UNITS_PER_WORD)
+      else if (REG_P (x))
 	{
-	  switch (*pcmi)
+	  unsigned int regno;
+
+	  /* We will see an outer SUBREG before we see the inner REG, so
+	     when we see a plain REG here it means a direct reference to
+	     the register.
+
+	     If this is not a simple copy from one location to another,
+	     then we can not decompose this register.  If this is a simple
+	     copy we want to decompose, and the mode is right,
+	     then we mark the register as decomposable.
+	     Otherwise we don't say anything about this register --
+	     it could be decomposed, but whether that would be
+	     profitable depends upon how it is used elsewhere.
+
+	     We only set bits in the bitmap for multi-word
+	     pseudo-registers, since those are the only ones we care about
+	     and it keeps the size of the bitmaps down.  */
+
+	  regno = REGNO (x);
+	  if (!HARD_REGISTER_NUM_P (regno)
+	      && GET_MODE_SIZE (GET_MODE (x)) > UNITS_PER_WORD)
 	    {
-	    case NOT_SIMPLE_MOVE:
-	      bitmap_set_bit (non_decomposable_context, regno);
-	      break;
-	    case DECOMPOSABLE_SIMPLE_MOVE:
-	      if (MODES_TIEABLE_P (GET_MODE (x), word_mode))
-		bitmap_set_bit (decomposable_context, regno);
-	      break;
-	    case SIMPLE_MOVE:
-	      break;
-	    default:
-	      gcc_unreachable ();
+	      switch (*pcmi)
+		{
+		case NOT_SIMPLE_MOVE:
+		  bitmap_set_bit (non_decomposable_context, regno);
+		  break;
+		case DECOMPOSABLE_SIMPLE_MOVE:
+		  if (MODES_TIEABLE_P (GET_MODE (x), word_mode))
+		    bitmap_set_bit (decomposable_context, regno);
+		  break;
+		case SIMPLE_MOVE:
+		  break;
+		default:
+		  gcc_unreachable ();
+		}
 	    }
 	}
-    }
-  else if (MEM_P (x))
-    {
-      enum classify_move_insn cmi_mem = NOT_SIMPLE_MOVE;
+      else if (MEM_P (x))
+	{
+	  enum classify_move_insn cmi_mem = NOT_SIMPLE_MOVE;
 
-      /* Any registers used in a MEM do not participate in a
-	 SIMPLE_MOVE or DECOMPOSABLE_SIMPLE_MOVE.  Do our own recursion
-	 here, and return -1 to block the parent's recursion.  */
-      for_each_rtx (&XEXP (x, 0), find_decomposable_subregs, &cmi_mem);
-      return -1;
+	  /* Any registers used in a MEM do not participate in a
+	     SIMPLE_MOVE or DECOMPOSABLE_SIMPLE_MOVE.  Do our own recursion
+	     here, and return -1 to block the parent's recursion.  */
+	  find_decomposable_subregs (&XEXP (x, 0), &cmi_mem);
+	  iter.skip_subrtxes ();
+	}
     }
-
-  return 0;
 }
 
 /* Decompose REGNO into word-sized components.  We smash the REG node
@@ -1493,9 +1494,7 @@ decompose_multiword_subregs (bool decomp
 	  n = recog_data.n_operands;
 	  for (i = 0; i < n; ++i)
 	    {
-	      for_each_rtx (&recog_data.operand[i],
-			    find_decomposable_subregs,
-			    &cmi);
+	      find_decomposable_subregs (&recog_data.operand[i], &cmi);
 
 	      /* We handle ASM_OPERANDS as a special case to support
 		 things like x86 rdtsc which returns a DImode value.

Re: [PATCH 32/50] lower-subreg.c:find_decomposable_subregs

[Jeff Law]

On 08/03/14 08:17, Richard Sandiford wrote:
> Mostly just reindentation.
>
>
> gcc/
> 	* lower-subreg.c (find_decomposable_subregs): Turn from being
> 	a for_each_rtx callback to being a function that examines each
> 	subrtx itself.  Remove handling of null rtxes.
> 	(decompose_multiword_subregs): Update accordingly.
OK.
Jeff

[PATCH 33/50] reg-stack.c:subst_all_stack_regs_in_debug_insn

[Richard Sandiford]

gcc/
	* reg-stack.c: Include rtl-iter.h.
	(subst_stack_regs_in_debug_insn): Delete.
	(subst_all_stack_regs_in_debug_insn): Use FOR_EACH_SUBRTX_PTR
	instead of for_each_rtx.

Index: gcc/reg-stack.c
===================================================================
--- gcc/reg-stack.c	2014-08-03 11:25:09.766952738 +0100
+++ gcc/reg-stack.c	2014-08-03 11:25:29.318146033 +0100
@@ -170,6 +170,7 @@
 #include "target.h"
 #include "df.h"
 #include "emit-rtl.h"  /* FIXME: Can go away once crtl is moved to rtl.h.  */
+#include "rtl-iter.h"
 
 #ifdef STACK_REGS
 
@@ -1307,31 +1308,6 @@ compare_for_stack_reg (rtx insn, stack_p
     }
 }
 \f
-/* Substitute new registers in LOC, which is part of a debug insn.
-   REGSTACK is the current register layout.  */
-
-static int
-subst_stack_regs_in_debug_insn (rtx *loc, void *data)
-{
-  stack_ptr regstack = (stack_ptr)data;
-  int hard_regno;
-
-  if (!STACK_REG_P (*loc))
-    return 0;
-
-  hard_regno = get_hard_regnum (regstack, *loc);
-
-  /* If we can't find an active register, reset this debug insn.  */
-  if (hard_regno == -1)
-    return 1;
-
-  gcc_assert (hard_regno >= FIRST_STACK_REG);
-
-  replace_reg (loc, hard_regno);
-
-  return -1;
-}
-
 /* Substitute hardware stack regs in debug insn INSN, using stack
    layout REGSTACK.  If we can't find a hardware stack reg for any of
    the REGs in it, reset the debug insn.  */
@@ -1339,14 +1315,27 @@ subst_stack_regs_in_debug_insn (rtx *loc
 static void
 subst_all_stack_regs_in_debug_insn (rtx insn, struct stack_def *regstack)
 {
-  int ret = for_each_rtx (&INSN_VAR_LOCATION_LOC (insn),
-			  subst_stack_regs_in_debug_insn,
-			  regstack);
+  subrtx_ptr_iterator::array_type array;
+  FOR_EACH_SUBRTX_PTR (iter, array, &INSN_VAR_LOCATION_LOC (insn), NONCONST)
+    {
+      rtx *loc = *iter;
+      rtx x = *loc;
+      if (STACK_REG_P (x))
+	{
+	  int hard_regno = get_hard_regnum (regstack, x);
 
-  if (ret == 1)
-    INSN_VAR_LOCATION_LOC (insn) = gen_rtx_UNKNOWN_VAR_LOC ();
-  else
-    gcc_checking_assert (ret == 0);
+	  /* If we can't find an active register, reset this debug insn.  */
+	  if (hard_regno == -1)
+	    {
+	      INSN_VAR_LOCATION_LOC (insn) = gen_rtx_UNKNOWN_VAR_LOC ();
+	      return;
+	    }
+
+	  gcc_assert (hard_regno >= FIRST_STACK_REG);
+	  replace_reg (loc, hard_regno);
+	  iter.skip_subrtxes ();
+	}
+    }
 }
 
 /* Substitute new registers in PAT, which is part of INSN.  REGSTACK

Re: [PATCH 33/50] reg-stack.c:subst_all_stack_regs_in_debug_insn

[Jeff Law]

On 08/03/14 08:18, Richard Sandiford wrote:
> gcc/
> 	* reg-stack.c: Include rtl-iter.h.
> 	(subst_stack_regs_in_debug_insn): Delete.
> 	(subst_all_stack_regs_in_debug_insn): Use FOR_EACH_SUBRTX_PTR
> 	instead of for_each_rtx.
OK.
Jeff

[PATCH 34/50] regcprop.c:kill_autoinc_value

[Richard Sandiford]

gcc/
	* regcprop.c: Include rtl-iter.h.
	(kill_value): Take a const_rtx.
	(kill_autoinc_value): Turn from being a for_each_rtx callback
	to being a function that examines each subrtx itself.
	(copyprop_hardreg_forward_1): Update accordingly.

Index: gcc/regcprop.c
===================================================================
--- gcc/regcprop.c	2014-08-03 11:25:09.748952560 +0100
+++ gcc/regcprop.c	2014-08-03 11:25:29.603148851 +0100
@@ -36,6 +36,7 @@
 #include "obstack.h"
 #include "tree-pass.h"
 #include "df.h"
+#include "rtl-iter.h"
 
 /* The following code does forward propagation of hard register copies.
    The object is to eliminate as many dependencies as possible, so that
@@ -81,12 +82,11 @@ struct value_data
 
 static void kill_value_one_regno (unsigned, struct value_data *);
 static void kill_value_regno (unsigned, unsigned, struct value_data *);
-static void kill_value (rtx, struct value_data *);
+static void kill_value (const_rtx, struct value_data *);
 static void set_value_regno (unsigned, enum machine_mode, struct value_data *);
 static void init_value_data (struct value_data *);
 static void kill_clobbered_value (rtx, const_rtx, void *);
 static void kill_set_value (rtx, const_rtx, void *);
-static int kill_autoinc_value (rtx *, void *);
 static void copy_value (rtx, rtx, struct value_data *);
 static bool mode_change_ok (enum machine_mode, enum machine_mode,
 			    unsigned int);
@@ -190,16 +190,13 @@ kill_value_regno (unsigned int regno, un
    so that we mind the mode the register is in.  */
 
 static void
-kill_value (rtx x, struct value_data *vd)
+kill_value (const_rtx x, struct value_data *vd)
 {
-  rtx orig_rtx = x;
-
   if (GET_CODE (x) == SUBREG)
     {
-      x = simplify_subreg (GET_MODE (x), SUBREG_REG (x),
-			   GET_MODE (SUBREG_REG (x)), SUBREG_BYTE (x));
-      if (x == NULL_RTX)
-	x = SUBREG_REG (orig_rtx);
+      rtx tmp = simplify_subreg (GET_MODE (x), SUBREG_REG (x),
+				 GET_MODE (SUBREG_REG (x)), SUBREG_BYTE (x));
+      x = tmp ? tmp : SUBREG_REG (x);
     }
   if (REG_P (x))
     {
@@ -276,25 +273,24 @@ kill_set_value (rtx x, const_rtx set, vo
     }
 }
 
-/* Called through for_each_rtx.  Kill any register used as the base of an
-   auto-increment expression, and install that register as the root of its
-   own value list.  */
+/* Kill any register used in X as the base of an auto-increment expression,
+   and install that register as the root of its own value list.  */
 
-static int
-kill_autoinc_value (rtx *px, void *data)
+static void
+kill_autoinc_value (rtx insn, struct value_data *vd)
 {
-  rtx x = *px;
-  struct value_data *const vd = (struct value_data *) data;
-
-  if (GET_RTX_CLASS (GET_CODE (x)) == RTX_AUTOINC)
+  subrtx_iterator::array_type array;
+  FOR_EACH_SUBRTX (iter, array, PATTERN (insn), NONCONST)
     {
-      x = XEXP (x, 0);
-      kill_value (x, vd);
-      set_value_regno (REGNO (x), GET_MODE (x), vd);
-      return -1;
+      const_rtx x = *iter;
+      if (GET_RTX_CLASS (GET_CODE (x)) == RTX_AUTOINC)
+	{
+	  x = XEXP (x, 0);
+	  kill_value (x, vd);
+	  set_value_regno (REGNO (x), GET_MODE (x), vd);
+	  iter.skip_subrtxes ();
+	}
     }
-
-  return 0;
 }
 
 /* Assert that SRC has been copied to DEST.  Adjust the data structures
@@ -807,7 +803,7 @@ copyprop_hardreg_forward_1 (basic_block
 
       /* Kill all auto-incremented values.  */
       /* ??? REG_INC is useless, since stack pushes aren't done that way.  */
-      for_each_rtx (&PATTERN (insn), kill_autoinc_value, vd);
+      kill_autoinc_value (insn, vd);
 
       /* Kill all early-clobbered operands.  */
       for (i = 0; i < n_ops; i++)

Re: [PATCH 34/50] regcprop.c:kill_autoinc_value

[Jeff Law]

On 08/03/14 08:19, Richard Sandiford wrote:
> gcc/
> 	* regcprop.c: Include rtl-iter.h.
> 	(kill_value): Take a const_rtx.
> 	(kill_autoinc_value): Turn from being a for_each_rtx callback
> 	to being a function that examines each subrtx itself.
> 	(copyprop_hardreg_forward_1): Update accordingly.
OK.

FWIW, I obviously focused on patches that fit on a page or so first. 
I'd hoped to make a full pass through today, then pick up the rest 
tomorrow, but family commitments are starting earlier than planned.

I'm going to get through as many of the painfully trivial patches as I 
can before disappearing for the day.

jeff

[PATCH 35/50] regcprop.c:cprop_find_used_regs

[Richard Sandiford]

gcc/
	* regcprop.c (cprop_find_used_regs_1): Delete.
	(cprop_find_used_regs): Use FOR_EACH_SUBRTX instead of for_each_rtx.

Index: gcc/regcprop.c
===================================================================
--- gcc/regcprop.c	2014-08-03 11:25:29.603148851 +0100
+++ gcc/regcprop.c	2014-08-03 11:25:29.878151570 +0100
@@ -702,33 +702,28 @@ apply_debug_insn_changes (struct value_d
   apply_change_group ();
 }
 
-/* Called via for_each_rtx, for all used registers in a real
-   insn apply DEBUG_INSN changes that change registers to the
-   used register.  */
-
-static int
-cprop_find_used_regs_1 (rtx *loc, void *data)
-{
-  if (REG_P (*loc))
-    {
-      struct value_data *vd = (struct value_data *) data;
-      if (vd->e[REGNO (*loc)].debug_insn_changes)
-	{
-	  apply_debug_insn_changes (vd, REGNO (*loc));
-	  free_debug_insn_changes (vd, REGNO (*loc));
-	}
-    }
-  return 0;
-}
-
 /* Called via note_uses, for all used registers in a real insn
    apply DEBUG_INSN changes that change registers to the used
    registers.  */
 
 static void
-cprop_find_used_regs (rtx *loc, void *vd)
+cprop_find_used_regs (rtx *loc, void *data)
 {
-  for_each_rtx (loc, cprop_find_used_regs_1, vd);
+  struct value_data *const vd = (struct value_data *) data;
+  subrtx_iterator::array_type array;
+  FOR_EACH_SUBRTX (iter, array, *loc, NONCONST)
+    {
+      const_rtx x = *iter;
+      if (REG_P (x))
+	{
+	  unsigned int regno = REGNO (x);
+	  if (vd->e[regno].debug_insn_changes)
+	    {
+	      apply_debug_insn_changes (vd, regno);
+	      free_debug_insn_changes (vd, regno);
+	    }
+	}
+    }
 }
 
 /* Perform the forward copy propagation on basic block BB.  */

Re: [PATCH 35/50] regcprop.c:cprop_find_used_regs

[Jeff Law]

On 08/03/14 08:19, Richard Sandiford wrote:
> gcc/
> 	* regcprop.c (cprop_find_used_regs_1): Delete.
> 	(cprop_find_used_regs): Use FOR_EACH_SUBRTX instead of for_each_rtx.
OK.
jeff

[PATCH 36/50] reload1.c:note_reg_elim_costly

[Richard Sandiford]

gcc/
	* reload1.c: Include rtl-iter.h.
	(note_reg_elim_costly): Turn from being a for_each_rtx callback
	to being a function that examines each subrtx itself.
	(eliminate_regs_1, elimination_costs_in_insn): Update accordingly.

Index: gcc/reload1.c
===================================================================
--- gcc/reload1.c	2014-08-03 11:25:09.697952056 +0100
+++ gcc/reload1.c	2014-08-03 11:25:30.135154111 +0100
@@ -45,6 +45,7 @@ Software Foundation; either version 3, o
 #include "target.h"
 #include "emit-rtl.h"
 #include "dumpfile.h"
+#include "rtl-iter.h"
 
 /* This file contains the reload pass of the compiler, which is
    run after register allocation has been done.  It checks that
@@ -2486,34 +2487,33 @@ set_label_offsets (rtx x, rtx insn, int
     }
 }
 \f
-/* Called through for_each_rtx, this function examines every reg that occurs
-   in PX and adjusts the costs for its elimination which are gathered by IRA.
-   DATA is the insn in which PX occurs.  We do not recurse into MEM
-   expressions.  */
+/* This function examines every reg that occurs in X and adjusts the
+   costs for its elimination which are gathered by IRA.  INSN is the
+   insn in which X occurs.  We do not recurse into MEM expressions.  */
 
-static int
-note_reg_elim_costly (rtx *px, void *data)
+static void
+note_reg_elim_costly (const_rtx x, rtx insn)
 {
-  rtx insn = (rtx)data;
-  rtx x = *px;
-
-  if (MEM_P (x))
-    return -1;
-
-  if (REG_P (x)
-      && REGNO (x) >= FIRST_PSEUDO_REGISTER
-      && reg_equiv_init (REGNO (x))
-      && reg_equiv_invariant (REGNO (x)))
+  subrtx_iterator::array_type array;
+  FOR_EACH_SUBRTX (iter, array, x, NONCONST)
     {
-      rtx t = reg_equiv_invariant (REGNO (x));
-      rtx new_rtx = eliminate_regs_1 (t, Pmode, insn, true, true);
-      int cost = set_src_cost (new_rtx, optimize_bb_for_speed_p (elim_bb));
-      int freq = REG_FREQ_FROM_BB (elim_bb);
+      const_rtx x = *iter;
+      if (MEM_P (x))
+	iter.skip_subrtxes ();
+      else if (REG_P (x)
+	       && REGNO (x) >= FIRST_PSEUDO_REGISTER
+	       && reg_equiv_init (REGNO (x))
+	       && reg_equiv_invariant (REGNO (x)))
+	{
+	  rtx t = reg_equiv_invariant (REGNO (x));
+	  rtx new_rtx = eliminate_regs_1 (t, Pmode, insn, true, true);
+	  int cost = set_src_cost (new_rtx, optimize_bb_for_speed_p (elim_bb));
+	  int freq = REG_FREQ_FROM_BB (elim_bb);
 
-      if (cost != 0)
-	ira_adjust_equiv_reg_cost (REGNO (x), -cost * freq);
+	  if (cost != 0)
+	    ira_adjust_equiv_reg_cost (REGNO (x), -cost * freq);
+	}
     }
-  return 0;
 }
 
 /* Scan X and replace any eliminable registers (such as fp) with a
@@ -2888,7 +2888,7 @@ eliminate_regs_1 (rtx x, enum machine_mo
       if (for_costs
 	  && memory_address_p (GET_MODE (x), XEXP (x, 0))
 	  && !memory_address_p (GET_MODE (x), new_rtx))
-	for_each_rtx (&XEXP (x, 0), note_reg_elim_costly, insn);
+	note_reg_elim_costly (XEXP (x, 0), insn);
 
       return replace_equiv_address_nv (x, new_rtx);
 
@@ -3731,7 +3731,7 @@ elimination_costs_in_insn (rtx insn)
 	  if (old_set && recog_data.operand_loc[i] == &SET_SRC (old_set))
 	    is_set_src = true;
 	  if (is_set_src && !sets_reg_p)
-	    note_reg_elim_costly (&SET_SRC (old_set), insn);
+	    note_reg_elim_costly (SET_SRC (old_set), insn);
 	  in_plus = false;
 	  if (plus_src && sets_reg_p
 	      && (recog_data.operand_loc[i] == &XEXP (plus_src, 0)

Re: [PATCH 36/50] reload1.c:note_reg_elim_costly

[Jeff Law]

On 08/03/14 08:20, Richard Sandiford wrote:
> gcc/
> 	* reload1.c: Include rtl-iter.h.
> 	(note_reg_elim_costly): Turn from being a for_each_rtx callback
> 	to being a function that examines each subrtx itself.
> 	(eliminate_regs_1, elimination_costs_in_insn): Update accordingly.
OK.
jeff

[PATCH 37/50] rtlanal.c:rtx_referenced_p

[Richard Sandiford]

The old function handled constant pool SYMBOL_REFs by going straight to the
underlying constant, which meant you couldn't test for the SYMBOL_REF itself.


gcc/
	* rtl.h (get_pool_constant, rtx_referenced_p): Replace rtx parameters
	with const_rtx parameters.
	* varasm.c (get_pool_constant): Likewise.
	* rtlanal.c (rtx_referenced_p_1): Delete.
	(rtx_referenced_p): Use FOR_EACH_SUBRTX instead of for_each_rtx.
	Assert that the rtx we're looking for is nonnull.  Allow searches
	for constant pool SYMBOL_REFs.

Index: gcc/rtl.h
===================================================================
--- gcc/rtl.h	2014-08-03 11:25:09.679951878 +0100
+++ gcc/rtl.h	2014-08-03 11:25:30.447157195 +0100
@@ -2045,7 +2045,7 @@ extern rtx force_const_mem (enum machine
 /* In varasm.c  */
 
 struct function;
-extern rtx get_pool_constant (rtx);
+extern rtx get_pool_constant (const_rtx);
 extern rtx get_pool_constant_mark (rtx, bool *);
 extern enum machine_mode get_pool_mode (const_rtx);
 extern rtx simplify_subtraction (rtx);
@@ -2288,7 +2288,7 @@ extern void copy_reg_eh_region_note_back
 extern int inequality_comparisons_p (const_rtx);
 extern rtx replace_rtx (rtx, rtx, rtx);
 extern int replace_label (rtx *, void *);
-extern int rtx_referenced_p (rtx, rtx);
+extern bool rtx_referenced_p (const_rtx, const_rtx);
 extern bool tablejump_p (const_rtx, rtx *, rtx *);
 extern int computed_jump_p (const_rtx);
 extern bool tls_referenced_p (rtx);
Index: gcc/varasm.c
===================================================================
--- gcc/varasm.c	2014-08-03 11:25:09.679951878 +0100
+++ gcc/varasm.c	2014-08-03 11:25:30.450157225 +0100
@@ -3720,7 +3720,7 @@ force_const_mem (enum machine_mode mode,
 /* Given a constant pool SYMBOL_REF, return the corresponding constant.  */
 
 rtx
-get_pool_constant (rtx addr)
+get_pool_constant (const_rtx addr)
 {
   return SYMBOL_REF_CONSTANT (addr)->constant;
 }
Index: gcc/rtlanal.c
===================================================================
--- gcc/rtlanal.c	2014-08-03 11:25:20.309056964 +0100
+++ gcc/rtlanal.c	2014-08-03 11:25:30.448157205 +0100
@@ -43,7 +43,6 @@ Software Foundation; either version 3, o
 static void set_of_1 (rtx, const_rtx, void *);
 static bool covers_regno_p (const_rtx, unsigned int);
 static bool covers_regno_no_parallel_p (const_rtx, unsigned int);
-static int rtx_referenced_p_1 (rtx *, void *);
 static int computed_jump_p_1 (const_rtx);
 static void parms_set (rtx, const_rtx, void *);
 
@@ -2832,37 +2831,28 @@ replace_label (rtx *x, void *data)
   return 0;
 }
 
-/* When *BODY is equal to X or X is directly referenced by *BODY
-   return nonzero, thus FOR_EACH_RTX stops traversing and returns nonzero
-   too, otherwise FOR_EACH_RTX continues traversing *BODY.  */
-
-static int
-rtx_referenced_p_1 (rtx *body, void *x)
-{
-  rtx y = (rtx) x;
-
-  if (*body == NULL_RTX)
-    return y == NULL_RTX;
-
-  /* Return true if a label_ref *BODY refers to label Y.  */
-  if (GET_CODE (*body) == LABEL_REF && LABEL_P (y))
-    return XEXP (*body, 0) == y;
-
-  /* If *BODY is a reference to pool constant traverse the constant.  */
-  if (GET_CODE (*body) == SYMBOL_REF
-      && CONSTANT_POOL_ADDRESS_P (*body))
-    return rtx_referenced_p (y, get_pool_constant (*body));
-
-  /* By default, compare the RTL expressions.  */
-  return rtx_equal_p (*body, y);
-}
-
 /* Return true if X is referenced in BODY.  */
 
-int
-rtx_referenced_p (rtx x, rtx body)
+bool
+rtx_referenced_p (const_rtx x, const_rtx body)
 {
-  return for_each_rtx (&body, rtx_referenced_p_1, x);
+  subrtx_iterator::array_type array;
+  FOR_EACH_SUBRTX (iter, array, body, ALL)
+    if (const_rtx y = *iter)
+      {
+	/* Check if a label_ref Y refers to label X.  */
+	if (GET_CODE (y) == LABEL_REF && LABEL_P (y) && XEXP (y, 0) == x)
+	  return true;
+
+	if (rtx_equal_p (x, y))
+	  return true;
+
+	/* If Y is a reference to pool constant traverse the constant.  */
+	if (GET_CODE (y) == SYMBOL_REF
+	    && CONSTANT_POOL_ADDRESS_P (y))
+	  iter.substitute (get_pool_constant (y));
+      }
+  return false;
 }
 
 /* If INSN is a tablejump return true and store the label (before jump table) to

Re: [PATCH 37/50] rtlanal.c:rtx_referenced_p

[Jeff Law]

On 08/03/14 08:22, Richard Sandiford wrote:
> The old function handled constant pool SYMBOL_REFs by going straight to the
> underlying constant, which meant you couldn't test for the SYMBOL_REF itself.
>
>
> gcc/
> 	* rtl.h (get_pool_constant, rtx_referenced_p): Replace rtx parameters
> 	with const_rtx parameters.
> 	* varasm.c (get_pool_constant): Likewise.
> 	* rtlanal.c (rtx_referenced_p_1): Delete.
> 	(rtx_referenced_p): Use FOR_EACH_SUBRTX instead of for_each_rtx.
> 	Assert that the rtx we're looking for is nonnull.  Allow searches
> 	for constant pool SYMBOL_REFs.
OK.
Jeff

[PATCH 38/50] rtlanal.c:replace_label

[Richard Sandiford]

The main change here is to handle ADDR_VEC and ADDR_DIFF_VECs specially,
since they can have many elements and are a relatively important case
for this function.  This is for speed rather than correctness.


gcc/
	* rtl.h (replace_label_data): Delete.
	(replace_label): Take the old label, new label and update-nuses flag
	as direct arguments.  Return void.
	* cfgcleanup.c (outgoing_edges_match): Update accordingly.
	* rtlanal.c (replace_label): Update interface as above.  Handle
	JUMP_TABLE_DATA as a special case.  Handle JUMPs outside the
	iterator.  Use FOR_EACH_SUBRTX_PTR.

Index: gcc/rtl.h
===================================================================
--- gcc/rtl.h	2014-08-03 11:25:30.447157195 +0100
+++ gcc/rtl.h	2014-08-03 11:25:30.745160142 +0100
@@ -2211,14 +2211,6 @@ #define single_set(I) (INSN_P (I) \
 		       : NULL_RTX)
 #define single_set_1(I) single_set_2 (I, PATTERN (I))
 
-/* Structure used for passing data to REPLACE_LABEL.  */
-struct replace_label_data
-{
-  rtx r1;
-  rtx r2;
-  bool update_label_nuses;
-};
-
 extern enum machine_mode get_address_mode (rtx mem);
 extern int rtx_addr_can_trap_p (const_rtx);
 extern bool nonzero_address_p (const_rtx);
@@ -2287,7 +2279,7 @@ extern void copy_reg_eh_region_note_forw
 extern void copy_reg_eh_region_note_backward (rtx, rtx, rtx);
 extern int inequality_comparisons_p (const_rtx);
 extern rtx replace_rtx (rtx, rtx, rtx);
-extern int replace_label (rtx *, void *);
+extern void replace_label (rtx *, rtx, rtx, bool);
 extern bool rtx_referenced_p (const_rtx, const_rtx);
 extern bool tablejump_p (const_rtx, rtx *, rtx *);
 extern int computed_jump_p (const_rtx);
Index: gcc/cfgcleanup.c
===================================================================
--- gcc/cfgcleanup.c	2014-08-03 11:25:22.602079634 +0100
+++ gcc/cfgcleanup.c	2014-08-03 11:25:30.745160142 +0100
@@ -1716,15 +1716,11 @@ outgoing_edges_match (int mode, basic_bl
 
 	      if (identical)
 		{
-		  replace_label_data rr;
 		  bool match;
 
 		  /* Temporarily replace references to LABEL1 with LABEL2
 		     in BB1->END so that we could compare the instructions.  */
-		  rr.r1 = label1;
-		  rr.r2 = label2;
-		  rr.update_label_nuses = false;
-		  for_each_rtx (&BB_END (bb1), replace_label, &rr);
+		  replace_label (&BB_END (bb1), label1, label2, false);
 
 		  match = (old_insns_match_p (mode, BB_END (bb1), BB_END (bb2))
 			   == dir_both);
@@ -1736,9 +1732,7 @@ outgoing_edges_match (int mode, basic_bl
 		  /* Set the original label in BB1->END because when deleting
 		     a block whose end is a tablejump, the tablejump referenced
 		     from the instruction is deleted too.  */
-		  rr.r1 = label2;
-		  rr.r2 = label1;
-		  for_each_rtx (&BB_END (bb1), replace_label, &rr);
+		  replace_label (&BB_END (bb1), label2, label1, false);
 
 		  return match;
 		}
@@ -1985,20 +1979,16 @@ #define SWAP(T, X, Y) do { T tmp = (X);
 	  && tablejump_p (BB_END (osrc2), &label2, &table2)
 	  && label1 != label2)
 	{
-	  replace_label_data rr;
 	  rtx insn;
 
 	  /* Replace references to LABEL1 with LABEL2.  */
-	  rr.r1 = label1;
-	  rr.r2 = label2;
-	  rr.update_label_nuses = true;
 	  for (insn = get_insns (); insn; insn = NEXT_INSN (insn))
 	    {
 	      /* Do not replace the label in SRC1->END because when deleting
 		 a block whose end is a tablejump, the tablejump referenced
 		 from the instruction is deleted too.  */
 	      if (insn != BB_END (osrc1))
-		for_each_rtx (&insn, replace_label, &rr);
+		replace_label (&insn, label1, label2, true);
 	    }
 	}
     }
Index: gcc/rtlanal.c
===================================================================
--- gcc/rtlanal.c	2014-08-03 11:25:30.448157205 +0100
+++ gcc/rtlanal.c	2014-08-03 11:25:30.746160151 +0100
@@ -2770,65 +2770,79 @@ replace_rtx (rtx x, rtx from, rtx to)
   return x;
 }
 \f
-/* Replace occurrences of the old label in *X with the new one.
-   DATA is a REPLACE_LABEL_DATA containing the old and new labels.  */
+/* Replace occurrences of the OLD_LABEL in *LOC with NEW_LABEL.  Also track
+   the change in LABEL_NUSES if UPDATE_LABEL_NUSES.  */
 
-int
-replace_label (rtx *x, void *data)
+void
+replace_label (rtx *loc, rtx old_label, rtx new_label, bool update_label_nuses)
 {
-  rtx l = *x;
-  rtx old_label = ((replace_label_data *) data)->r1;
-  rtx new_label = ((replace_label_data *) data)->r2;
-  bool update_label_nuses = ((replace_label_data *) data)->update_label_nuses;
-
-  if (l == NULL_RTX)
-    return 0;
-
-  if (GET_CODE (l) == SYMBOL_REF
-      && CONSTANT_POOL_ADDRESS_P (l))
+  /* Handle jump tables specially, since ADDR_{DIFF_,}VECs can be long.  */
+  rtx x = *loc;
+  if (JUMP_TABLE_DATA_P (x))
     {
-      rtx c = get_pool_constant (l);
-      if (rtx_referenced_p (old_label, c))
+      x = PATTERN (x);
+      rtvec vec = XVEC (x, GET_CODE (x) == ADDR_DIFF_VEC);
+      int len = GET_NUM_ELEM (vec);
+      for (int i = 0; i < len; ++i)
 	{
-	  rtx new_c, new_l;
-	  replace_label_data *d = (replace_label_data *) data;
-
-	  /* Create a copy of constant C; replace the label inside
-	     but do not update LABEL_NUSES because uses in constant pool
-	     are not counted.  */
-	  new_c = copy_rtx (c);
-	  d->update_label_nuses = false;
-	  for_each_rtx (&new_c, replace_label, data);
-	  d->update_label_nuses = update_label_nuses;
-
-	  /* Add the new constant NEW_C to constant pool and replace
-	     the old reference to constant by new reference.  */
-	  new_l = XEXP (force_const_mem (get_pool_mode (l), new_c), 0);
-	  *x = replace_rtx (l, l, new_l);
+	  rtx ref = RTVEC_ELT (vec, i);
+	  if (XEXP (ref, 0) == old_label)
+	    {
+	      XEXP (ref, 0) = new_label;
+	      if (update_label_nuses)
+		{
+		  ++LABEL_NUSES (new_label);
+		  --LABEL_NUSES (old_label);
+		}
+	    }
 	}
-      return 0;
+      return;
     }
 
   /* If this is a JUMP_INSN, then we also need to fix the JUMP_LABEL
-     field.  This is not handled by for_each_rtx because it doesn't
+     field.  This is not handled by the iterator because it doesn't
      handle unprinted ('0') fields.  */
-  if (JUMP_P (l) && JUMP_LABEL (l) == old_label)
-    JUMP_LABEL (l) = new_label;
+  if (JUMP_P (x) && JUMP_LABEL (x) == old_label)
+    JUMP_LABEL (x) = new_label;
 
-  if ((GET_CODE (l) == LABEL_REF
-       || GET_CODE (l) == INSN_LIST)
-      && XEXP (l, 0) == old_label)
+  subrtx_ptr_iterator::array_type array;
+  FOR_EACH_SUBRTX_PTR (iter, array, loc, ALL)
     {
-      XEXP (l, 0) = new_label;
-      if (update_label_nuses)
+      rtx *loc = *iter;
+      if (rtx x = *loc)
 	{
-	  ++LABEL_NUSES (new_label);
-	  --LABEL_NUSES (old_label);
+	  if (GET_CODE (x) == SYMBOL_REF
+	      && CONSTANT_POOL_ADDRESS_P (x))
+	    {
+	      rtx c = get_pool_constant (x);
+	      if (rtx_referenced_p (old_label, c))
+		{
+		  /* Create a copy of constant C; replace the label inside
+		     but do not update LABEL_NUSES because uses in constant pool
+		     are not counted.  */
+		  rtx new_c = copy_rtx (c);
+		  replace_label (&new_c, old_label, new_label, false);
+
+		  /* Add the new constant NEW_C to constant pool and replace
+		     the old reference to constant by new reference.  */
+		  rtx new_mem = force_const_mem (get_pool_mode (x), new_c);
+		  *loc = replace_rtx (x, x, XEXP (new_mem, 0));
+		}
+	    }
+
+	  if ((GET_CODE (x) == LABEL_REF
+	       || GET_CODE (x) == INSN_LIST)
+	      && XEXP (x, 0) == old_label)
+	    {
+	      XEXP (x, 0) = new_label;
+	      if (update_label_nuses)
+		{
+		  ++LABEL_NUSES (new_label);
+		  --LABEL_NUSES (old_label);
+		}
+	    }
 	}
-      return 0;
     }
-
-  return 0;
 }
 
 /* Return true if X is referenced in BODY.  */

Re: [PATCH 38/50] rtlanal.c:replace_label

[Jeff Law]

On 08/03/14 08:25, Richard Sandiford wrote:
> The main change here is to handle ADDR_VEC and ADDR_DIFF_VECs specially,
> since they can have many elements and are a relatively important case
> for this function.  This is for speed rather than correctness.
>
>
> gcc/
> 	* rtl.h (replace_label_data): Delete.
> 	(replace_label): Take the old label, new label and update-nuses flag
> 	as direct arguments.  Return void.
> 	* cfgcleanup.c (outgoing_edges_match): Update accordingly.
> 	* rtlanal.c (replace_label): Update interface as above.  Handle
> 	JUMP_TABLE_DATA as a special case.  Handle JUMPs outside the
> 	iterator.  Use FOR_EACH_SUBRTX_PTR.
OK.
jeff

[PATCH 39/50] rtlanal.c:record_hard_reg_uses

[Richard Sandiford]

find_all_hard_regs seems like a useful function so I split it out of
the note_uses callback and exposed it in rtl.h.  I have (or had)
other patches that make use of it.


gcc/
	* rtl.h (find_all_hard_regs): Declare.
	* rtlanal.c (find_all_hard_regs): New function.
	(record_hard_reg_uses_1): Delete.
	(record_hard_reg_uses): Use find_all_hard_regs.

Index: gcc/rtl.h
===================================================================
--- gcc/rtl.h	2014-08-03 11:25:30.745160142 +0100
+++ gcc/rtl.h	2014-08-03 11:25:31.042163078 +0100
@@ -2245,6 +2245,7 @@ extern const_rtx set_of (const_rtx, cons
 extern void record_hard_reg_sets (rtx, const_rtx, void *);
 extern void record_hard_reg_uses (rtx *, void *);
 #ifdef HARD_CONST
+extern void find_all_hard_regs (const_rtx, HARD_REG_SET *);
 extern void find_all_hard_reg_sets (const_rtx, HARD_REG_SET *, bool);
 #endif
 extern void note_stores (const_rtx, void (*) (rtx, const_rtx, void *), void *);
Index: gcc/rtlanal.c
===================================================================
--- gcc/rtlanal.c	2014-08-03 11:25:30.746160151 +0100
+++ gcc/rtlanal.c	2014-08-03 11:25:31.043163088 +0100
@@ -1122,6 +1122,19 @@ set_of (const_rtx pat, const_rtx insn)
   return data.found;
 }
 
+/* Add all hard register in X to *PSET.  */
+void
+find_all_hard_regs (const_rtx x, HARD_REG_SET *pset)
+{
+  subrtx_iterator::array_type array;
+  FOR_EACH_SUBRTX (iter, array, x, NONCONST)
+    {
+      const_rtx x = *iter;
+      if (REG_P (x) && REGNO (x) < FIRST_PSEUDO_REGISTER)
+	add_to_hard_reg_set (pset, GET_MODE (x), REGNO (x));
+    }
+}
+
 /* This function, called through note_stores, collects sets and
    clobbers of hard registers in a HARD_REG_SET, which is pointed to
    by DATA.  */
@@ -1155,27 +1168,11 @@ find_all_hard_reg_sets (const_rtx insn,
       record_hard_reg_sets (XEXP (link, 0), NULL, pset);
 }
 
-/* A for_each_rtx subroutine of record_hard_reg_uses.  */
-static int
-record_hard_reg_uses_1 (rtx *px, void *data)
-{
-  rtx x = *px;
-  HARD_REG_SET *pused = (HARD_REG_SET *)data;
-
-  if (REG_P (x) && REGNO (x) < FIRST_PSEUDO_REGISTER)
-    {
-      int nregs = hard_regno_nregs[REGNO (x)][GET_MODE (x)];
-      while (nregs-- > 0)
-	SET_HARD_REG_BIT (*pused, REGNO (x) + nregs);
-    }
-  return 0;
-}
-
 /* Like record_hard_reg_sets, but called through note_uses.  */
 void
 record_hard_reg_uses (rtx *px, void *data)
 {
-  for_each_rtx (px, record_hard_reg_uses_1, data);
+  find_all_hard_regs (*px, (HARD_REG_SET *) data);
 }
 \f
 /* Given an INSN, return a SET expression if this insn has only a single SET.

Re: [PATCH 39/50] rtlanal.c:record_hard_reg_uses

[Jeff Law]

On 08/03/14 08:27, Richard Sandiford wrote:
> find_all_hard_regs seems like a useful function so I split it out of
> the note_uses callback and exposed it in rtl.h.  I have (or had)
> other patches that make use of it.
>
>
> gcc/
> 	* rtl.h (find_all_hard_regs): Declare.
> 	* rtlanal.c (find_all_hard_regs): New function.
> 	(record_hard_reg_uses_1): Delete.
> 	(record_hard_reg_uses): Use find_all_hard_regs.
OK.
Jeff

[PATCH 40/50] rtlanal.c:for_each_inc_dec

[Richard Sandiford]

The old for_each_inc_dec callback had a for_each_rtx-like return value,
with >0 being returned directly, 0 meaning "continue" and <0 meaning
"skip subrtxes".  But there's no reason to distinguish the latter two
cases since auto-inc/dec expressions aren't allowed to contain other
auto-inc/dec expressions.  And if for_each_rtx is going away, there's
no longer any consistency argument for using the same interface.


gcc/
	* rtl.h (for_each_inc_dec_fn): Remove special case for -1.
	* cselib.c (cselib_record_autoinc_cb): Update accordingly.
	(cselib_record_sets): Likewise.
	* dse.c (emit_inc_dec_insn_before, check_for_inc_dec_1)
	(check_for_inc_dec): Likewise.
	* rtlanal.c (for_each_inc_dec_ops): Delete.
	(for_each_inc_dec_find_inc_dec): Take the MEM as argument,
	rather than a pointer to the memory address.  Replace
	for_each_inc_dec_ops argument with separate function and data
	arguments.  Abort on non-autoinc addresses.
	(for_each_inc_dec_find_mem): Delete.
	(for_each_inc_dec): Use FOR_EACH_SUBRTX_VAR to visit every
	autoinc MEM.

Index: gcc/rtl.h
===================================================================
--- gcc/rtl.h	2014-08-03 11:25:31.042163078 +0100
+++ gcc/rtl.h	2014-08-03 11:25:31.324165866 +0100
@@ -2293,9 +2293,8 @@ extern int for_each_rtx (rtx *, rtx_func
    within MEM that sets DEST to SRC + SRCOFF, or SRC if SRCOFF is
    NULL.  The callback is passed the same opaque ARG passed to
    for_each_inc_dec.  Return zero to continue looking for other
-   autoinc operations, -1 to skip OP's operands, and any other value
-   to interrupt the traversal and return that value to the caller of
-   for_each_inc_dec.  */
+   autoinc operations or any other value to interrupt the traversal and
+   return that value to the caller of for_each_inc_dec.  */
 typedef int (*for_each_inc_dec_fn) (rtx mem, rtx op, rtx dest, rtx src,
 				    rtx srcoff, void *arg);
 extern int for_each_inc_dec (rtx *, for_each_inc_dec_fn, void *arg);
Index: gcc/cselib.c
===================================================================
--- gcc/cselib.c	2014-08-03 11:25:09.536950464 +0100
+++ gcc/cselib.c	2014-08-03 11:25:31.323165856 +0100
@@ -2464,7 +2464,7 @@ cselib_record_autoinc_cb (rtx mem ATTRIB
 
   data->n_sets++;
 
-  return -1;
+  return 0;
 }
 
 /* Record the effects of any sets and autoincs in INSN.  */
@@ -2523,7 +2523,7 @@ cselib_record_sets (rtx insn)
 
   data.sets = sets;
   data.n_sets = n_sets_before_autoinc = n_sets;
-  for_each_inc_dec (&insn, cselib_record_autoinc_cb, &data);
+  for_each_inc_dec (&PATTERN (insn), cselib_record_autoinc_cb, &data);
   n_sets = data.n_sets;
 
   /* Look up the values that are read.  Do this before invalidating the
Index: gcc/dse.c
===================================================================
--- gcc/dse.c	2014-08-03 11:25:25.192105241 +0100
+++ gcc/dse.c	2014-08-03 11:25:31.323165856 +0100
@@ -895,7 +895,7 @@ emit_inc_dec_insn_before (rtx mem ATTRIB
 
   emit_insn_before (new_insn, insn);
 
-  return -1;
+  return 0;
 }
 
 /* Before we delete INSN_INFO->INSN, make sure that the auto inc/dec, if it
@@ -908,7 +908,8 @@ check_for_inc_dec_1 (insn_info_t insn_in
   rtx insn = insn_info->insn;
   rtx note = find_reg_note (insn, REG_INC, NULL_RTX);
   if (note)
-    return for_each_inc_dec (&insn, emit_inc_dec_insn_before, insn_info) == 0;
+    return for_each_inc_dec (&PATTERN (insn), emit_inc_dec_insn_before,
+			     insn_info) == 0;
   return true;
 }
 
@@ -927,7 +928,8 @@ check_for_inc_dec (rtx insn)
   insn_info.fixed_regs_live = NULL;
   note = find_reg_note (insn, REG_INC, NULL_RTX);
   if (note)
-    return for_each_inc_dec (&insn, emit_inc_dec_insn_before, &insn_info) == 0;
+    return for_each_inc_dec (&PATTERN (insn), emit_inc_dec_insn_before,
+			     &insn_info) == 0;
   return true;
 }
 
Index: gcc/rtlanal.c
===================================================================
--- gcc/rtlanal.c	2014-08-03 11:25:31.043163088 +0100
+++ gcc/rtlanal.c	2014-08-03 11:25:31.325165876 +0100
@@ -3105,49 +3105,32 @@ for_each_rtx (rtx *x, rtx_function f, vo
 
 \f
 
-/* Data structure that holds the internal state communicated between
-   for_each_inc_dec, for_each_inc_dec_find_mem and
-   for_each_inc_dec_find_inc_dec.  */
-
-struct for_each_inc_dec_ops {
-  /* The function to be called for each autoinc operation found.  */
-  for_each_inc_dec_fn fn;
-  /* The opaque argument to be passed to it.  */
-  void *arg;
-  /* The MEM we're visiting, if any.  */
-  rtx mem;
-};
-
-static int for_each_inc_dec_find_mem (rtx *r, void *d);
-
-/* Find PRE/POST-INC/DEC/MODIFY operations within *R, extract the
-   operands of the equivalent add insn and pass the result to the
-   operator specified by *D.  */
+/* MEM has a PRE/POST-INC/DEC/MODIFY address X.  Extract the operands of
+   the equivalent add insn and pass the result to FN, using DATA as the
+   final argument.  */
 
 static int
-for_each_inc_dec_find_inc_dec (rtx *r, void *d)
+for_each_inc_dec_find_inc_dec (rtx mem, for_each_inc_dec_fn fn, void *data)
 {
-  rtx x = *r;
-  struct for_each_inc_dec_ops *data = (struct for_each_inc_dec_ops *)d;
-
+  rtx x = XEXP (mem, 0);
   switch (GET_CODE (x))
     {
     case PRE_INC:
     case POST_INC:
       {
-	int size = GET_MODE_SIZE (GET_MODE (data->mem));
+	int size = GET_MODE_SIZE (GET_MODE (mem));
 	rtx r1 = XEXP (x, 0);
 	rtx c = gen_int_mode (size, GET_MODE (r1));
-	return data->fn (data->mem, x, r1, r1, c, data->arg);
+	return fn (mem, x, r1, r1, c, data);
       }
 
     case PRE_DEC:
     case POST_DEC:
       {
-	int size = GET_MODE_SIZE (GET_MODE (data->mem));
+	int size = GET_MODE_SIZE (GET_MODE (mem));
 	rtx r1 = XEXP (x, 0);
 	rtx c = gen_int_mode (-size, GET_MODE (r1));
-	return data->fn (data->mem, x, r1, r1, c, data->arg);
+	return fn (mem, x, r1, r1, c, data);
       }
 
     case PRE_MODIFY:
@@ -3155,69 +3138,43 @@ for_each_inc_dec_find_inc_dec (rtx *r, v
       {
 	rtx r1 = XEXP (x, 0);
 	rtx add = XEXP (x, 1);
-	return data->fn (data->mem, x, r1, add, NULL, data->arg);
-      }
-
-    case MEM:
-      {
-	rtx save = data->mem;
-	int ret = for_each_inc_dec_find_mem (r, d);
-	data->mem = save;
-	return ret;
+	return fn (mem, x, r1, add, NULL, data);
       }
 
     default:
-      return 0;
-    }
-}
-
-/* If *R is a MEM, find PRE/POST-INC/DEC/MODIFY operations within its
-   address, extract the operands of the equivalent add insn and pass
-   the result to the operator specified by *D.  */
-
-static int
-for_each_inc_dec_find_mem (rtx *r, void *d)
-{
-  rtx x = *r;
-  if (x != NULL_RTX && MEM_P (x))
-    {
-      struct for_each_inc_dec_ops *data = (struct for_each_inc_dec_ops *) d;
-      int result;
-
-      data->mem = x;
-
-      result = for_each_rtx (&XEXP (x, 0), for_each_inc_dec_find_inc_dec,
-			     data);
-      if (result)
-	return result;
-
-      return -1;
+      gcc_unreachable ();
     }
-  return 0;
 }
 
-/* Traverse *X looking for MEMs, and for autoinc operations within
-   them.  For each such autoinc operation found, call FN, passing it
+/* Traverse *LOC looking for MEMs that have autoinc addresses.
+   For each such autoinc operation found, call FN, passing it
    the innermost enclosing MEM, the operation itself, the RTX modified
    by the operation, two RTXs (the second may be NULL) that, once
    added, represent the value to be held by the modified RTX
-   afterwards, and ARG.  FN is to return -1 to skip looking for other
-   autoinc operations within the visited operation, 0 to continue the
-   traversal, or any other value to have it returned to the caller of
+   afterwards, and DATA.  FN is to return 0 to continue the
+   traversal or any other value to have it returned to the caller of
    for_each_inc_dec.  */
 
 int
-for_each_inc_dec (rtx *x,
+for_each_inc_dec (rtx *loc,
 		  for_each_inc_dec_fn fn,
-		  void *arg)
+		  void *data)
 {
-  struct for_each_inc_dec_ops data;
-
-  data.fn = fn;
-  data.arg = arg;
-  data.mem = NULL;
-
-  return for_each_rtx (x, for_each_inc_dec_find_mem, &data);
+  subrtx_var_iterator::array_type array;
+  FOR_EACH_SUBRTX_VAR (iter, array, *loc, NONCONST)
+    {
+      rtx mem = *iter;
+      if (mem
+	  && MEM_P (mem)
+	  && GET_RTX_CLASS (GET_CODE (XEXP (mem, 0))) == RTX_AUTOINC)
+	{
+	  int res = for_each_inc_dec_find_inc_dec (mem, fn, data);
+	  if (res != 0)
+	    return res;
+	  iter.skip_subrtxes ();
+	}
+    }
+  return 0;
 }
 
 \f

Re: [PATCH 40/50] rtlanal.c:for_each_inc_dec

[Jeff Law]

On 08/03/14 08:32, Richard Sandiford wrote:
> The old for_each_inc_dec callback had a for_each_rtx-like return value,
> with >0 being returned directly, 0 meaning "continue" and <0 meaning
> "skip subrtxes".  But there's no reason to distinguish the latter two
> cases since auto-inc/dec expressions aren't allowed to contain other
> auto-inc/dec expressions.  And if for_each_rtx is going away, there's
> no longer any consistency argument for using the same interface.
>
>
> gcc/
> 	* rtl.h (for_each_inc_dec_fn): Remove special case for -1.
> 	* cselib.c (cselib_record_autoinc_cb): Update accordingly.
> 	(cselib_record_sets): Likewise.
> 	* dse.c (emit_inc_dec_insn_before, check_for_inc_dec_1)
> 	(check_for_inc_dec): Likewise.
> 	* rtlanal.c (for_each_inc_dec_ops): Delete.
> 	(for_each_inc_dec_find_inc_dec): Take the MEM as argument,
> 	rather than a pointer to the memory address.  Replace
> 	for_each_inc_dec_ops argument with separate function and data
> 	arguments.  Abort on non-autoinc addresses.
> 	(for_each_inc_dec_find_mem): Delete.
> 	(for_each_inc_dec): Use FOR_EACH_SUBRTX_VAR to visit every
> 	autoinc MEM.
So this patch has me a little bit concerned.

> @@ -2523,7 +2523,7 @@ cselib_record_sets (rtx insn)
>
>     data.sets = sets;
>     data.n_sets = n_sets_before_autoinc = n_sets;
> -  for_each_inc_dec (&insn, cselib_record_autoinc_cb, &data);
> +  for_each_inc_dec (&PATTERN (insn), cselib_record_autoinc_cb, &data);
>     n_sets = data.n_sets;
So wouldn't this miss an autoincrement operation embedded in a note, 
such as a REG_EQUAL note?  My memory is very fuzzy here, but I can't 
recall any policy which prohibits an autoincrement addressing mode from 
appearing in a REG_EQUAL note.  Worse yet, I have vague memories of 
embedded side effects actually showing up in REG_EQUAL notes.

Similarly for other places where you're passing down the pattern rather 
than the insn itself.

Thoughts/comments?  Does this trigger any disturbing memories for anyone 
else?

jeff

Re: [PATCH 40/50] rtlanal.c:for_each_inc_dec

[Richard Sandiford]

Jeff Law <law@redhat.com> writes:
> On 08/03/14 08:32, Richard Sandiford wrote:
>> The old for_each_inc_dec callback had a for_each_rtx-like return value,
>> with >0 being returned directly, 0 meaning "continue" and <0 meaning
>> "skip subrtxes".  But there's no reason to distinguish the latter two
>> cases since auto-inc/dec expressions aren't allowed to contain other
>> auto-inc/dec expressions.  And if for_each_rtx is going away, there's
>> no longer any consistency argument for using the same interface.
>>
>>
>> gcc/
>> 	* rtl.h (for_each_inc_dec_fn): Remove special case for -1.
>> 	* cselib.c (cselib_record_autoinc_cb): Update accordingly.
>> 	(cselib_record_sets): Likewise.
>> 	* dse.c (emit_inc_dec_insn_before, check_for_inc_dec_1)
>> 	(check_for_inc_dec): Likewise.
>> 	* rtlanal.c (for_each_inc_dec_ops): Delete.
>> 	(for_each_inc_dec_find_inc_dec): Take the MEM as argument,
>> 	rather than a pointer to the memory address.  Replace
>> 	for_each_inc_dec_ops argument with separate function and data
>> 	arguments.  Abort on non-autoinc addresses.
>> 	(for_each_inc_dec_find_mem): Delete.
>> 	(for_each_inc_dec): Use FOR_EACH_SUBRTX_VAR to visit every
>> 	autoinc MEM.
> So this patch has me a little bit concerned.
>
>> @@ -2523,7 +2523,7 @@ cselib_record_sets (rtx insn)
>>
>>     data.sets = sets;
>>     data.n_sets = n_sets_before_autoinc = n_sets;
>> -  for_each_inc_dec (&insn, cselib_record_autoinc_cb, &data);
>> +  for_each_inc_dec (&PATTERN (insn), cselib_record_autoinc_cb, &data);
>>     n_sets = data.n_sets;
> So wouldn't this miss an autoincrement operation embedded in a note, 
> such as a REG_EQUAL note?  My memory is very fuzzy here, but I can't 
> recall any policy which prohibits an autoincrement addressing mode from 
> appearing in a REG_EQUAL note.  Worse yet, I have vague memories of 
> embedded side effects actually showing up in REG_EQUAL notes.

But either:

(a) those notes would contain side effects that are also present in the
    main pattern, e.g.:

      (set (reg Z) (plus (mem (pre_inc X)) (reg Y)))
      REG_EQUAL: (plus (mem (pre_inc X)) (const_int Z))

(b) those notes would contain side effects that are not present in the
    main pattern.

(b) seems completely invalid to me.  REG_EQUAL notes are just a hint
and it's perfectly OK to remove them if they're no longer accurate
(e.g. because a register value used in the note is no longer available).
It's also possible to remove them if the set destination gets combined
with something else.  Plus the whole idea of a REG_EQUAL note is that
you could replace the SET_SRC with the note value without changing the
effect of the instruction.

For (a) the current code would end up recording the same side-effect
twice, so looking at just the pattern is likely to be a bug fix.
But (a) is probably invalid too in practice.

Just a guess, but maybe the thing you were thinking of was related to
the old REG_LIBCALL/REG_RETVAL support?  Although I only vaguely remember
how that worked now...

Thanks,
Richard

Re: [PATCH 40/50] rtlanal.c:for_each_inc_dec

[Jeff Law]

On 08/09/14 04:13, Richard Sandiford wrote:
> Jeff Law <law@redhat.com> writes:
>> On 08/03/14 08:32, Richard Sandiford wrote:
>>> The old for_each_inc_dec callback had a for_each_rtx-like return value,
>>> with >0 being returned directly, 0 meaning "continue" and <0 meaning
>>> "skip subrtxes".  But there's no reason to distinguish the latter two
>>> cases since auto-inc/dec expressions aren't allowed to contain other
>>> auto-inc/dec expressions.  And if for_each_rtx is going away, there's
>>> no longer any consistency argument for using the same interface.
>>>
>>>
>>> gcc/
>>> 	* rtl.h (for_each_inc_dec_fn): Remove special case for -1.
>>> 	* cselib.c (cselib_record_autoinc_cb): Update accordingly.
>>> 	(cselib_record_sets): Likewise.
>>> 	* dse.c (emit_inc_dec_insn_before, check_for_inc_dec_1)
>>> 	(check_for_inc_dec): Likewise.
>>> 	* rtlanal.c (for_each_inc_dec_ops): Delete.
>>> 	(for_each_inc_dec_find_inc_dec): Take the MEM as argument,
>>> 	rather than a pointer to the memory address.  Replace
>>> 	for_each_inc_dec_ops argument with separate function and data
>>> 	arguments.  Abort on non-autoinc addresses.
>>> 	(for_each_inc_dec_find_mem): Delete.
>>> 	(for_each_inc_dec): Use FOR_EACH_SUBRTX_VAR to visit every
>>> 	autoinc MEM.
>> So this patch has me a little bit concerned.
>>
>>> @@ -2523,7 +2523,7 @@ cselib_record_sets (rtx insn)
>>>
>>>      data.sets = sets;
>>>      data.n_sets = n_sets_before_autoinc = n_sets;
>>> -  for_each_inc_dec (&insn, cselib_record_autoinc_cb, &data);
>>> +  for_each_inc_dec (&PATTERN (insn), cselib_record_autoinc_cb, &data);
>>>      n_sets = data.n_sets;
>> So wouldn't this miss an autoincrement operation embedded in a note,
>> such as a REG_EQUAL note?  My memory is very fuzzy here, but I can't
>> recall any policy which prohibits an autoincrement addressing mode from
>> appearing in a REG_EQUAL note.  Worse yet, I have vague memories of
>> embedded side effects actually showing up in REG_EQUAL notes.
>
> But either:
>
> (a) those notes would contain side effects that are also present in the
>      main pattern, e.g.:
>
>        (set (reg Z) (plus (mem (pre_inc X)) (reg Y)))
>        REG_EQUAL: (plus (mem (pre_inc X)) (const_int Z))
>
> (b) those notes would contain side effects that are not present in the
>      main pattern.
>
> (b) seems completely invalid to me.  REG_EQUAL notes are just a hint
> and it's perfectly OK to remove them if they're no longer accurate
> (e.g. because a register value used in the note is no longer available).
> It's also possible to remove them if the set destination gets combined
> with something else.  Plus the whole idea of a REG_EQUAL note is that
> you could replace the SET_SRC with the note value without changing the
> effect of the instruction.
>
> For (a) the current code would end up recording the same side-effect
> twice, so looking at just the pattern is likely to be a bug fix.
> But (a) is probably invalid too in practice.
The note shows another way to express what appears on the RHS.  In 
theory the note is supposed to be a simpler form.   So in the case of 
(a) we might have a PARALLEL in the pattern, but a REG_INC in the note. 
  I don't see how that'd be terribly helpful though.

I agree (b) is invalid.




>
> Just a guess, but maybe the thing you were thinking of was related to
> the old REG_LIBCALL/REG_RETVAL support?  Although I only vaguely remember
> how that worked now...
I thought it was something in reload's reg_equiv handling that I 
stumbled over at some point.  The tiny bit I remember was an auto-inc in 
the note and something wanting to substitute the note for a use 
elsewhere in the insn stream.  My recollection was the note had an 
auto-inc addressing mode which significantly complicates the validity of 
such a transformation.

However, the only thread I can find is one from 2009 between myself and 
Ian, but it's not dealing with an autoinc addressing mode in the note. 
And at some level it simply doesn't make much sense to have the auto-inc 
addressing mode in a REG_EQUAL note.  I guess we could declare that 
invalid and cope with it if we ever find one.  Perhaps a bit of 
ENABLE_CHECKING to detect if we ever create such a note?

Jeff

Re: [PATCH 40/50] rtlanal.c:for_each_inc_dec

[Richard Sandiford]

[Jeff, sorry for the duplicate, sent the original from an account that
adds disclaimers.]

Jeff Law <law@redhat.com> writes:
> On 08/09/14 04:13, Richard Sandiford wrote:
>> Jeff Law <law@redhat.com> writes:
>>> On 08/03/14 08:32, Richard Sandiford wrote:
>>>> The old for_each_inc_dec callback had a for_each_rtx-like return value,
>>>> with >0 being returned directly, 0 meaning "continue" and <0 meaning
>>>> "skip subrtxes".  But there's no reason to distinguish the latter two
>>>> cases since auto-inc/dec expressions aren't allowed to contain other
>>>> auto-inc/dec expressions.  And if for_each_rtx is going away, there's
>>>> no longer any consistency argument for using the same interface.
>>>>
>>>>
>>>> gcc/
>>>> 	* rtl.h (for_each_inc_dec_fn): Remove special case for -1.
>>>> 	* cselib.c (cselib_record_autoinc_cb): Update accordingly.
>>>> 	(cselib_record_sets): Likewise.
>>>> 	* dse.c (emit_inc_dec_insn_before, check_for_inc_dec_1)
>>>> 	(check_for_inc_dec): Likewise.
>>>> 	* rtlanal.c (for_each_inc_dec_ops): Delete.
>>>> 	(for_each_inc_dec_find_inc_dec): Take the MEM as argument,
>>>> 	rather than a pointer to the memory address.  Replace
>>>> 	for_each_inc_dec_ops argument with separate function and data
>>>> 	arguments.  Abort on non-autoinc addresses.
>>>> 	(for_each_inc_dec_find_mem): Delete.
>>>> 	(for_each_inc_dec): Use FOR_EACH_SUBRTX_VAR to visit every
>>>> 	autoinc MEM.
>>> So this patch has me a little bit concerned.
>>>
>>>> @@ -2523,7 +2523,7 @@ cselib_record_sets (rtx insn)
>>>>
>>>>      data.sets = sets;
>>>>      data.n_sets = n_sets_before_autoinc = n_sets;
>>>> -  for_each_inc_dec (&insn, cselib_record_autoinc_cb, &data);
>>>> +  for_each_inc_dec (&PATTERN (insn), cselib_record_autoinc_cb, &data);
>>>>      n_sets = data.n_sets;
>>> So wouldn't this miss an autoincrement operation embedded in a note,
>>> such as a REG_EQUAL note?  My memory is very fuzzy here, but I can't
>>> recall any policy which prohibits an autoincrement addressing mode from
>>> appearing in a REG_EQUAL note.  Worse yet, I have vague memories of
>>> embedded side effects actually showing up in REG_EQUAL notes.
>>
>> But either:
>>
>> (a) those notes would contain side effects that are also present in the
>>      main pattern, e.g.:
>>
>>        (set (reg Z) (plus (mem (pre_inc X)) (reg Y)))
>>        REG_EQUAL: (plus (mem (pre_inc X)) (const_int Z))
>>
>> (b) those notes would contain side effects that are not present in the
>>      main pattern.
>>
>> (b) seems completely invalid to me.  REG_EQUAL notes are just a hint
>> and it's perfectly OK to remove them if they're no longer accurate
>> (e.g. because a register value used in the note is no longer available).
>> It's also possible to remove them if the set destination gets combined
>> with something else.  Plus the whole idea of a REG_EQUAL note is that
>> you could replace the SET_SRC with the note value without changing the
>> effect of the instruction.
>>
>> For (a) the current code would end up recording the same side-effect
>> twice, so looking at just the pattern is likely to be a bug fix.
>> But (a) is probably invalid too in practice.
> The note shows another way to express what appears on the RHS.  In 
> theory the note is supposed to be a simpler form.   So in the case of 
> (a) we might have a PARALLEL in the pattern, but a REG_INC in the note. 
>   I don't see how that'd be terribly helpful though.
>
> I agree (b) is invalid.
>
>
>
>
>>
>> Just a guess, but maybe the thing you were thinking of was related to
>> the old REG_LIBCALL/REG_RETVAL support?  Although I only vaguely remember
>> how that worked now...
> I thought it was something in reload's reg_equiv handling that I 
> stumbled over at some point.  The tiny bit I remember was an auto-inc in 
> the note and something wanting to substitute the note for a use 
> elsewhere in the insn stream.  My recollection was the note had an 
> auto-inc addressing mode which significantly complicates the validity of 
> such a transformation.
>
> However, the only thread I can find is one from 2009 between myself and 
> Ian, but it's not dealing with an autoinc addressing mode in the note. 
> And at some level it simply doesn't make much sense to have the auto-inc 
> addressing mode in a REG_EQUAL note.  I guess we could declare that 
> invalid and cope with it if we ever find one.  Perhaps a bit of 
> ENABLE_CHECKING to detect if we ever create such a note?

I suppose an assert means that it'd be up to each piece of code that
creates a note to check whether the equivalent value has autoinc addresses.
How about just dropping those REG_EQUAL and REG_EQUIV notes instead,
like we already do for ASM_OPERANDS?

Here I've extended it to all notes with side effects.  The additional
cases are:

    case CLOBBER:
      /* Reject CLOBBER with a non-VOID mode.  These are made by combine.c
	 when some combination can't be done.  If we see one, don't think
	 that we can simplify the expression.  */
      return (GET_MODE (x) != VOIDmode);

    [...snip autoincs...]
    case CALL:
    case UNSPEC_VOLATILE:
      return 1;

    case MEM:
    case ASM_INPUT:
    case ASM_OPERANDS:
      if (MEM_VOLATILE_P (x))
	return 1;

The combine clobbers shouldn't make their way into a note anyway,
since it represents a failed optimisation.  ASM_INPUT is a top-level
rtx rather than a SET_SRC, so isn't important.  Checking for volatile
ASM_OPERANDS is just a subset of the current:

      /* Don't add ASM_OPERAND REG_EQUAL/REG_EQUIV notes.
	 It serves no useful purpose and breaks eliminate_regs.  */
      if (GET_CODE (datum) == ASM_OPERANDS)
	return NULL_RTX;

So the remaining cases are CALL, UNSPEC_VOLATILE and volatile MEMs.
I think UNSPEC_VOLATILE and volatile MEMs really do fall into the same
category as auto-inc/dec.  Const CALLs should be OK in practice,
but I'm not sure why they'd ever need to be treated as having
side effects.  Other CALLs are more dangerous.  In practice the only
interesting notes for calls are (a) that the call address is equal
to some other rtx, which is recorded in the insn that sets the
address register rather than on the call itself and (b) that the result
of the call is equivalent to some non-CALL rtx (e.g. after a libcall).

Tested on x86_64-linux-gnu.  OK to install?

Thanks,
Richard


gcc/
	* emit-rtl.c (set_unique_reg_note): Discard notes with side effects.

Index: gcc/emit-rtl.c
===================================================================
--- gcc/emit-rtl.c	2014-08-26 12:09:28.242659157 +0100
+++ gcc/emit-rtl.c	2014-08-26 12:09:50.694400018 +0100
@@ -5160,6 +5160,14 @@ set_unique_reg_note (rtx insn, enum reg_
 	 It serves no useful purpose and breaks eliminate_regs.  */
       if (GET_CODE (datum) == ASM_OPERANDS)
 	return NULL_RTX;
+
+      /* Notes with side effects are dangerous.  Even if the side-effect
+	 initially mirrors one in PATTERN (INSN), later optimizations
+	 might alter the way that the final register value is calculated
+	 and so move or alter the side-effect in some way.  The note would
+	 then no longer be a valid substitution for SET_SRC.  */
+      if (side_effects_p (datum))
+	return NULL_RTX;
       break;
 
     default:

Re: [PATCH 40/50] rtlanal.c:for_each_inc_dec

[Jeff Law]

On 08/26/14 13:28, Richard Sandiford wrote:
> [Jeff, sorry for the duplicate, sent the original from an account that
> adds disclaimers.]
No worries.  Given the 3000+ messages that were waiting for me when I 
got back from PTO, what's another duplicate here and there :-)

[ Big snip. ]

>> invalid and cope with it if we ever find one.  Perhaps a bit of
>> ENABLE_CHECKING to detect if we ever create such a note?
>
> I suppose an assert means that it'd be up to each piece of code that
> creates a note to check whether the equivalent value has autoinc addresses.
> How about just dropping those REG_EQUAL and REG_EQUIV notes instead,
> like we already do for ASM_OPERANDS?
I can live with that.

>
> Here I've extended it to all notes with side effects.  The additional
> cases are:
>
>      case CLOBBER:
>        /* Reject CLOBBER with a non-VOID mode.  These are made by combine.c
> 	 when some combination can't be done.  If we see one, don't think
> 	 that we can simplify the expression.  */
>        return (GET_MODE (x) != VOIDmode);
>
>      [...snip autoincs...]
>      case CALL:
>      case UNSPEC_VOLATILE:
>        return 1;
>
>      case MEM:
>      case ASM_INPUT:
>      case ASM_OPERANDS:
>        if (MEM_VOLATILE_P (x))
> 	return 1;
>
> The combine clobbers shouldn't make their way into a note anyway,
> since it represents a failed optimisation.  ASM_INPUT is a top-level
> rtx rather than a SET_SRC, so isn't important.  Checking for volatile
> ASM_OPERANDS is just a subset of the current:
>
>        /* Don't add ASM_OPERAND REG_EQUAL/REG_EQUIV notes.
> 	 It serves no useful purpose and breaks eliminate_regs.  */
>        if (GET_CODE (datum) == ASM_OPERANDS)
> 	return NULL_RTX;
>
> So the remaining cases are CALL, UNSPEC_VOLATILE and volatile MEMs.
> I think UNSPEC_VOLATILE and volatile MEMs really do fall into the same
> category as auto-inc/dec.
Agreed.

  Const CALLs should be OK in practice,
> but I'm not sure why they'd ever need to be treated as having
> side effects.
They probably don't, but I think that's independent of your changes. 
There may be subtle things that depend on the const calls having "side 
effects" -- for example, they might be considered as clobbering argument 
saveback aeras on the stack and other such nonsense.



   Other CALLs are more dangerous.  In practice the only
> interesting notes for calls are (a) that the call address is equal
> to some other rtx, which is recorded in the insn that sets the
> address register rather than on the call itself and (b) that the result
> of the call is equivalent to some non-CALL rtx (e.g. after a libcall).
Clearly dropping these is safe as well.  And in the case of a libcall, 
the note shouldn't look like a call, the note should be the "obvious" 
RTL form that the libcall is implementing.  ie instead of a call to 
mulsi the note looks like (mult (a) (b))




>
> Tested on x86_64-linux-gnu.  OK to install?
>
> Thanks,
> Richard
>
>
> gcc/
> 	* emit-rtl.c (set_unique_reg_note): Discard notes with side effects.
OK.

Jeff

[PATCH 41/50] rtlanal.c:tls_referenced_p

[Richard Sandiford]

gcc/
	* rtl.h (tls_referenced_p): Take a const_rtx rather than an rtx.
	* rtlanal.c (tls_referenced_p_1): Delete.
	(tls_referenced_p): Take a const_rtx rather than an rtx.
	Use FOR_EACH_SUBRTX rather than for_each_rtx.

Index: gcc/rtl.h
===================================================================
--- gcc/rtl.h	2014-08-03 11:25:31.324165866 +0100
+++ gcc/rtl.h	2014-08-03 11:25:31.649169079 +0100
@@ -2284,7 +2284,7 @@ extern void replace_label (rtx *, rtx, r
 extern bool rtx_referenced_p (const_rtx, const_rtx);
 extern bool tablejump_p (const_rtx, rtx *, rtx *);
 extern int computed_jump_p (const_rtx);
-extern bool tls_referenced_p (rtx);
+extern bool tls_referenced_p (const_rtx);
 
 typedef int (*rtx_function) (rtx *, void *);
 extern int for_each_rtx (rtx *, rtx_function, void *);
Index: gcc/rtlanal.c
===================================================================
--- gcc/rtlanal.c	2014-08-03 11:25:31.325165876 +0100
+++ gcc/rtlanal.c	2014-08-03 11:25:31.650169089 +0100
@@ -6048,21 +6048,17 @@ get_index_code (const struct address_inf
   return SCRATCH;
 }
 
-/* Return 1 if *X is a thread-local symbol.  */
-
-static int
-tls_referenced_p_1 (rtx *x, void *)
-{
-  return GET_CODE (*x) == SYMBOL_REF && SYMBOL_REF_TLS_MODEL (*x) != 0;
-}
-
 /* Return true if X contains a thread-local symbol.  */
 
 bool
-tls_referenced_p (rtx x)
+tls_referenced_p (const_rtx x)
 {
   if (!targetm.have_tls)
     return false;
 
-  return for_each_rtx (&x, &tls_referenced_p_1, 0);
+  subrtx_iterator::array_type array;
+  FOR_EACH_SUBRTX (iter, array, x, NONCONST)
+    if (GET_CODE (*iter) == SYMBOL_REF && SYMBOL_REF_TLS_MODEL (*iter) != 0)
+      return true;
+  return false;
 }

Re: [PATCH 41/50] rtlanal.c:tls_referenced_p

[Jeff Law]

On 08/03/14 08:33, Richard Sandiford wrote:
> gcc/
> 	* rtl.h (tls_referenced_p): Take a const_rtx rather than an rtx.
> 	* rtlanal.c (tls_referenced_p_1): Delete.
> 	(tls_referenced_p): Take a const_rtx rather than an rtx.
> 	Use FOR_EACH_SUBRTX rather than for_each_rtx.
OK.
Jeff

[PATCH 42/50] sel-sched.c:count_occurrences_equiv

[Richard Sandiford]

gcc/
	* sel-sched.c: Include rtl-iter.h
	(count_occurrences_1): Delete.
	(count_occurrences_equiv): Turn rtxes into const_rtxes.
	Use FOR_EACH_SUBRTX rather than for_each_rtx.

Index: gcc/sel-sched.c
===================================================================
--- gcc/sel-sched.c	2014-08-03 11:23:46.458129098 +0100
+++ gcc/sel-sched.c	2014-08-03 11:25:31.949172045 +0100
@@ -42,6 +42,7 @@ Software Foundation; either version 3, o
 #include "rtlhooks-def.h"
 #include "emit-rtl.h"
 #include "ira.h"
+#include "rtl-iter.h"
 
 #ifdef INSN_SCHEDULING
 #include "sel-sched-ir.h"
@@ -798,58 +799,35 @@ substitute_reg_in_expr (expr_t expr, ins
     return false;
 }
 
-/* Helper function for count_occurences_equiv.  */
-static int
-count_occurrences_1 (rtx *cur_rtx, void *arg)
-{
-  rtx_search_arg_p p = (rtx_search_arg_p) arg;
-
-  if (REG_P (*cur_rtx) && REGNO (*cur_rtx) == REGNO (p->x))
-    {
-      /* Bail out if mode is different or more than one register is used.  */
-      if (GET_MODE (*cur_rtx) != GET_MODE (p->x)
-          || (HARD_REGISTER_P (*cur_rtx)
-	      && hard_regno_nregs[REGNO (*cur_rtx)][GET_MODE (*cur_rtx)] > 1))
-        {
-          p->n = 0;
-          return 1;
-        }
-
-      p->n++;
-
-      /* Do not traverse subexprs.  */
-      return -1;
-    }
-
-  if (GET_CODE (*cur_rtx) == SUBREG
-      && (!REG_P (SUBREG_REG (*cur_rtx))
-	  || REGNO (SUBREG_REG (*cur_rtx)) == REGNO (p->x)))
-    {
-      /* ??? Do not support substituting regs inside subregs.  In that case,
-         simplify_subreg will be called by validate_replace_rtx, and
-         unsubstitution will fail later.  */
-      p->n = 0;
-      return 1;
-    }
-
-  /* Continue search.  */
-  return 0;
-}
-
 /* Return the number of places WHAT appears within WHERE.
    Bail out when we found a reference occupying several hard registers.  */
 static int
-count_occurrences_equiv (rtx what, rtx where)
+count_occurrences_equiv (const_rtx what, const_rtx where)
 {
-  struct rtx_search_arg arg;
-
-  gcc_assert (REG_P (what));
-  arg.x = what;
-  arg.n = 0;
-
-  for_each_rtx (&where, &count_occurrences_1, (void *) &arg);
-
-  return arg.n;
+  int count = 0;
+  subrtx_iterator::array_type array;
+  FOR_EACH_SUBRTX (iter, array, where, NONCONST)
+    {
+      const_rtx x = *iter;
+      if (REG_P (x) && REGNO (x) == REGNO (what))
+	{
+	  /* Bail out if mode is different or more than one register is
+	     used.  */
+	  if (GET_MODE (x) != GET_MODE (what)
+	      || (HARD_REGISTER_P (x)
+		  && hard_regno_nregs[REGNO (x)][GET_MODE (x)] > 1))
+	    return 0;
+	  count += 1;
+	}
+      else if (GET_CODE (x) == SUBREG
+	       && (!REG_P (SUBREG_REG (x))
+		   || REGNO (SUBREG_REG (x)) == REGNO (what)))
+	/* ??? Do not support substituting regs inside subregs.  In that case,
+	   simplify_subreg will be called by validate_replace_rtx, and
+	   unsubstitution will fail later.  */
+	return 0;
+    }
+  return count;
 }
 
 /* Returns TRUE if WHAT is found in WHERE rtx tree.  */

Re: [PATCH 42/50] sel-sched.c:count_occurrences_equiv

[Jeff Law]

On 08/03/14 08:33, Richard Sandiford wrote:
> gcc/
> 	* sel-sched.c: Include rtl-iter.h
> 	(count_occurrences_1): Delete.
> 	(count_occurrences_equiv): Turn rtxes into const_rtxes.
> 	Use FOR_EACH_SUBRTX rather than for_each_rtx.
OK.
Jeff

[PATCH 43/50] store-motion.c:extract_mentioned_regs

[Richard Sandiford]

gcc/
	* store-motion.c: Include rtl-iter.h.
	(extract_mentioned_regs_1): Delete.
	(extract_mentioned_regs): Use FOR_EACH_SUBRTX_VAR rather than
	for_each_rtx to iterate over subrtxes.

Index: gcc/store-motion.c
===================================================================
--- gcc/store-motion.c	2014-08-03 11:12:52.402662718 +0100
+++ gcc/store-motion.c	2014-08-03 11:25:32.215174675 +0100
@@ -42,6 +42,7 @@ Software Foundation; either version 3, o
 #include "hash-table.h"
 #include "df.h"
 #include "dbgcnt.h"
+#include "rtl-iter.h"
 
 /* This pass implements downward store motion.
    As of May 1, 2009, the pass is not enabled by default on any target,
@@ -278,19 +279,6 @@ store_ops_ok (const_rtx x, int *regs_set
   return true;
 }
 
-/* Helper for extract_mentioned_regs.  */
-
-static int
-extract_mentioned_regs_1 (rtx *loc, void *data)
-{
-  rtx *mentioned_regs_p = (rtx *) data;
-
-  if (REG_P (*loc))
-    *mentioned_regs_p = alloc_EXPR_LIST (0, *loc, *mentioned_regs_p);
-
-  return 0;
-}
-
 /* Returns a list of registers mentioned in X.
    FIXME: A regset would be prettier and less expensive.  */
 
@@ -298,7 +286,13 @@ extract_mentioned_regs_1 (rtx *loc, void
 extract_mentioned_regs (rtx x)
 {
   rtx mentioned_regs = NULL;
-  for_each_rtx (&x, extract_mentioned_regs_1, &mentioned_regs);
+  subrtx_var_iterator::array_type array;
+  FOR_EACH_SUBRTX_VAR (iter, array, x, NONCONST)
+    {
+      rtx x = *iter;
+      if (REG_P (x))
+	mentioned_regs = alloc_EXPR_LIST (0, x, mentioned_regs);
+    }
   return mentioned_regs;
 }
 

Re: [PATCH 43/50] store-motion.c:extract_mentioned_regs

[Jeff Law]

On 08/03/14 08:34, Richard Sandiford wrote:
> gcc/
> 	* store-motion.c: Include rtl-iter.h.
> 	(extract_mentioned_regs_1): Delete.
> 	(extract_mentioned_regs): Use FOR_EACH_SUBRTX_VAR rather than
> 	for_each_rtx to iterate over subrtxes.
OK.
jeff

[PATCH 44/50] var-tracking.c:rtx_debug_expr_p

[Richard Sandiford]

gcc/
	* var-tracking.c: Include rtl-iter.h.
	(rtx_debug_expr_p): Turn from being a for_each_rtx callback
	to being a function that examines each subrtx itself.
	(use_type): Update accordingly.

Index: gcc/var-tracking.c
===================================================================
--- gcc/var-tracking.c	2014-08-03 11:23:46.439128910 +0100
+++ gcc/var-tracking.c	2014-08-03 11:25:32.493177423 +0100
@@ -119,6 +119,7 @@
 #include "recog.h"
 #include "tm_p.h"
 #include "alias.h"
+#include "rtl-iter.h"
 
 /* var-tracking.c assumes that tree code with the same value as VALUE rtx code
    has no chance to appear in REG_EXPR/MEM_EXPRs and isn't a decl.
@@ -5366,16 +5367,16 @@ replace_expr_with_values (rtx loc)
     return cselib_subst_to_values (loc, VOIDmode);
 }
 
-/* Return true if *X is a DEBUG_EXPR.  Usable as an argument to
-   for_each_rtx to tell whether there are any DEBUG_EXPRs within
-   RTX.  */
+/* Return true if X contains a DEBUG_EXPR.  */
 
-static int
-rtx_debug_expr_p (rtx *x, void *data ATTRIBUTE_UNUSED)
+static bool
+rtx_debug_expr_p (const_rtx x)
 {
-  rtx loc = *x;
-
-  return GET_CODE (loc) == DEBUG_EXPR;
+  subrtx_iterator::array_type array;
+  FOR_EACH_SUBRTX (iter, array, x, ALL)
+    if (GET_CODE (*iter) == DEBUG_EXPR)
+      return true;
+  return false;
 }
 
 /* Determine what kind of micro operation to choose for a USE.  Return
@@ -5465,7 +5466,7 @@ use_type (rtx loc, struct count_use_info
 		  DEBUG_EXPRs (only happens in the presence of debug
 		  insns).  */
 	       && (!MAY_HAVE_DEBUG_INSNS
-		   || !for_each_rtx (&XEXP (loc, 0), rtx_debug_expr_p, NULL)))
+		   || !rtx_debug_expr_p (XEXP (loc, 0))))
 	return MO_USE;
       else
 	return MO_CLOBBER;

Re: [PATCH 44/50] var-tracking.c:rtx_debug_expr_p

[Jeff Law]

On 08/03/14 08:35, Richard Sandiford wrote:
> gcc/
> 	* var-tracking.c: Include rtl-iter.h.
> 	(rtx_debug_expr_p): Turn from being a for_each_rtx callback
> 	to being a function that examines each subrtx itself.
> 	(use_type): Update accordingly.
OK.
jeff

[PATCH 45/50] var-tracking.c:non_suitable_const

[Richard Sandiford]

gcc/
	* var-tracking.c (non_suitable_const): Turn from being a for_each_rtx
	callback to being a function that examines each subrtx itself.
	Remove handling of null rtxes.
	(add_uses): Update accordingly.

Index: gcc/var-tracking.c
===================================================================
--- gcc/var-tracking.c	2014-08-03 11:25:32.493177423 +0100
+++ gcc/var-tracking.c	2014-08-03 11:25:32.786180320 +0100
@@ -5523,27 +5523,32 @@ preserve_value (cselib_val *val)
    any rtxes not suitable for CONST use not replaced by VALUEs
    are discovered.  */
 
-static int
-non_suitable_const (rtx *x, void *data ATTRIBUTE_UNUSED)
+static bool
+non_suitable_const (const_rtx x)
 {
-  if (*x == NULL_RTX)
-    return 0;
-
-  switch (GET_CODE (*x))
+  subrtx_iterator::array_type array;
+  FOR_EACH_SUBRTX (iter, array, x, ALL)
     {
-    case REG:
-    case DEBUG_EXPR:
-    case PC:
-    case SCRATCH:
-    case CC0:
-    case ASM_INPUT:
-    case ASM_OPERANDS:
-      return 1;
-    case MEM:
-      return !MEM_READONLY_P (*x);
-    default:
-      return 0;
+      const_rtx x = *iter;
+      switch (GET_CODE (x))
+	{
+	case REG:
+	case DEBUG_EXPR:
+	case PC:
+	case SCRATCH:
+	case CC0:
+	case ASM_INPUT:
+	case ASM_OPERANDS:
+	  return true;
+	case MEM:
+	  if (!MEM_READONLY_P (x))
+	    return true;
+	  break;
+	default:
+	  break;
+	}
     }
+  return false;
 }
 
 /* Add uses (register and memory references) LOC which will be tracked
@@ -5589,8 +5594,7 @@ add_uses (rtx *ploc, void *data)
 	    }
 
 	  if (CONSTANT_P (vloc)
-	      && (GET_CODE (vloc) != CONST
-		  || for_each_rtx (&vloc, non_suitable_const, NULL)))
+	      && (GET_CODE (vloc) != CONST || non_suitable_const (vloc)))
 	    /* For constants don't look up any value.  */;
 	  else if (!VAR_LOC_UNKNOWN_P (vloc) && !unsuitable_loc (vloc)
 		   && (val = find_use_val (vloc, GET_MODE (oloc), cui)))

Re: [PATCH 45/50] var-tracking.c:non_suitable_const

[Jeff Law]

On 08/03/14 08:36, Richard Sandiford wrote:
> gcc/
> 	* var-tracking.c (non_suitable_const): Turn from being a for_each_rtx
> 	callback to being a function that examines each subrtx itself.
> 	Remove handling of null rtxes.
> 	(add_uses): Update accordingly.
OK.
Jeff

[PATCH 46/50] var-tracking.c:use_narrower_mode_test

[Richard Sandiford]

gcc/
	* var-tracking.c (use_narrower_mode_test): Turn from being a
	for_each_rtx callback to being a function that examines each
	subrtx itself.
	(adjust_mems): Update accordingly.

Index: gcc/var-tracking.c
===================================================================
--- gcc/var-tracking.c	2014-08-03 11:25:32.786180320 +0100
+++ gcc/var-tracking.c	2014-08-03 11:25:33.067183098 +0100
@@ -953,41 +953,41 @@ struct adjust_mem_data
   rtx side_effects;
 };
 
-/* Helper for adjust_mems.  Return 1 if *loc is unsuitable for
-   transformation of wider mode arithmetics to narrower mode,
-   -1 if it is suitable and subexpressions shouldn't be
-   traversed and 0 if it is suitable and subexpressions should
-   be traversed.  Called through for_each_rtx.  */
+/* Helper for adjust_mems.  Return true if X is suitable for
+   transformation of wider mode arithmetics to narrower mode.  */
 
-static int
-use_narrower_mode_test (rtx *loc, void *data)
+static bool
+use_narrower_mode_test (rtx x, const_rtx subreg)
 {
-  rtx subreg = (rtx) data;
-
-  if (CONSTANT_P (*loc))
-    return -1;
-  switch (GET_CODE (*loc))
+  subrtx_var_iterator::array_type array;
+  FOR_EACH_SUBRTX_VAR (iter, array, x, NONCONST)
     {
-    case REG:
-      if (cselib_lookup (*loc, GET_MODE (SUBREG_REG (subreg)), 0, VOIDmode))
-	return 1;
-      if (!validate_subreg (GET_MODE (subreg), GET_MODE (*loc),
-			    *loc, subreg_lowpart_offset (GET_MODE (subreg),
-							 GET_MODE (*loc))))
-	return 1;
-      return -1;
-    case PLUS:
-    case MINUS:
-    case MULT:
-      return 0;
-    case ASHIFT:
-      if (for_each_rtx (&XEXP (*loc, 0), use_narrower_mode_test, data))
-	return 1;
+      rtx x = *iter;
+      if (CONSTANT_P (x))
+	iter.skip_subrtxes ();
       else
-	return -1;
-    default:
-      return 1;
+	switch (GET_CODE (x))
+	  {
+	  case REG:
+	    if (cselib_lookup (x, GET_MODE (SUBREG_REG (subreg)), 0, VOIDmode))
+	      return false;
+	    if (!validate_subreg (GET_MODE (subreg), GET_MODE (x), x,
+				  subreg_lowpart_offset (GET_MODE (subreg),
+							 GET_MODE (x))))
+	      return false;
+	    break;
+	  case PLUS:
+	  case MINUS:
+	  case MULT:
+	    break;
+	  case ASHIFT:
+	    iter.substitute (XEXP (x, 0));
+	    break;
+	  default:
+	    return false;
+	  }
     }
+  return true;
 }
 
 /* Transform X into narrower mode MODE from wider mode WMODE.  */
@@ -1148,7 +1148,7 @@ adjust_mems (rtx loc, const_rtx old_rtx,
 	  && GET_MODE_SIZE (GET_MODE (tem))
 	     < GET_MODE_SIZE (GET_MODE (SUBREG_REG (tem)))
 	  && subreg_lowpart_p (tem)
-	  && !for_each_rtx (&SUBREG_REG (tem), use_narrower_mode_test, tem))
+	  && use_narrower_mode_test (SUBREG_REG (tem), tem))
 	return use_narrower_mode (SUBREG_REG (tem), GET_MODE (tem),
 				  GET_MODE (SUBREG_REG (tem)));
       return tem;

Re: [PATCH 46/50] var-tracking.c:use_narrower_mode_test

[Jeff Law]

On 08/03/14 08:38, Richard Sandiford wrote:
> gcc/
> 	* var-tracking.c (use_narrower_mode_test): Turn from being a
> 	for_each_rtx callback to being a function that examines each
> 	subrtx itself.
> 	(adjust_mems): Update accordingly.
OK.
jeff

[PATCH 47/50] var-tracking.c:add_uses

[Richard Sandiford]

gcc/
	* var-tracking.c (add_uses): Take an rtx rather than an rtx *.
	Give real type of data parameter.  Remove return value.
	(add_uses_1): Use FOR_EACH_SUBRTX_VAR rather than for_each_rtx
	to iterate over subrtxes.

Index: gcc/var-tracking.c
===================================================================
--- gcc/var-tracking.c	2014-08-03 11:25:33.067183098 +0100
+++ gcc/var-tracking.c	2014-08-03 11:25:33.326185659 +0100
@@ -666,7 +666,6 @@ static void dataflow_set_destroy (datafl
 static bool contains_symbol_ref (rtx);
 static bool track_expr_p (tree, bool);
 static bool same_variable_part_p (rtx, tree, HOST_WIDE_INT);
-static int add_uses (rtx *, void *);
 static void add_uses_1 (rtx *, void *);
 static void add_stores (rtx, const_rtx, void *);
 static bool compute_bb_dataflow (basic_block);
@@ -5552,14 +5551,12 @@ non_suitable_const (const_rtx x)
 }
 
 /* Add uses (register and memory references) LOC which will be tracked
-   to VTI (bb)->mos.  INSN is instruction which the LOC is part of.  */
+   to VTI (bb)->mos.  */
 
-static int
-add_uses (rtx *ploc, void *data)
+static void
+add_uses (rtx loc, struct count_use_info *cui)
 {
-  rtx loc = *ploc;
   enum machine_mode mode = VOIDmode;
-  struct count_use_info *cui = (struct count_use_info *)data;
   enum micro_operation_type type = use_type (loc, cui, &mode);
 
   if (type != MO_CLOBBER)
@@ -5705,8 +5702,6 @@ add_uses (rtx *ploc, void *data)
 	log_op_type (mo.u.loc, cui->bb, cui->insn, mo.type, dump_file);
       VTI (bb)->mos.safe_push (mo);
     }
-
-  return 0;
 }
 
 /* Helper function for finding all uses of REG/MEM in X in insn INSN.  */
@@ -5714,7 +5709,9 @@ add_uses (rtx *ploc, void *data)
 static void
 add_uses_1 (rtx *x, void *cui)
 {
-  for_each_rtx (x, add_uses, cui);
+  subrtx_var_iterator::array_type array;
+  FOR_EACH_SUBRTX_VAR (iter, array, *x, NONCONST)
+    add_uses (*iter, (struct count_use_info *) cui);
 }
 
 /* This is the value used during expansion of locations.  We want it

Re: [PATCH 47/50] var-tracking.c:add_uses

[Jeff Law]

On 08/03/14 08:38, Richard Sandiford wrote:
> gcc/
> 	* var-tracking.c (add_uses): Take an rtx rather than an rtx *.
> 	Give real type of data parameter.  Remove return value.
> 	(add_uses_1): Use FOR_EACH_SUBRTX_VAR rather than for_each_rtx
> 	to iterate over subrtxes.
OK.
jeff

[PATCH 48/50] varasm.c:const_rtx_hash

[Richard Sandiford]

const_rtx_hash_1 had code to hash all elements of a CONST_VECTOR,
but const_rtx_hash also hashes all subrtxes, so we'd end up hashing
the same thing twice.  This looked unintentional so I just removed the
CONST_VECTOR case.  If instead it was a deliberate decision then I think
it deserves a comment.


gcc/
	* varasm.c: Include rtl-iter.h.
	(const_rtx_hash_1): Take a const_rtx rather than an rtx *.
	Remove the pointer to the cumulative hashval_t and just return
	the hash for this rtx instead.  Remove recursive CONST_VECTOR case.
	(const_rtx_hash): Use FOR_EACH_SUBRTX instead of for_each_rtx.
	Accumulate the hashval_ts here instead of const_rtx_hash_1.

Index: gcc/varasm.c
===================================================================
--- gcc/varasm.c	2014-08-03 11:25:30.450157225 +0100
+++ gcc/varasm.c	2014-08-03 11:25:33.667189030 +0100
@@ -54,6 +54,7 @@ Software Foundation; either version 3, o
 #include "hash-set.h"
 #include "asan.h"
 #include "basic-block.h"
+#include "rtl-iter.h"
 
 #ifdef XCOFF_DEBUGGING_INFO
 #include "xcoffout.h"		/* Needed for external data
@@ -3481,19 +3482,17 @@ const_desc_rtx_eq (const void *a, const
   return rtx_equal_p (x->constant, y->constant);
 }
 
-/* This is the worker function for const_rtx_hash, called via for_each_rtx.  */
+/* Hash one component of a constant.  */
 
-static int
-const_rtx_hash_1 (rtx *xp, void *data)
+static hashval_t
+const_rtx_hash_1 (const_rtx x)
 {
   unsigned HOST_WIDE_INT hwi;
   enum machine_mode mode;
   enum rtx_code code;
-  hashval_t h, *hp;
-  rtx x;
+  hashval_t h;
   int i;
 
-  x = *xp;
   code = GET_CODE (x);
   mode = GET_MODE (x);
   h = (hashval_t) code * 1048573 + mode;
@@ -3539,14 +3538,6 @@ const_rtx_hash_1 (rtx *xp, void *data)
       h ^= fixed_hash (CONST_FIXED_VALUE (x));
       break;
 
-    case CONST_VECTOR:
-      {
-	int i;
-	for (i = XVECLEN (x, 0); i-- > 0; )
-	  h = h * 251 + const_rtx_hash_1 (&XVECEXP (x, 0, i), data);
-      }
-      break;
-
     case SYMBOL_REF:
       h ^= htab_hash_string (XSTR (x, 0));
       break;
@@ -3564,9 +3555,7 @@ const_rtx_hash_1 (rtx *xp, void *data)
       break;
     }
 
-  hp = (hashval_t *) data;
-  *hp = *hp * 509 + h;
-  return 0;
+  return h;
 }
 
 /* Compute a hash value for X, which should be a constant.  */
@@ -3575,7 +3564,9 @@ const_rtx_hash_1 (rtx *xp, void *data)
 const_rtx_hash (rtx x)
 {
   hashval_t h = 0;
-  for_each_rtx (&x, const_rtx_hash_1, &h);
+  subrtx_iterator::array_type array;
+  FOR_EACH_SUBRTX (iter, array, x, ALL)
+    h = h * 509 + const_rtx_hash_1 (*iter);
   return h;
 }
 

Re: [PATCH 48/50] varasm.c:const_rtx_hash

[Jeff Law]

On 08/03/14 08:42, Richard Sandiford wrote:
> const_rtx_hash_1 had code to hash all elements of a CONST_VECTOR,
> but const_rtx_hash also hashes all subrtxes, so we'd end up hashing
> the same thing twice.  This looked unintentional so I just removed the
> CONST_VECTOR case.  If instead it was a deliberate decision then I think
> it deserves a comment.
>
>
> gcc/
> 	* varasm.c: Include rtl-iter.h.
> 	(const_rtx_hash_1): Take a const_rtx rather than an rtx *.
> 	Remove the pointer to the cumulative hashval_t and just return
> 	the hash for this rtx instead.  Remove recursive CONST_VECTOR case.
> 	(const_rtx_hash): Use FOR_EACH_SUBRTX instead of for_each_rtx.
> 	Accumulate the hashval_ts here instead of const_rtx_hash_1.
This is fine.  Though I wonder if it's going to conflict with Andi's 
work on inchash.  If so, consider adjustments to handle Andi's changes 
pre-approved.  Just post the final patch for archival purposes if you 
need to change it.

Thanks,
jeff

[PATCH 49/50] varasm.c:mark_constants

[Richard Sandiford]

gcc/
	* varasm.c (mark_constant): Replace this for_each_rtx callback with...
	(mark_constants_in_pattern): ...this new function to iterate over
	all the subrtxes.
	(mark_constants): Update accordingly.

Index: gcc/varasm.c
===================================================================
--- gcc/varasm.c	2014-08-03 11:25:33.667189030 +0100
+++ gcc/varasm.c	2014-08-03 11:25:33.944191769 +0100
@@ -3867,38 +3867,38 @@ output_constant_pool_1 (struct constant_
   return;
 }
 
-/* Given a SYMBOL_REF CURRENT_RTX, mark it and all constants it refers
-   to as used.  Emit referenced deferred strings.  This function can
-   be used with for_each_rtx to mark all SYMBOL_REFs in an rtx.  */
+/* Mark all constants that are referenced by SYMBOL_REFs in X.
+   Emit referenced deferred strings.  */
 
-static int
-mark_constant (rtx *current_rtx, void *data ATTRIBUTE_UNUSED)
+static void
+mark_constants_in_pattern (rtx insn)
 {
-  rtx x = *current_rtx;
-
-  if (x == NULL_RTX || GET_CODE (x) != SYMBOL_REF)
-    return 0;
-
-  if (CONSTANT_POOL_ADDRESS_P (x))
-    {
-      struct constant_descriptor_rtx *desc = SYMBOL_REF_CONSTANT (x);
-      if (desc->mark == 0)
-	{
-	  desc->mark = 1;
-	  for_each_rtx (&desc->constant, mark_constant, NULL);
-	}
-    }
-  else if (TREE_CONSTANT_POOL_ADDRESS_P (x))
+  subrtx_iterator::array_type array;
+  FOR_EACH_SUBRTX (iter, array, PATTERN (insn), ALL)
     {
-      tree decl = SYMBOL_REF_DECL (x);
-      if (!TREE_ASM_WRITTEN (DECL_INITIAL (decl)))
+      const_rtx x = *iter;
+      if (GET_CODE (x) == SYMBOL_REF)
 	{
-	  n_deferred_constants--;
-	  output_constant_def_contents (x);
+	  if (CONSTANT_POOL_ADDRESS_P (x))
+	    {
+	      struct constant_descriptor_rtx *desc = SYMBOL_REF_CONSTANT (x);
+	      if (desc->mark == 0)
+		{
+		  desc->mark = 1;
+		  iter.substitute (desc->constant);
+		}
+	    }
+	  else if (TREE_CONSTANT_POOL_ADDRESS_P (x))
+	    {
+	      tree decl = SYMBOL_REF_DECL (x);
+	      if (!TREE_ASM_WRITTEN (DECL_INITIAL (decl)))
+		{
+		  n_deferred_constants--;
+		  output_constant_def_contents (CONST_CAST_RTX (x));
+		}
+	    }
 	}
     }
-
-  return -1;
 }
 
 /* Look through appropriate parts of INSN, marking all entries in the
@@ -3923,11 +3923,11 @@ mark_constants (rtx insn)
 	{
 	  rtx subinsn = XVECEXP (seq, 0, i);
 	  if (INSN_P (subinsn))
-	    for_each_rtx (&PATTERN (subinsn), mark_constant, NULL);
+	    mark_constants_in_pattern (subinsn);
 	}
     }
   else
-    for_each_rtx (&PATTERN (insn), mark_constant, NULL);
+    mark_constants_in_pattern (insn);
 }
 
 /* Look through the instructions for this function, and mark all the

Re: [PATCH 49/50] varasm.c:mark_constants

[Jeff Law]

On 08/03/14 08:43, Richard Sandiford wrote:
> gcc/
> 	* varasm.c (mark_constant): Replace this for_each_rtx callback with...
> 	(mark_constants_in_pattern): ...this new function to iterate over
> 	all the subrtxes.
> 	(mark_constants): Update accordingly.
OK.
jeff

[PATCH 50/50] varasm.c:compute_reloc_for_rtx

[Richard Sandiford]

There's no point calling for_each_rtx/FOR_EACH_SUBRTX on a LABEL_REF
or SYMBOL_REF.  We can just handle them directly instead.


gcc/
	* varasm.c (compute_reloc_for_rtx_1): Take a const_rtx.  Remove the
	pointer to the cumulative reloc value and return the value for
	this reloc instead.
	(compute_reloc_for_rtx): Take a const_rtx.  Call
	compute_reloc_for_rtx_1 directly for SYMBOL_REF and LABEL_REF,
	avoiding any recursion.  Use FOR_EACH_SUBRTX rather than
	for_each_rtx for the CONST case.

Index: gcc/varasm.c
===================================================================
--- gcc/varasm.c	2014-08-03 11:25:33.944191769 +0100
+++ gcc/varasm.c	2014-08-03 11:25:34.217194468 +0100
@@ -6484,44 +6484,43 @@ default_unique_section (tree decl, int r
   set_decl_section_name (decl, string);
 }
 
-/* Like compute_reloc_for_constant, except for an RTX.  The return value
-   is a mask for which bit 1 indicates a global relocation, and bit 0
-   indicates a local relocation.  */
+/* Subroutine of compute_reloc_for_rtx for leaf rtxes.  */
 
 static int
-compute_reloc_for_rtx_1 (rtx *xp, void *data)
+compute_reloc_for_rtx_1 (const_rtx x)
 {
-  int *preloc = (int *) data;
-  rtx x = *xp;
-
   switch (GET_CODE (x))
     {
     case SYMBOL_REF:
-      *preloc |= SYMBOL_REF_LOCAL_P (x) ? 1 : 2;
-      break;
+      return SYMBOL_REF_LOCAL_P (x) ? 1 : 2;
     case LABEL_REF:
-      *preloc |= 1;
-      break;
+      return 1;
     default:
-      break;
+      return 0;
     }
-
-  return 0;
 }
 
+/* Like compute_reloc_for_constant, except for an RTX.  The return value
+   is a mask for which bit 1 indicates a global relocation, and bit 0
+   indicates a local relocation.  */
+
 static int
-compute_reloc_for_rtx (rtx x)
+compute_reloc_for_rtx (const_rtx x)
 {
-  int reloc;
-
   switch (GET_CODE (x))
     {
-    case CONST:
     case SYMBOL_REF:
     case LABEL_REF:
-      reloc = 0;
-      for_each_rtx (&x, compute_reloc_for_rtx_1, &reloc);
-      return reloc;
+      return compute_reloc_for_rtx_1 (x);
+
+    case CONST:
+      {
+	int reloc = 0;
+	subrtx_iterator::array_type array;
+	FOR_EACH_SUBRTX (iter, array, x, ALL)
+	  reloc |= compute_reloc_for_rtx_1 (*iter);
+	return reloc;
+      }
 
     default:
       return 0;

Re: [PATCH 50/50] varasm.c:compute_reloc_for_rtx

[Jeff Law]

On 08/03/14 08:45, Richard Sandiford wrote:
> There's no point calling for_each_rtx/FOR_EACH_SUBRTX on a LABEL_REF
> or SYMBOL_REF.  We can just handle them directly instead.
>
>
> gcc/
> 	* varasm.c (compute_reloc_for_rtx_1): Take a const_rtx.  Remove the
> 	pointer to the cumulative reloc value and return the value for
> 	this reloc instead.
> 	(compute_reloc_for_rtx): Take a const_rtx.  Call
> 	compute_reloc_for_rtx_1 directly for SYMBOL_REF and LABEL_REF,
> 	avoiding any recursion.  Use FOR_EACH_SUBRTX rather than
> 	for_each_rtx for the CONST case.
OK.
Jeff