Jeff Law <l...@redhat.com> writes:
> On 10/23/2017 05:21 AM, Richard Sandiford wrote:
>> This patch adds a way of treating certain kinds of CONST as unique,
>> so that pointer equality is equivalent to value equality.  For now it
>> is restricted to VEC_DUPLICATE and VEC_SERIES, although the code to
>> generate them remains in the else arm of an "if (1)" until a later
>> patch.
>> 
>> This is needed so that (const (vec_duplicate xx)) can used as the
>> CONSTxx_RTX of a variable-length vector.
> You're brave :-)  I know we looked at making CONST_INTs behave in this
> manner eons ago in an effort to reduce memory consumption and it was
> just plain painful.   There may still be comments from that project
> littering the source code.
>
> I do wonder if we might want to revisit this again as we have better
> infrastructure in place.

For vectors it isn't so bad, since we already do the same thing
for CONST_VECTOR.  Fortunately CONST_VECTOR and CONST always have
a mode, so there's no awkward sharing between modes...

>> 2017-10-23  Richard Sandiford  <richard.sandif...@linaro.org>
>>          Alan Hayward  <alan.hayw...@arm.com>
>>          David Sherwood  <david.sherw...@arm.com>
>> 
>> gcc/
>>      * rtl.h (unique_const_p): New function.
>>      (gen_rtx_CONST): Declare.
>>      * emit-rtl.c (const_hasher): New struct.
>>      (const_htab): New variable.
>>      (init_emit_once): Initialize it.
>>      (const_hasher::hash, const_hasher::equal): New functions.
>>      (gen_rtx_CONST): New function.
>>      (spare_vec_duplicate, spare_vec_series): New variables.
>>      (gen_const_vec_duplicate_1): Add code for use (const (vec_duplicate)),
>>      but disable it for now.
>>      (gen_const_vec_series): Likewise (const (vec_series)).
>>      * gengenrtl.c (special_rtx): Return true for CONST.
>>      * rtl.c (shared_const_p): Return true if unique_const_p.
> ISTM that you need an update the rtl.texi's structure sharing
> assumptions section to describe the new rules around CONSTs.

Oops, yeah.  How about the attached?

> So what's the purpose of the sparc_vec_* stuff that you're going to use
> in the future?  It looks like a single element cache to me.    Am I
> missing something?

No, that's right.  When looking up the const for (vec_duplicate x), say,
it's easier to create the vec_duplicate rtx first.  But if the lookup
succeeds (and so we already have an rtx with that value), we keep the
discarded vec_duplicate around so that we can reuse it for the next
lookup.

Thanks for the reviews,

Richard


2017-10-27  Richard Sandiford  <richard.sandif...@linaro.org>
            Alan Hayward  <alan.hayw...@arm.com>
            David Sherwood  <david.sherw...@arm.com>

gcc/
        * doc/rtl.texi: Document rtl sharing rules.
        * rtl.h (unique_const_p): New function.
        (gen_rtx_CONST): Declare.
        * emit-rtl.c (const_hasher): New struct.
        (const_htab): New variable.
        (init_emit_once): Initialize it.
        (const_hasher::hash, const_hasher::equal): New functions.
        (gen_rtx_CONST): New function.
        (spare_vec_duplicate, spare_vec_series): New variables.
        (gen_const_vec_duplicate_1): Add code for use (const (vec_duplicate)),
        but disable it for now.
        (gen_const_vec_series): Likewise (const (vec_series)).
        * gengenrtl.c (special_rtx): Return true for CONST.
        * rtl.c (shared_const_p): Return true if unique_const_p.

Index: gcc/doc/rtl.texi
===================================================================
--- gcc/doc/rtl.texi    2017-10-27 16:48:35.827706696 +0100
+++ gcc/doc/rtl.texi    2017-10-27 16:48:37.617270148 +0100
@@ -4197,6 +4197,20 @@ There is only one @code{pc} expression.
 @item
 There is only one @code{cc0} expression.
 
+@cindex @code{const}, RTL sharing
+@item
+There is only one instance of the following structures for a given
+@var{m}, @var{x} and @var{y}:
+@example
+(const:@var{m} (vec_duplicate:@var{m} @var{x}))
+(const:@var{m} (vec_series:@var{m} @var{x} @var{y}))
+@end example
+This means, for example, that for a given @var{n} there is only ever a
+single instance of an expression like:
+@example
+(const:V@var{n}DI (vec_duplicate:V@var{n}DI (const_int 0)))
+@end example
+
 @cindex @code{const_double}, RTL sharing
 @item
 There is only one @code{const_double} expression with value 0 for
Index: gcc/rtl.h
===================================================================
--- gcc/rtl.h   2017-10-27 16:48:37.433286940 +0100
+++ gcc/rtl.h   2017-10-27 16:48:37.619280894 +0100
@@ -2861,6 +2861,23 @@ vec_series_p (const_rtx x, rtx *base_out
   return const_vec_series_p (x, base_out, step_out);
 }
 
+/* Return true if there should only ever be one instance of (const X),
+   so that constants of this type can be compared using pointer equality.  */
+
+inline bool
+unique_const_p (const_rtx x)
+{
+  switch (GET_CODE (x))
+    {
+    case VEC_DUPLICATE:
+    case VEC_SERIES:
+      return true;
+
+    default:
+      return false;
+    }
+}
+
 /* Return the unpromoted (outer) mode of SUBREG_PROMOTED_VAR_P subreg X.  */
 
 inline scalar_int_mode
@@ -3560,6 +3577,7 @@ extern rtx_insn_list *gen_rtx_INSN_LIST
 gen_rtx_INSN (machine_mode mode, rtx_insn *prev_insn, rtx_insn *next_insn,
              basic_block bb, rtx pattern, int location, int code,
              rtx reg_notes);
+extern rtx gen_rtx_CONST (machine_mode, rtx);
 extern rtx gen_rtx_CONST_INT (machine_mode, HOST_WIDE_INT);
 extern rtx gen_rtx_CONST_VECTOR (machine_mode, rtvec);
 extern void set_mode_and_regno (rtx, machine_mode, unsigned int);
Index: gcc/emit-rtl.c
===================================================================
--- gcc/emit-rtl.c      2017-10-27 16:48:37.433286940 +0100
+++ gcc/emit-rtl.c      2017-10-27 16:48:37.618275521 +0100
@@ -175,6 +175,15 @@ struct const_fixed_hasher : ggc_cache_pt
 
 static GTY ((cache)) hash_table<const_fixed_hasher> *const_fixed_htab;
 
+/* A hash table storing unique CONSTs.  */
+struct const_hasher : ggc_cache_ptr_hash<rtx_def>
+{
+  static hashval_t hash (rtx x);
+  static bool equal (rtx x, rtx y);
+};
+
+static GTY ((cache)) hash_table<const_hasher> *const_htab;
+
 #define cur_insn_uid (crtl->emit.x_cur_insn_uid)
 #define cur_debug_insn_uid (crtl->emit.x_cur_debug_insn_uid)
 #define first_label_num (crtl->emit.x_first_label_num)
@@ -310,6 +319,28 @@ const_fixed_hasher::equal (rtx x, rtx y)
   return fixed_identical (CONST_FIXED_VALUE (a), CONST_FIXED_VALUE (b));
 }
 
+/* Returns a hash code for X (which is either an existing unique CONST
+   or an operand to gen_rtx_CONST).  */
+
+hashval_t
+const_hasher::hash (rtx x)
+{
+  if (GET_CODE (x) == CONST)
+    x = XEXP (x, 0);
+
+  int do_not_record_p = 0;
+  return hash_rtx (x, GET_MODE (x), &do_not_record_p, NULL, false);
+}
+
+/* Returns true if the operand of unique CONST X is equal to Y.  */
+
+bool
+const_hasher::equal (rtx x, rtx y)
+{
+  gcc_checking_assert (GET_CODE (x) == CONST);
+  return rtx_equal_p (XEXP (x, 0), y);
+}
+
 /* Return true if the given memory attributes are equal.  */
 
 bool
@@ -5772,16 +5803,55 @@ init_emit (void)
 #endif
 }
 
+rtx
+gen_rtx_CONST (machine_mode mode, rtx val)
+{
+  if (unique_const_p (val))
+    {
+      /* Look up the CONST in the hash table.  */
+      rtx *slot = const_htab->find_slot (val, INSERT);
+      if (*slot == 0)
+       *slot = gen_rtx_raw_CONST (mode, val);
+      return *slot;
+    }
+
+  return gen_rtx_raw_CONST (mode, val);
+}
+
+/* Temporary rtx used by gen_const_vec_duplicate_1.  */
+static GTY((deletable)) rtx spare_vec_duplicate;
+
 /* Like gen_const_vec_duplicate, but ignore const_tiny_rtx.  */
 
 static rtx
 gen_const_vec_duplicate_1 (machine_mode mode, rtx el)
 {
   int nunits = GET_MODE_NUNITS (mode);
-  rtvec v = rtvec_alloc (nunits);
-  for (int i = 0; i < nunits; ++i)
-    RTVEC_ELT (v, i) = el;
-  return gen_rtx_raw_CONST_VECTOR (mode, v);
+  if (1)
+    {
+      rtvec v = rtvec_alloc (nunits);
+
+      for (int i = 0; i < nunits; ++i)
+       RTVEC_ELT (v, i) = el;
+
+      return gen_rtx_raw_CONST_VECTOR (mode, v);
+    }
+  else
+    {
+      if (spare_vec_duplicate)
+       {
+         PUT_MODE (spare_vec_duplicate, mode);
+         XEXP (spare_vec_duplicate, 0) = el;
+       }
+      else
+       spare_vec_duplicate = gen_rtx_VEC_DUPLICATE (mode, el);
+
+      rtx res = gen_rtx_CONST (mode, spare_vec_duplicate);
+      if (XEXP (res, 0) == spare_vec_duplicate)
+       spare_vec_duplicate = NULL_RTX;
+
+      return res;
+    }
 }
 
 /* Generate a vector constant of mode MODE in which every element has
@@ -5843,6 +5913,9 @@ const_vec_series_p_1 (const_rtx x, rtx *
   return true;
 }
 
+/* Temporary rtx used by gen_const_vec_series.  */
+static GTY((deletable)) rtx spare_vec_series;
+
 /* Generate a vector constant of mode MODE in which element I has
    the value BASE + I * STEP.  */
 
@@ -5852,13 +5925,33 @@ gen_const_vec_series (machine_mode mode,
   gcc_assert (CONSTANT_P (base) && CONSTANT_P (step));
 
   int nunits = GET_MODE_NUNITS (mode);
-  rtvec v = rtvec_alloc (nunits);
-  scalar_mode inner_mode = GET_MODE_INNER (mode);
-  RTVEC_ELT (v, 0) = base;
-  for (int i = 1; i < nunits; ++i)
-    RTVEC_ELT (v, i) = simplify_gen_binary (PLUS, inner_mode,
-                                           RTVEC_ELT (v, i - 1), step);
-  return gen_rtx_raw_CONST_VECTOR (mode, v);
+  if (1)
+    {
+      rtvec v = rtvec_alloc (nunits);
+      scalar_mode inner_mode = GET_MODE_INNER (mode);
+      RTVEC_ELT (v, 0) = base;
+      for (int i = 1; i < nunits; ++i)
+       RTVEC_ELT (v, i) = simplify_gen_binary (PLUS, inner_mode,
+                                               RTVEC_ELT (v, i - 1), step);
+      return gen_rtx_raw_CONST_VECTOR (mode, v);
+    }
+  else
+    {
+      if (spare_vec_series)
+       {
+         PUT_MODE (spare_vec_series, mode);
+         XEXP (spare_vec_series, 0) = base;
+         XEXP (spare_vec_series, 1) = step;
+       }
+      else
+       spare_vec_series = gen_rtx_VEC_SERIES (mode, base, step);
+
+      rtx res = gen_rtx_CONST (mode, spare_vec_series);
+      if (XEXP (res, 0) == spare_vec_series)
+       spare_vec_series = NULL_RTX;
+
+      return res;
+    }
 }
 
 /* Generate a vector of mode MODE in which element I has the value
@@ -6016,6 +6109,8 @@ init_emit_once (void)
 
   reg_attrs_htab = hash_table<reg_attr_hasher>::create_ggc (37);
 
+  const_htab = hash_table<const_hasher>::create_ggc (37);
+
 #ifdef INIT_EXPANDERS
   /* This is to initialize {init|mark|free}_machine_status before the first
      call to push_function_context_to.  This is needed by the Chill front
Index: gcc/gengenrtl.c
===================================================================
--- gcc/gengenrtl.c     2017-10-27 16:48:37.433286940 +0100
+++ gcc/gengenrtl.c     2017-10-27 16:48:37.618275521 +0100
@@ -143,7 +143,8 @@ special_rtx (int idx)
          || strcmp (defs[idx].enumname, "CC0") == 0
          || strcmp (defs[idx].enumname, "RETURN") == 0
          || strcmp (defs[idx].enumname, "SIMPLE_RETURN") == 0
-         || strcmp (defs[idx].enumname, "CONST_VECTOR") == 0);
+         || strcmp (defs[idx].enumname, "CONST_VECTOR") == 0
+         || strcmp (defs[idx].enumname, "CONST") == 0);
 }
 
 /* Return nonzero if the RTL code given by index IDX is one that we should
Index: gcc/rtl.c
===================================================================
--- gcc/rtl.c   2017-10-27 16:48:37.433286940 +0100
+++ gcc/rtl.c   2017-10-27 16:48:37.618275521 +0100
@@ -252,6 +252,9 @@ shared_const_p (const_rtx orig)
 {
   gcc_assert (GET_CODE (orig) == CONST);
 
+  if (unique_const_p (XEXP (orig, 0)))
+    return true;
+
   /* CONST can be shared if it contains a SYMBOL_REF.  If it contains
      a LABEL_REF, it isn't sharable.  */
   return (GET_CODE (XEXP (orig, 0)) == PLUS

Reply via email to