This is the mail archive of the gcc-patches@gcc.gnu.org mailing list for the GCC project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

RFA: Clean up ADDRESS handling in alias.c


The comment in alias.c says:

   The contents of an ADDRESS is not normally used, the mode of the
   ADDRESS determines whether the ADDRESS is a function argument or some
   other special value.  Pointer equality, not rtx_equal_p, determines whether
   two ADDRESS expressions refer to the same base address.

   The only use of the contents of an ADDRESS is for determining if the
   current function performs nonlocal memory memory references for the
   purposes of marking the function as a constant function.  */

The first paragraph is a bit misleading IMO.  AFAICT, rtx_equal_p has
always given ADDRESS the full recursive treatment, rather than saying
that pointer equality determines ADDRESS equality.  (This is in contrast
to something like VALUE, where pointer equality is used.)  And AFAICT
we've always had:

static int
base_alias_check (rtx x, rtx y, enum machine_mode x_mode,
		  enum machine_mode y_mode)
{
  ...
  /* If the base addresses are equal nothing is known about aliasing.  */
  if (rtx_equal_p (x_base, y_base))
    return 1;
  ...
}

So I think the contents of an ADDRESS _are_ used to distinguish
between different bases.

The second paragraph ceased to be true in 2005 when the pure/const
analysis moved to its own IPA pass.  Nothing now looks at the contents
beyond rtx_equal_p.

Also, base_alias_check effectively treats all arguments as a single base.
That makes conceptual sense, because this analysis isn't strong enough
to determine whether arguments are base values at all, never mind whether
accesses based on different arguments conflict.  But the fact that we have
a single base isn't obvious from the way the code is written, because we
create several separate, non-rtx_equal_p, ADDRESSes to represent arguments.
See:

  for (i = 0; i < FIRST_PSEUDO_REGISTER; i++)
    /* Check whether this register can hold an incoming pointer
       argument.  FUNCTION_ARG_REGNO_P tests outgoing register
       numbers, so translate if necessary due to register windows.  */
    if (FUNCTION_ARG_REGNO_P (OUTGOING_REGNO (i))
	&& HARD_REGNO_MODE_OK (i, Pmode))
      static_reg_base_value[i]
	= gen_rtx_ADDRESS (VOIDmode, gen_rtx_REG (Pmode, i));

and:

      /* Check for an argument passed in memory.  Only record in the
	 copying-arguments block; it is too hard to track changes
	 otherwise.  */
      if (copying_arguments
	  && (XEXP (src, 0) == arg_pointer_rtx
	      || (GET_CODE (XEXP (src, 0)) == PLUS
		  && XEXP (XEXP (src, 0), 0) == arg_pointer_rtx)))
	return gen_rtx_ADDRESS (VOIDmode, src);

I think it would be cleaner and less wasteful to use a single rtx for
the single "base" (really "potential base").

So if we wanted to, we could now remove the operand from ADDRESS and
simply rely on pointer equality.  I'm a bit reluctant to do that though.
It would make debugging harder, and it would mean either adding knowledge
of this alias-specific code to other files (specifically rtl.c:rtx_equal_p),
or adding special ADDRESS shortcuts to alias.c.  But I think the code
would be more obvious if we replaced the rtx operand with a unique id,
which is what we already use for the REG_NOALIAS case:

      new_reg_base_value[regno] = gen_rtx_ADDRESS (Pmode,
						   GEN_INT (unique_id++));

And if we do that, we can make the id a direct operand of the ADDRESS,
rather than a CONST_INT subrtx[*].  That should make rtx_equal_p cheaper too.

  [*] I'm trying to get rid of CONST_INTs like these that have
      no obvious mode.

All of which led to the patch below.  I checked that it didn't change
the code generated at -O2 for a recent set of cc1 .ii files.  Also
bootstrapped & regression-tested on x86_64-linux-gnu.  OK to install?

To cover my back: I'm just trying to rewrite the current code according
to its current assumptions.  Whether those assumptions are correct or not
is always open to debate...

Richard


gcc/
	* rtl.def (ADDRESS): Turn operand into a HOST_WIDE_INT.
	* alias.c (reg_base_value): Expand and update comment.
	(arg_base_value): New variable.
	(unique_id): Move up file.
	(unique_base_value, unique_base_value_p, known_base_value_p): New.
	(find_base_value): Use arg_base_value and known_base_value_p.
	(record_set): Document REG_NOALIAS handling.  Use unique_base_value.
	(find_base_term): Use known_base_value_p.
	(base_alias_check): Use unique_base_value_p.
	(init_alias_target): Initialize arg_base_value.  Use unique_base_value.
	(init_alias_analysis): Use 1 as the first id for REG_NOALIAS bases.

Index: gcc/rtl.def
===================================================================
--- gcc/rtl.def	2012-04-15 15:23:52.747632394 +0100
+++ gcc/rtl.def	2012-04-15 15:58:48.234515667 +0100
@@ -109,8 +109,8 @@ DEF_RTL_EXPR(INSN_LIST, "insn_list", "ue
    `emit_insn' takes the SEQUENCE apart and makes separate insns.  */
 DEF_RTL_EXPR(SEQUENCE, "sequence", "E", RTX_EXTRA)
 
-/* Refers to the address of its argument.  This is only used in alias.c.  */
-DEF_RTL_EXPR(ADDRESS, "address", "e", RTX_MATCH)
+/* Represents a non-global base address.  This is only used in alias.c.  */
+DEF_RTL_EXPR(ADDRESS, "address", "w", RTX_EXTRA)
 
 /* ----------------------------------------------------------------------
    Expression types used for things in the instruction chain.
Index: gcc/alias.c
===================================================================
--- gcc/alias.c	2012-04-15 15:23:52.748632394 +0100
+++ gcc/alias.c	2012-04-15 15:59:45.073512500 +0100
@@ -187,21 +187,42 @@ #define MAX_ALIAS_LOOP_PASSES 10
    of the first set.
 
    A base address can be an ADDRESS, SYMBOL_REF, or LABEL_REF.  ADDRESS
-   expressions represent certain special values: function arguments and
-   the stack, frame, and argument pointers.
+   expressions represent three types of base:
 
-   The contents of an ADDRESS is not normally used, the mode of the
-   ADDRESS determines whether the ADDRESS is a function argument or some
-   other special value.  Pointer equality, not rtx_equal_p, determines whether
-   two ADDRESS expressions refer to the same base address.
-
-   The only use of the contents of an ADDRESS is for determining if the
-   current function performs nonlocal memory memory references for the
-   purposes of marking the function as a constant function.  */
+     1. incoming arguments.  There is just one ADDRESS to represent all
+	arguments, since we do not know at this level whether accesses
+	based on different arguments can alias.  The ADDRESS has id 0.
+
+     2. stack_pointer_rtx, frame_pointer_rtx, hard_frame_pointer_rtx
+	(if distinct from frame_pointer_rtx) and arg_pointer_rtx.
+	Each of these rtxes has a separate ADDRESS associated with it,
+	each with a negative id.
+
+	GCC is (and is required to be) precise in which register it
+	chooses to access a particular region of stack.  We can therefore
+	assume that accesses based on one of these rtxes do not alias
+	accesses based on another of these rtxes.
+
+     3. bases that are derived from malloc()ed memory (REG_NOALIAS).
+	Each such piece of memory has a separate ADDRESS associated
+	with it, each with an id greater than 0.
+
+   Accesses based on one ADDRESS do not alias accesses based on other
+   ADDRESSes.  Accesses based on ADDRESSes in groups (2) and (3) do not
+   alias globals either; the ADDRESSes have Pmode to indicate this.
+   The ADDRESS in group (1) _may_ alias globals; it has VOIDmode to
+   indicate this.  */
 
 static GTY(()) VEC(rtx,gc) *reg_base_value;
 static rtx *new_reg_base_value;
 
+/* The single VOIDmode ADDRESS that represents all argument bases.
+   It has id 0.  */
+static GTY(()) rtx arg_base_value;
+
+/* Used to allocate unique ids to each REG_NOALIAS ADDRESS.  */
+static int unique_id;
+
 /* We preserve the copy of old array around to avoid amount of garbage
    produced.  About 8% of garbage produced were attributed to this
    array.  */
@@ -993,6 +1014,43 @@ get_frame_alias_set (void)
   return frame_set;
 }
 
+/* Create a new, unique base with id ID.  */
+
+static rtx
+unique_base_value (HOST_WIDE_INT id)
+{
+  return gen_rtx_ADDRESS (Pmode, id);
+}
+
+/* Return true if accesses based on any other base value cannot access
+   those based on X.  */
+
+static bool
+unique_base_value_p (rtx x)
+{
+  return GET_CODE (x) == ADDRESS && GET_MODE (x) == Pmode;
+}
+
+/* Return true if X is known to be a base value.  */
+
+static bool
+known_base_value_p (rtx x)
+{
+  switch (GET_CODE (x))
+    {
+    case LABEL_REF:
+    case SYMBOL_REF:
+      return true;
+
+    case ADDRESS:
+      /* Arguments may or may not be bases; we don't know for sure.  */
+      return GET_MODE (x) != VOIDmode;
+
+    default:
+      return false;
+    }
+}
+
 /* Inside SRC, the source of a SET, find a base address.  */
 
 static rtx
@@ -1049,7 +1107,7 @@ find_base_value (rtx src)
 	  && (XEXP (src, 0) == arg_pointer_rtx
 	      || (GET_CODE (XEXP (src, 0)) == PLUS
 		  && XEXP (XEXP (src, 0), 0) == arg_pointer_rtx)))
-	return gen_rtx_ADDRESS (VOIDmode, src);
+	return arg_base_value;
       return 0;
 
     case CONST:
@@ -1090,18 +1148,10 @@ find_base_value (rtx src)
 	/* If either base is named object or a special address
 	   (like an argument or stack reference), then use it for the
 	   base term.  */
-	if (src_0 != 0
-	    && (GET_CODE (src_0) == SYMBOL_REF
-		|| GET_CODE (src_0) == LABEL_REF
-		|| (GET_CODE (src_0) == ADDRESS
-		    && GET_MODE (src_0) != VOIDmode)))
+	if (src_0 != 0 && known_base_value_p (src_0))
 	  return src_0;
 
-	if (src_1 != 0
-	    && (GET_CODE (src_1) == SYMBOL_REF
-		|| GET_CODE (src_1) == LABEL_REF
-		|| (GET_CODE (src_1) == ADDRESS
-		    && GET_MODE (src_1) != VOIDmode)))
+	if (src_1 != 0 && known_base_value_p (src_1))
 	  return src_1;
 
 	/* Guess which operand is the base address:
@@ -1169,16 +1219,14 @@ find_base_value (rtx src)
   return 0;
 }
 
-/* Called from init_alias_analysis indirectly through note_stores.  */
+/* Called from init_alias_analysis indirectly through note_stores,
+   or directly if DEST is a register with a REG_NOALIAS note attached.
+   SET is null in the latter case.  */
 
 /* While scanning insns to find base values, reg_seen[N] is nonzero if
    register N has been set in this function.  */
 static char *reg_seen;
 
-/* Addresses which are known not to alias anything else are identified
-   by a unique integer.  */
-static int unique_id;
-
 static void
 record_set (rtx dest, const_rtx set, void *data ATTRIBUTE_UNUSED)
 {
@@ -1223,14 +1271,14 @@ record_set (rtx dest, const_rtx set, voi
     }
   else
     {
+      /* There's a REG_NOALIAS note against DEST.  */
       if (reg_seen[regno])
 	{
 	  new_reg_base_value[regno] = 0;
 	  return;
 	}
       reg_seen[regno] = 1;
-      new_reg_base_value[regno] = gen_rtx_ADDRESS (Pmode,
-						   GEN_INT (unique_id++));
+      new_reg_base_value[regno] = unique_base_value (unique_id++);
       return;
     }
 
@@ -1666,18 +1714,10 @@ find_base_term (rtx x)
 	/* If either base term is named object or a special address
 	   (like an argument or stack reference), then use it for the
 	   base term.  */
-	if (tmp1 != 0
-	    && (GET_CODE (tmp1) == SYMBOL_REF
-		|| GET_CODE (tmp1) == LABEL_REF
-		|| (GET_CODE (tmp1) == ADDRESS
-		    && GET_MODE (tmp1) != VOIDmode)))
+	if (tmp1 != 0 && known_base_value_p (tmp1))
 	  return tmp1;
 
-	if (tmp2 != 0
-	    && (GET_CODE (tmp2) == SYMBOL_REF
-		|| GET_CODE (tmp2) == LABEL_REF
-		|| (GET_CODE (tmp2) == ADDRESS
-		    && GET_MODE (tmp2) != VOIDmode)))
+	if (tmp2 != 0 && known_base_value_p (tmp2))
 	  return tmp2;
 
 	/* We could not determine which of the two operands was the
@@ -1762,12 +1802,7 @@ base_alias_check (rtx x, rtx y, enum mac
   if (GET_CODE (x_base) != ADDRESS && GET_CODE (y_base) != ADDRESS)
     return 0;
 
-  /* If one address is a stack reference there can be no alias:
-     stack references using different base registers do not alias,
-     a stack reference can not alias a parameter, and a stack reference
-     can not alias a global.  */
-  if ((GET_CODE (x_base) == ADDRESS && GET_MODE (x_base) == Pmode)
-      || (GET_CODE (y_base) == ADDRESS && GET_MODE (y_base) == Pmode))
+  if (unique_base_value_p (x_base) || unique_base_value_p (y_base))
     return 0;
 
   return 1;
@@ -2686,6 +2721,9 @@ init_alias_target (void)
 {
   int i;
 
+  if (!arg_base_value)
+    arg_base_value = gen_rtx_ADDRESS (VOIDmode, 0);
+
   memset (static_reg_base_value, 0, sizeof static_reg_base_value);
 
   for (i = 0; i < FIRST_PSEUDO_REGISTER; i++)
@@ -2694,18 +2732,13 @@ init_alias_target (void)
        numbers, so translate if necessary due to register windows.  */
     if (FUNCTION_ARG_REGNO_P (OUTGOING_REGNO (i))
 	&& HARD_REGNO_MODE_OK (i, Pmode))
-      static_reg_base_value[i]
-	= gen_rtx_ADDRESS (VOIDmode, gen_rtx_REG (Pmode, i));
+      static_reg_base_value[i] = arg_base_value;
 
-  static_reg_base_value[STACK_POINTER_REGNUM]
-    = gen_rtx_ADDRESS (Pmode, stack_pointer_rtx);
-  static_reg_base_value[ARG_POINTER_REGNUM]
-    = gen_rtx_ADDRESS (Pmode, arg_pointer_rtx);
-  static_reg_base_value[FRAME_POINTER_REGNUM]
-    = gen_rtx_ADDRESS (Pmode, frame_pointer_rtx);
+  static_reg_base_value[STACK_POINTER_REGNUM] = unique_base_value (-1);
+  static_reg_base_value[ARG_POINTER_REGNUM] = unique_base_value (-2);
+  static_reg_base_value[FRAME_POINTER_REGNUM] = unique_base_value (-3);
 #if !HARD_FRAME_POINTER_IS_FRAME_POINTER
-  static_reg_base_value[HARD_FRAME_POINTER_REGNUM]
-    = gen_rtx_ADDRESS (Pmode, hard_frame_pointer_rtx);
+  static_reg_base_value[HARD_FRAME_POINTER_REGNUM] = unique_base_value (-4);
 #endif
 }
 
@@ -2791,8 +2824,8 @@ init_alias_analysis (void)
       changed = 0;
 
       /* We want to assign the same IDs each iteration of this loop, so
-	 start counting from zero each iteration of the loop.  */
-      unique_id = 0;
+	 start counting from one each iteration of the loop.  */
+      unique_id = 1;
 
       /* We're at the start of the function each iteration through the
 	 loop, so we're copying arguments.  */


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]