diff mbox

[ARM,LRA] Prepare ARM build with LRA

Message ID 8761tnlfft.fsf@sandifor-thinkpad.stglab.manchester.uk.ibm.com
State New
Headers show

Commit Message

Richard Sandiford Sept. 26, 2013, 4:23 p.m. UTC 
Eric Botcazou <ebotcazou@adacore.com> writes:
>> So in the set_* routines it isn't about whether the value is definitely
>> a base or a definitely an index.  It's just about drilling down through
>> what we've already decided is a base or index to get the inner reg or mem,
>> and knowing which XEXPs to look at.  We could instead have used a
>> for_each_rtx, or something like that, without any code checks.  But I
>> wanted to be precise about the types of address we allow, so that we can
>> assert for things we don't understand.  In other words, it was "designed"
>> to require the kind of extension Yvan is adding here.
>
> Does this mean that the design is to require a parallel implementation in the 
> predicates and in the set routines, i.e. each time you add a new case to the 
> predicates, you need to add it (or do something) to the set routines as well?  
> If so, that's a little weird, but OK, feel free to revert the de-duplication 
> part, but add comments saying that the functions must be kept synchronized.

They don't need to be kept synchronised as such.  It's fine for the index
to allow more than must_be_index_p.  But if you're not keen on the current
structure, does the following look better?  Tested on x86_64-linux-gnu.

Thanks,
Richard


gcc/
	* rtlanal.c (must_be_base_p, must_be_index_p): Delete.
	(binary_scale_code_p, get_base_term, get_index_term): New functions.
	(set_address_segment, set_address_base, set_address_index)
	(set_address_disp): Accept the argument unconditionally.
	(baseness): Remove must_be_base_p and must_be_index_p checks.
	(decompose_normal_address): Classify as much as possible in the
	main loop.

Comments

Eric Botcazou Sept. 27, 2013, 8:40 a.m. UTC | #1 
> They don't need to be kept synchronised as such.  It's fine for the index
> to allow more than must_be_index_p.  But if you're not keen on the current
> structure, does the following look better?  Tested on x86_64-linux-gnu.
> 
> Thanks,
> Richard
> 
> 
> gcc/
> 	* rtlanal.c (must_be_base_p, must_be_index_p): Delete.
> 	(binary_scale_code_p, get_base_term, get_index_term): New functions.
> 	(set_address_segment, set_address_base, set_address_index)
> 	(set_address_disp): Accept the argument unconditionally.
> 	(baseness): Remove must_be_base_p and must_be_index_p checks.
> 	(decompose_normal_address): Classify as much as possible in the
> 	main loop.

Yes, fine by me, thanks.
diff mbox

Patch

Index: gcc/rtlanal.c
===================================================================
--- gcc/rtlanal.c	2013-09-25 22:42:16.870221821 +0100
+++ gcc/rtlanal.c	2013-09-26 09:11:41.273778750 +0100
@@ -5521,26 +5521,50 @@  strip_address_mutations (rtx *loc, enum
     }
 }
 
-/* Return true if X must be a base rather than an index.  */
+/* Return true if CODE applies some kind of scale.  The scaled value is
+   is the first operand and the scale is the second.  */
 
 static bool
-must_be_base_p (rtx x)
+binary_scale_code_p (enum rtx_code code)
 {
-  return GET_CODE (x) == LO_SUM;
+  return (code == MULT
+          || code == ASHIFT
+          /* Needed by ARM targets.  */
+          || code == ASHIFTRT
+          || code == LSHIFTRT
+          || code == ROTATE
+          || code == ROTATERT);
 }
 
-/* Return true if X must be an index rather than a base.  */
+/* If *INNER can be interpreted as a base, return a pointer to the inner term
+   (see address_info).  Return null otherwise.  */
 
-static bool
-must_be_index_p (rtx x)
+static rtx *
+get_base_term (rtx *inner)
 {
-  return (GET_CODE (x) == MULT
-          || GET_CODE (x) == ASHIFT
-          /* Needed by ARM targets.  */
-          || GET_CODE (x) == ASHIFTRT
-          || GET_CODE (x) == LSHIFTRT
-          || GET_CODE (x) == ROTATE
-          || GET_CODE (x) == ROTATERT);
+  if (GET_CODE (*inner) == LO_SUM)
+    inner = strip_address_mutations (&XEXP (*inner, 0));
+  if (REG_P (*inner)
+      || MEM_P (*inner)
+      || GET_CODE (*inner) == SUBREG)
+    return inner;
+  return 0;
+}
+
+/* If *INNER can be interpreted as an index, return a pointer to the inner term
+   (see address_info).  Return null otherwise.  */
+
+static rtx *
+get_index_term (rtx *inner)
+{
+  /* At present, only constant scales are allowed.  */
+  if (binary_scale_code_p (GET_CODE (*inner)) && CONSTANT_P (XEXP (*inner, 1)))
+    inner = strip_address_mutations (&XEXP (*inner, 0));
+  if (REG_P (*inner)
+      || MEM_P (*inner)
+      || GET_CODE (*inner) == SUBREG)
+    return inner;
+  return 0;
 }
 
 /* Set the segment part of address INFO to LOC, given that INNER is the
@@ -5549,8 +5573,6 @@  must_be_index_p (rtx x)
 static void
 set_address_segment (struct address_info *info, rtx *loc, rtx *inner)
 {
-  gcc_checking_assert (GET_CODE (*inner) == UNSPEC);
-
   gcc_assert (!info->segment);
   info->segment = loc;
   info->segment_term = inner;
@@ -5562,12 +5584,6 @@  set_address_segment (struct address_info
 static void
 set_address_base (struct address_info *info, rtx *loc, rtx *inner)
 {
-  if (must_be_base_p (*inner))
-    inner = strip_address_mutations (&XEXP (*inner, 0));
-  gcc_checking_assert (REG_P (*inner)
-		       || MEM_P (*inner)
-		       || GET_CODE (*inner) == SUBREG);
-
   gcc_assert (!info->base);
   info->base = loc;
   info->base_term = inner;
@@ -5579,12 +5595,6 @@  set_address_base (struct address_info *i
 static void
 set_address_index (struct address_info *info, rtx *loc, rtx *inner)
 {
-  if (must_be_index_p (*inner) && CONSTANT_P (XEXP (*inner, 1)))
-    inner = strip_address_mutations (&XEXP (*inner, 0));
-  gcc_checking_assert (REG_P (*inner)
-		       || MEM_P (*inner)
-		       || GET_CODE (*inner) == SUBREG);
-
   gcc_assert (!info->index);
   info->index = loc;
   info->index_term = inner;
@@ -5596,8 +5606,6 @@  set_address_index (struct address_info *
 static void
 set_address_disp (struct address_info *info, rtx *loc, rtx *inner)
 {
-  gcc_checking_assert (CONSTANT_P (*inner));
-
   gcc_assert (!info->disp);
   info->disp = loc;
   info->disp_term = inner;
@@ -5677,12 +5685,6 @@  extract_plus_operands (rtx *loc, rtx **p
 baseness (rtx x, enum machine_mode mode, addr_space_t as,
 	  enum rtx_code outer_code, enum rtx_code index_code)
 {
-  /* See whether we can be certain.  */
-  if (must_be_base_p (x))
-    return 3;
-  if (must_be_index_p (x))
-    return -3;
-
   /* Believe *_POINTER unless the address shape requires otherwise.  */
   if (REG_P (x) && REG_POINTER (x))
     return 2;
@@ -5717,8 +5719,8 @@  decompose_normal_address (struct address
   if (n_ops > 1)
     info->base_outer_code = PLUS;
 
-  /* Separate the parts that contain a REG or MEM from those that don't.
-     Record the latter in INFO and leave the former in OPS.  */
+  /* Try to classify each sum operand now.  Leave those that could be
+     either a base or an index in OPS.  */
   rtx *inner_ops[4];
   size_t out = 0;
   for (size_t in = 0; in < n_ops; ++in)
@@ -5731,18 +5733,31 @@  decompose_normal_address (struct address
 	set_address_segment (info, loc, inner);
       else
 	{
-	  ops[out] = loc;
-	  inner_ops[out] = inner;
-	  ++out;
+	  /* The only other possibilities are a base or an index.  */
+	  rtx *base_term = get_base_term (inner);
+	  rtx *index_term = get_index_term (inner);
+	  gcc_assert (base_term || index_term);
+	  if (!base_term)
+	    set_address_index (info, loc, index_term);
+	  else if (!index_term)
+	    set_address_base (info, loc, base_term);
+	  else
+	    {
+	      gcc_assert (base_term == index_term);
+	      ops[out] = loc;
+	      inner_ops[out] = base_term;
+	      ++out;
+	    }
 	}
     }
 
   /* Classify the remaining OPS members as bases and indexes.  */
   if (out == 1)
     {
-      /* Assume that the remaining value is a base unless the shape
-	 requires otherwise.  */
-      if (!must_be_index_p (*inner_ops[0]))
+      /* If we haven't seen a base or an index yet, assume that this is
+	 the base.  If we were confident that another term was the base
+	 or index, treat the remaining operand as the other kind.  */
+      if (!info->base)
 	set_address_base (info, ops[0], inner_ops[0]);
       else
 	set_address_index (info, ops[0], inner_ops[0]);