diff mbox

RFA: Merge definitions of get_some_local_dynamic_name

Message ID 8761fuim40.fsf@e105548-lin.cambridge.arm.com
State New
Headers show

Commit Message

Richard Sandiford Oct. 8, 2014, 4:04 p.m. UTC 
Rainer Orth <ro@cebitec.uni-bielefeld.de> writes:
> Hi Richard,
>
>> Does this work for you?  I tested it on x86_64-linux-gnu but obviously
>> that's not particularly useful for SEQUENCEs.
>
> the patch is fine, as tested on both sparc-sun-solaris2.11 and (for good
> measure) i386-pc-solaris2.11.

OK, great.

To recap, the idea is that if PATTERN (insn) is a SEQUENCE:

   FOR_EACH_SUBRTX (...., insn, x)
     ...

should iterate over the insns in the SEQUENCE (including pattern, notes,
jump label, etc.).  However:

   FOR_EACH_SUBRTX (...., PATTERN (insn), x)
     ...

should only iterate over the patterns of the insns in the SEQUENCE,
since the user of FOR_EACH_SUBRTX isn't expecting to see things like
REG_NOTES (and might not handle them correctly).

Also tested on x86_64-linux-gnu.  OK to install?

Thanks,
Richard


gcc/
	* rtlanal.c (generic_subrtx_iterator <T>::add_subrtxes_to_queue):
	Add the parts of an insn in reverse order, with the pattern at
	the top of the queue.  Detect when we're iterating over a SEQUENCE
	pattern and in that case just consider patterns of subinstructions.

Comments

Jeff Law Oct. 10, 2014, 7:53 p.m. UTC | #1 
On 10/08/14 10:04, Richard Sandiford wrote:
> Rainer Orth <ro@cebitec.uni-bielefeld.de> writes:
>> Hi Richard,
>>
>>> Does this work for you?  I tested it on x86_64-linux-gnu but obviously
>>> that's not particularly useful for SEQUENCEs.
>>
>> the patch is fine, as tested on both sparc-sun-solaris2.11 and (for good
>> measure) i386-pc-solaris2.11.
>
> OK, great.
>
> To recap, the idea is that if PATTERN (insn) is a SEQUENCE:
>
>     FOR_EACH_SUBRTX (...., insn, x)
>       ...
>
> should iterate over the insns in the SEQUENCE (including pattern, notes,
> jump label, etc.).  However:
>
>     FOR_EACH_SUBRTX (...., PATTERN (insn), x)
>       ...
>
> should only iterate over the patterns of the insns in the SEQUENCE,
> since the user of FOR_EACH_SUBRTX isn't expecting to see things like
> REG_NOTES (and might not handle them correctly).
>
> Also tested on x86_64-linux-gnu.  OK to install?
>
> Thanks,
> Richard
>
>
> gcc/
> 	* rtlanal.c (generic_subrtx_iterator <T>::add_subrtxes_to_queue):
> 	Add the parts of an insn in reverse order, with the pattern at
> 	the top of the queue.  Detect when we're iterating over a SEQUENCE
> 	pattern and in that case just consider patterns of subinstructions.
OK.
jeff
diff mbox

Patch

Index: gcc/rtlanal.c
===================================================================
--- gcc/rtlanal.c	2014-09-25 16:40:44.944406590 +0100
+++ gcc/rtlanal.c	2014-10-07 13:13:57.698132753 +0100
@@ -128,29 +128,58 @@  generic_subrtx_iterator <T>::add_subrtxe
 						    value_type *base,
 						    size_t end, rtx_type x)
 {
-  const char *format = GET_RTX_FORMAT (GET_CODE (x));
+  enum rtx_code code = GET_CODE (x);
+  const char *format = GET_RTX_FORMAT (code);
   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]));
-      }
+  if (__builtin_expect (INSN_P (x), false))
+    {
+      /* Put the pattern at the top of the queue, since that's what
+	 we're likely to want most.  It also allows for the SEQUENCE
+	 code below.  */
+      for (int i = GET_RTX_LENGTH (GET_CODE (x)) - 1; i >= 0; --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
+    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')
+	{
+	  unsigned 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 (unsigned int j = 0; j < length; j++)
+	      base[end++] = T::get_value (vec[j]);
+	  else
+	    for (unsigned int j = 0; j < length; j++)
+	      base = add_single_to_queue (array, base, end++,
+					  T::get_value (vec[j]));
+	  if (code == SEQUENCE && end == length)
+	    /* If the subrtxes of the sequence fill the entire array then
+	       we know that no other parts of a containing insn are queued.
+	       The caller is therefore iterating over the sequence as a
+	       PATTERN (...), so we also want the patterns of the
+	       subinstructions.  */
+	    for (unsigned int j = 0; j < length; j++)
+	      {
+		typename T::rtx_type x = T::get_rtx (base[j]);
+		if (INSN_P (x))
+		  base[j] = T::get_value (PATTERN (x));
+	      }
+	}
   return end - orig_end;
 }