public inbox for gcc-patches@gcc.gnu.org
 help / color / mirror / Atom feed
* [PATCH 0/6] remove some usage of rtx_{insn,expr}_list
@ 2016-06-20 10:15 tbsaunde+gcc
  2016-06-20 10:15 ` [PATCH 2/6] remove unused loads rtx_insn_list tbsaunde+gcc
                   ` (6 more replies)
  0 siblings, 7 replies; 15+ messages in thread
From: tbsaunde+gcc @ 2016-06-20 10:15 UTC (permalink / raw)
  To: gcc-patches

From: Trevor Saunders <tbsaunde+gcc@tbsaunde.org>

Hi,

These few patches to get rid of rtx insn and expr lists should be pretty un
controvertial.  In each case the list is clearly used as a stack and using a
vec as a stack is clearly the same.

In theory I would expect if anything this helps performance since it isn't
necessary to malloc every time a node is added, however the data is less clear.
Here are times for O2 and O0 for fold-const.ii from gcc, and
Unified_js_src_cpp_32.ii from Spider Monkey.  These are best of 3 based on user
time.

fold const O2 new
real    0m5.034s
user    0m3.408s
sys     0m0.364s

fold const O2 old
real    0m4.012s
user    0m3.420s
sys     0m0.340s

fold const O0 new
real    0m1.637s
user    0m1.124s
sys     0m0.280s

fold const O0 old
real    0m2.483s
user    0m1.092s
sys     0m0.280s

mozjs O2 new
real    0m15.565s
user    0m12.420s
sys     0m1.536s

mozjs O2 old
real    0m13.662s
user    0m12.136s
sys     0m1.440s

mozjs O0 new
real    0m9.860s
user    0m6.796s
sys     0m1.368s

mozjs O0 old
real    0m8.922s
user    0m6.888s
sys     0m1.388s

So a couple got about .3s slower, and others got about .1 faster, I'm not
really sure but inclined to say any change is too small to easily measure.

bootstrapped + regtested patches individually on x86_64-linux-gnu, ok?

Trev

Trevor Saunders (6):
  make antic_stores a vec<rtx_insn *>
  remove unused loads rtx_insn_list
  make stores rtx_insn_list a vec
  make side_effects a vec<rtx>
  make pattern_regs a vec
  loop-iv.c: make cond_list a vec

 gcc/gcse.c         |  36 ++++++-------------
 gcc/loop-iv.c      |  55 ++++++++++------------------
 gcc/store-motion.c | 103 +++++++++++++++++++++++++----------------------------
 gcc/var-tracking.c |  33 ++++++++---------
 4 files changed, 90 insertions(+), 137 deletions(-)

-- 
2.7.4

^ permalink raw reply	[flat|nested] 15+ messages in thread

* [PATCH 3/6] make stores rtx_insn_list a vec
  2016-06-20 10:15 [PATCH 0/6] remove some usage of rtx_{insn,expr}_list tbsaunde+gcc
  2016-06-20 10:15 ` [PATCH 2/6] remove unused loads rtx_insn_list tbsaunde+gcc
@ 2016-06-20 10:15 ` tbsaunde+gcc
  2016-06-20 10:16 ` [PATCH 4/6] make side_effects a vec<rtx> tbsaunde+gcc
                   ` (4 subsequent siblings)
  6 siblings, 0 replies; 15+ messages in thread
From: tbsaunde+gcc @ 2016-06-20 10:15 UTC (permalink / raw)
  To: gcc-patches

From: Trevor Saunders <tbsaunde+gcc@tbsaunde.org>

gcc/ChangeLog:

2016-06-20  Trevor Saunders  <tbsaunde+gcc@tbsaunde.org>

	* gcse.c (struct ls_expr): Make stores field a vector.
	(ldst_entry): Adjust.
	(free_ldst_entry): Likewise.
	(print_ldst_list): Likewise.
	(compute_ld_motion_mems): Likewise.
	(update_ld_motion_stores): Likewise.
---
 gcc/gcse.c | 22 +++++++++-------------
 1 file changed, 9 insertions(+), 13 deletions(-)

diff --git a/gcc/gcse.c b/gcc/gcse.c
index 127a65a..49534f2 100644
--- a/gcc/gcse.c
+++ b/gcc/gcse.c
@@ -143,6 +143,7 @@ along with GCC; see the file COPYING3.  If not see
 #include "df.h"
 #include "tm_p.h"
 #include "insn-config.h"
+#include "print-rtl.h"
 #include "regs.h"
 #include "ira.h"
 #include "recog.h"
@@ -342,7 +343,7 @@ struct ls_expr
   struct gcse_expr * expr;	/* Gcse expression reference for LM.  */
   rtx pattern;			/* Pattern of this mem.  */
   rtx pattern_regs;		/* List of registers mentioned by the mem.  */
-  rtx_insn_list *stores;	/* INSN list of stores seen.  */
+  vec<rtx_insn *> stores;	/* INSN list of stores seen.  */
   struct ls_expr * next;	/* Next in the list.  */
   int invalid;			/* Invalid for some reason.  */
   int index;			/* If it maps to a bitmap index.  */
@@ -3604,7 +3605,7 @@ ldst_entry (rtx x)
   ptr->expr         = NULL;
   ptr->pattern      = x;
   ptr->pattern_regs = NULL_RTX;
-  ptr->stores       = NULL;
+  ptr->stores.create (0);
   ptr->reaching_reg = NULL_RTX;
   ptr->invalid      = 0;
   ptr->index        = 0;
@@ -3620,7 +3621,7 @@ ldst_entry (rtx x)
 static void
 free_ldst_entry (struct ls_expr * ptr)
 {
-  free_INSN_LIST_list (& ptr->stores);
+  ptr->stores.release ();
 
   free (ptr);
 }
@@ -3661,11 +3662,7 @@ print_ldst_list (FILE * file)
       print_rtl (file, ptr->pattern);
 
       fprintf (file, "\n	Stores : ");
-
-      if (ptr->stores)
-	print_rtl (file, ptr->stores);
-      else
-	fprintf (file, "(nil)");
+      print_rtx_insn_vec (file, ptr->stores);
 
       fprintf (file, "\n\n");
     }
@@ -3822,7 +3819,7 @@ compute_ld_motion_mems (void)
 			     returns 0 for all REGs.  */
 			  && can_assign_to_reg_without_clobbers_p (src,
 								    src_mode))
-			ptr->stores = alloc_INSN_LIST (insn, ptr->stores);
+			ptr->stores.safe_push (insn);
 		      else
 			ptr->invalid = 1;
 		    }
@@ -3915,11 +3912,10 @@ update_ld_motion_stores (struct gcse_expr * expr)
 	 where reg is the reaching reg used in the load.  We checked in
 	 compute_ld_motion_mems that we can replace (set mem expr) with
 	 (set reg expr) in that insn.  */
-      rtx list = mem_ptr->stores;
-
-      for ( ; list != NULL_RTX; list = XEXP (list, 1))
+      rtx_insn *insn;
+      unsigned int i;
+      FOR_EACH_VEC_ELT_REVERSE (mem_ptr->stores, i, insn)
 	{
-	  rtx_insn *insn = as_a <rtx_insn *> (XEXP (list, 0));
 	  rtx pat = PATTERN (insn);
 	  rtx src = SET_SRC (pat);
 	  rtx reg = expr->reaching_reg;
-- 
2.7.4

^ permalink raw reply	[flat|nested] 15+ messages in thread

* [PATCH 2/6] remove unused loads rtx_insn_list
  2016-06-20 10:15 [PATCH 0/6] remove some usage of rtx_{insn,expr}_list tbsaunde+gcc
@ 2016-06-20 10:15 ` tbsaunde+gcc
  2016-06-20 10:15 ` [PATCH 3/6] make stores rtx_insn_list a vec tbsaunde+gcc
                   ` (5 subsequent siblings)
  6 siblings, 0 replies; 15+ messages in thread
From: tbsaunde+gcc @ 2016-06-20 10:15 UTC (permalink / raw)
  To: gcc-patches

From: Trevor Saunders <tbsaunde+gcc@tbsaunde.org>

gcc/ChangeLog:

2016-06-20  Trevor Saunders  <tbsaunde+gcc@tbsaunde.org>

	* gcse.c (struct ls_expr): Remove loads field.
	(ldst_entry): Adjust.
	(free_ldst_entry): Likewise.
	(print_ldst_list): Likewise.
	(compute_ld_motion_mems): Likewise.
---
 gcc/gcse.c | 14 +-------------
 1 file changed, 1 insertion(+), 13 deletions(-)

diff --git a/gcc/gcse.c b/gcc/gcse.c
index a3a7dc3..127a65a 100644
--- a/gcc/gcse.c
+++ b/gcc/gcse.c
@@ -342,7 +342,6 @@ struct ls_expr
   struct gcse_expr * expr;	/* Gcse expression reference for LM.  */
   rtx pattern;			/* Pattern of this mem.  */
   rtx pattern_regs;		/* List of registers mentioned by the mem.  */
-  rtx_insn_list *loads;		/* INSN list of loads seen.  */
   rtx_insn_list *stores;	/* INSN list of stores seen.  */
   struct ls_expr * next;	/* Next in the list.  */
   int invalid;			/* Invalid for some reason.  */
@@ -3605,7 +3604,6 @@ ldst_entry (rtx x)
   ptr->expr         = NULL;
   ptr->pattern      = x;
   ptr->pattern_regs = NULL_RTX;
-  ptr->loads        = NULL;
   ptr->stores       = NULL;
   ptr->reaching_reg = NULL_RTX;
   ptr->invalid      = 0;
@@ -3622,7 +3620,6 @@ ldst_entry (rtx x)
 static void
 free_ldst_entry (struct ls_expr * ptr)
 {
-  free_INSN_LIST_list (& ptr->loads);
   free_INSN_LIST_list (& ptr->stores);
 
   free (ptr);
@@ -3663,13 +3660,6 @@ print_ldst_list (FILE * file)
 
       print_rtl (file, ptr->pattern);
 
-      fprintf (file, "\n	 Loads : ");
-
-      if (ptr->loads)
-	print_rtl (file, ptr->loads);
-      else
-	fprintf (file, "(nil)");
-
       fprintf (file, "\n	Stores : ");
 
       if (ptr->stores)
@@ -3801,9 +3791,7 @@ compute_ld_motion_mems (void)
 		  if (MEM_P (src) && simple_mem (src))
 		    {
 		      ptr = ldst_entry (src);
-		      if (REG_P (dest))
-			ptr->loads = alloc_INSN_LIST (insn, ptr->loads);
-		      else
+		      if (!REG_P (dest))
 			ptr->invalid = 1;
 		    }
 		  else
-- 
2.7.4

^ permalink raw reply	[flat|nested] 15+ messages in thread

* [PATCH 5/6] make pattern_regs a vec
  2016-06-20 10:15 [PATCH 0/6] remove some usage of rtx_{insn,expr}_list tbsaunde+gcc
                   ` (4 preceding siblings ...)
  2016-06-20 10:16 ` [PATCH 6/6] loop-iv.c: make cond_list a vec tbsaunde+gcc
@ 2016-06-20 10:16 ` tbsaunde+gcc
  2016-06-20 20:04   ` Richard Sandiford
  2016-06-20 16:52 ` [PATCH 0/6] remove some usage of rtx_{insn,expr}_list Bernd Schmidt
  6 siblings, 1 reply; 15+ messages in thread
From: tbsaunde+gcc @ 2016-06-20 10:16 UTC (permalink / raw)
  To: gcc-patches

From: Trevor Saunders <tbsaunde+gcc@tbsaunde.org>

gcc/ChangeLog:

2016-06-20  Trevor Saunders  <tbsaunde+gcc@tbsaunde.org>

	* store-motion.c (struct st_expr): Make pattern_regs a vector.
	(st_expr_entry): Adjust.
	(store_ops_ok): Likewise.
	(extract_mentioned_regs): Likewise.
	(store_killed_in_insn): Likewise.
	(find_moveable_store): Likewise.
---
 gcc/store-motion.c | 49 +++++++++++++++++++++++--------------------------
 1 file changed, 23 insertions(+), 26 deletions(-)

diff --git a/gcc/store-motion.c b/gcc/store-motion.c
index 6d7d37f..c2ef0d0 100644
--- a/gcc/store-motion.c
+++ b/gcc/store-motion.c
@@ -63,7 +63,7 @@ struct st_expr
   /* Pattern of this mem.  */
   rtx pattern;
   /* List of registers mentioned by the mem.  */
-  rtx pattern_regs;
+  vec<rtx> pattern_regs;
   /* INSN list of stores that are locally anticipatable.  */
   vec<rtx_insn *> antic_stores;
   /* INSN list of stores that are locally available.  */
@@ -146,7 +146,7 @@ st_expr_entry (rtx x)
 
   ptr->next         = store_motion_mems;
   ptr->pattern      = x;
-  ptr->pattern_regs = NULL_RTX;
+  ptr->pattern_regs.create (0);
   ptr->antic_stores.create (0);
   ptr->avail_stores.create (0);
   ptr->reaching_reg = NULL_RTX;
@@ -248,16 +248,12 @@ print_store_motion_mems (FILE * file)
    due to set of registers in bitmap REGS_SET.  */
 
 static bool
-store_ops_ok (const_rtx x, int *regs_set)
+store_ops_ok (const vec<rtx> &x, int *regs_set)
 {
-  const_rtx reg;
-
-  for (; x; x = XEXP (x, 1))
-    {
-      reg = XEXP (x, 0);
-      if (regs_set[REGNO (reg)])
-	return false;
-    }
+  unsigned int len = x.length ();
+  for (unsigned int i = 0; i < len; i++)
+    if (regs_set[REGNO (x[i])])
+      return false;
 
   return true;
 }
@@ -265,18 +261,16 @@ store_ops_ok (const_rtx x, int *regs_set)
 /* Returns a list of registers mentioned in X.
    FIXME: A regset would be prettier and less expensive.  */
 
-static rtx_expr_list *
-extract_mentioned_regs (rtx x)
+static void
+extract_mentioned_regs (rtx x, vec<rtx> *mentioned_regs)
 {
-  rtx_expr_list *mentioned_regs = NULL;
   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);
+	mentioned_regs->safe_push (x);
     }
-  return mentioned_regs;
 }
 
 /* Check to see if the load X is aliased with STORE_PATTERN.
@@ -373,9 +367,10 @@ store_killed_in_pat (const_rtx x, const_rtx pat, int after)
    after the insn.  Return true if it does.  */
 
 static bool
-store_killed_in_insn (const_rtx x, const_rtx x_regs, const rtx_insn *insn, int after)
+store_killed_in_insn (const_rtx x, const vec<rtx> &x_regs,
+		      const rtx_insn *insn, int after)
 {
-  const_rtx reg, note, pat;
+  const_rtx note, pat;
 
   if (! NONDEBUG_INSN_P (insn))
     return false;
@@ -389,8 +384,9 @@ store_killed_in_insn (const_rtx x, const_rtx x_regs, const rtx_insn *insn, int a
 
       /* But even a const call reads its parameters.  Check whether the
 	 base of some of registers used in mem is stack pointer.  */
-      for (reg = x_regs; reg; reg = XEXP (reg, 1))
-	if (may_be_sp_based_p (XEXP (reg, 0)))
+      unsigned int len = x_regs.length ();
+      for (unsigned int i = 0; i < len; i++)
+	if (may_be_sp_based_p (x_regs[i]))
 	  return true;
 
       return false;
@@ -435,8 +431,8 @@ store_killed_in_insn (const_rtx x, const_rtx x_regs, const rtx_insn *insn, int a
    is killed, return the last insn in that it occurs in FAIL_INSN.  */
 
 static bool
-store_killed_after (const_rtx x, const_rtx x_regs, const rtx_insn *insn,
-		    const_basic_block bb,
+store_killed_after (const_rtx x, const vec<rtx> &x_regs,
+		    const rtx_insn *insn, const_basic_block bb,
 		    int *regs_set_after, rtx *fail_insn)
 {
   rtx_insn *last = BB_END (bb), *act;
@@ -465,8 +461,9 @@ store_killed_after (const_rtx x, const_rtx x_regs, const rtx_insn *insn,
    within basic block BB. X_REGS is list of registers mentioned in X.
    REGS_SET_BEFORE is bitmap of registers set before or in this insn.  */
 static bool
-store_killed_before (const_rtx x, const_rtx x_regs, const rtx_insn *insn,
-		     const_basic_block bb, int *regs_set_before)
+store_killed_before (const_rtx x, const vec<rtx> &x_regs,
+		     const rtx_insn *insn, const_basic_block bb,
+		     int *regs_set_before)
 {
   rtx_insn *first = BB_HEAD (bb);
 
@@ -555,8 +552,8 @@ find_moveable_store (rtx_insn *insn, int *regs_set_before, int *regs_set_after)
     return;
 
   ptr = st_expr_entry (dest);
-  if (!ptr->pattern_regs)
-    ptr->pattern_regs = extract_mentioned_regs (dest);
+  if (ptr->pattern_regs.is_empty ())
+    extract_mentioned_regs (dest, &ptr->pattern_regs);
 
   /* Do not check for anticipatability if we either found one anticipatable
      store already, or tested for one and found out that it was killed.  */
-- 
2.7.4

^ permalink raw reply	[flat|nested] 15+ messages in thread

* [PATCH 1/6] make antic_stores a vec<rtx_insn *>
  2016-06-20 10:15 [PATCH 0/6] remove some usage of rtx_{insn,expr}_list tbsaunde+gcc
                   ` (2 preceding siblings ...)
  2016-06-20 10:16 ` [PATCH 4/6] make side_effects a vec<rtx> tbsaunde+gcc
@ 2016-06-20 10:16 ` tbsaunde+gcc
  2016-06-20 10:16 ` [PATCH 6/6] loop-iv.c: make cond_list a vec tbsaunde+gcc
                   ` (2 subsequent siblings)
  6 siblings, 0 replies; 15+ messages in thread
From: tbsaunde+gcc @ 2016-06-20 10:16 UTC (permalink / raw)
  To: gcc-patches

From: Trevor Saunders <tbsaunde+gcc@tbsaunde.org>

gcc/ChangeLog:

2016-06-20  Trevor Saunders  <tbsaunde+gcc@tbsaunde.org>

	* store-motion.c (struct st_expr): Make antic_stores a vector.
	(st_expr_entry): Adjust.
	(free_st_expr_entry): Likewise.
	(print_store_motion_mems): Likewise.
	(find_moveable_store): Likewise.
	(compute_store_table): Likewise.
	(remove_reachable_equiv_notes): Likewise.
	(replace_store_insn): Likewise.
	(build_store_vectors): Likewise.
---
 gcc/store-motion.c | 54 +++++++++++++++++++++++++-----------------------------
 1 file changed, 25 insertions(+), 29 deletions(-)

diff --git a/gcc/store-motion.c b/gcc/store-motion.c
index 301b69b..6d7d37f 100644
--- a/gcc/store-motion.c
+++ b/gcc/store-motion.c
@@ -46,7 +46,6 @@ along with GCC; see the file COPYING3.  If not see
      a compile time hog that needs a rewrite (maybe cache st_exprs to
      invalidate REG_EQUAL/REG_EQUIV notes for?).
    - pattern_regs in st_expr should be a regset (on its own obstack).
-   - antic_stores and avail_stores should be VECs instead of lists.
    - store_motion_mems should be a vec instead of a list.
    - there should be an alloc pool for struct st_expr objects.
    - investigate whether it is helpful to make the address of an st_expr
@@ -66,7 +65,7 @@ struct st_expr
   /* List of registers mentioned by the mem.  */
   rtx pattern_regs;
   /* INSN list of stores that are locally anticipatable.  */
-  rtx_insn_list *antic_stores;
+  vec<rtx_insn *> antic_stores;
   /* INSN list of stores that are locally available.  */
   vec<rtx_insn *> avail_stores;
   /* Next in the list.  */
@@ -148,7 +147,7 @@ st_expr_entry (rtx x)
   ptr->next         = store_motion_mems;
   ptr->pattern      = x;
   ptr->pattern_regs = NULL_RTX;
-  ptr->antic_stores = NULL;
+  ptr->antic_stores.create (0);
   ptr->avail_stores.create (0);
   ptr->reaching_reg = NULL_RTX;
   ptr->index        = 0;
@@ -164,8 +163,8 @@ st_expr_entry (rtx x)
 static void
 free_st_expr_entry (struct st_expr * ptr)
 {
-  free_INSN_LIST_list (& ptr->antic_stores);
-   ptr->avail_stores.release ();
+  ptr->antic_stores.release ();
+  ptr->avail_stores.release ();
 
   free (ptr);
 }
@@ -233,11 +232,7 @@ print_store_motion_mems (FILE * file)
       print_rtl (file, ptr->pattern);
 
       fprintf (file, "\n	 ANTIC stores : ");
-
-      if (ptr->antic_stores)
-	print_rtl (file, ptr->antic_stores);
-      else
-	fprintf (file, "(nil)");
+      print_rtx_insn_vec (file, ptr->antic_stores);
 
       fprintf (file, "\n	 AVAIL stores : ");
 
@@ -566,11 +561,11 @@ find_moveable_store (rtx_insn *insn, int *regs_set_before, int *regs_set_after)
   /* Do not check for anticipatability if we either found one anticipatable
      store already, or tested for one and found out that it was killed.  */
   check_anticipatable = 0;
-  if (!ptr->antic_stores)
+  if (ptr->antic_stores.is_empty ())
     check_anticipatable = 1;
   else
     {
-      rtx_insn *tmp = ptr->antic_stores->insn ();
+      rtx_insn *tmp = ptr->antic_stores.last ();
       if (tmp != NULL_RTX
 	  && BLOCK_FOR_INSN (tmp) != bb)
 	check_anticipatable = 1;
@@ -582,7 +577,7 @@ find_moveable_store (rtx_insn *insn, int *regs_set_before, int *regs_set_after)
 	tmp = NULL;
       else
 	tmp = insn;
-      ptr->antic_stores = alloc_INSN_LIST (tmp, ptr->antic_stores);
+      ptr->antic_stores.safe_push (tmp);
     }
 
   /* It is not necessary to check whether store is available if we did
@@ -683,9 +678,9 @@ compute_store_table (void)
       for (ptr = first_st_expr (); ptr != NULL; ptr = next_st_expr (ptr))
 	{
 	  LAST_AVAIL_CHECK_FAILURE (ptr) = NULL_RTX;
-	  if (ptr->antic_stores
-	      && (tmp = ptr->antic_stores->insn ()) == NULL_RTX)
-	    ptr->antic_stores = ptr->antic_stores->next ();
+	  if (!ptr->antic_stores.is_empty ()
+	      && (tmp = ptr->antic_stores.last ()) == NULL)
+	    ptr->antic_stores.pop ();
 	}
     }
 
@@ -831,7 +826,7 @@ remove_reachable_equiv_notes (basic_block bb, struct st_expr *smexpr)
   int sp;
   edge act;
   sbitmap visited = sbitmap_alloc (last_basic_block_for_fn (cfun));
-  rtx last, note;
+  rtx note;
   rtx_insn *insn;
   rtx mem = smexpr->pattern;
 
@@ -866,13 +861,13 @@ remove_reachable_equiv_notes (basic_block bb, struct st_expr *smexpr)
 	}
       bitmap_set_bit (visited, bb->index);
 
+      rtx_insn *last;
       if (bitmap_bit_p (st_antloc[bb->index], smexpr->index))
 	{
-	  for (last = smexpr->antic_stores;
-	       BLOCK_FOR_INSN (XEXP (last, 0)) != bb;
-	       last = XEXP (last, 1))
-	    continue;
-	  last = XEXP (last, 0);
+	  unsigned int i;
+	  FOR_EACH_VEC_ELT_REVERSE (smexpr->antic_stores, i, last)
+	    if (BLOCK_FOR_INSN (last) == bb)
+	      break;
 	}
       else
 	last = NEXT_INSN (BB_END (bb));
@@ -911,15 +906,17 @@ replace_store_insn (rtx reg, rtx_insn *del, basic_block bb,
 		    struct st_expr *smexpr)
 {
   rtx_insn *insn;
-  rtx mem, note, set, ptr;
+  rtx mem, note, set;
 
   mem = smexpr->pattern;
   insn = gen_move_insn (reg, SET_SRC (single_set (del)));
 
-  for (ptr = smexpr->antic_stores; ptr; ptr = XEXP (ptr, 1))
-    if (XEXP (ptr, 0) == del)
+  unsigned int i;
+  rtx_insn *temp;
+  FOR_EACH_VEC_ELT_REVERSE (smexpr->antic_stores, i, temp)
+    if (temp == del)
       {
-	XEXP (ptr, 0) = insn;
+	smexpr->antic_stores[i] = insn;
 	break;
       }
 
@@ -1001,7 +998,6 @@ build_store_vectors (void)
   basic_block bb;
   int *regs_set_in_block;
   rtx_insn *insn;
-  rtx_insn_list *st;
   struct st_expr * ptr;
   unsigned int max_gcse_regno = max_reg_num ();
 
@@ -1038,9 +1034,9 @@ build_store_vectors (void)
 	  bitmap_set_bit (st_avloc[bb->index], ptr->index);
 	}
 
-      for (st = ptr->antic_stores; st != NULL; st = st->next ())
+      unsigned int i;
+      FOR_EACH_VEC_ELT_REVERSE (ptr->antic_stores, i, insn)
 	{
-	  insn = st->insn ();
 	  bb = BLOCK_FOR_INSN (insn);
 	  bitmap_set_bit (st_antloc[bb->index], ptr->index);
 	}
-- 
2.7.4

^ permalink raw reply	[flat|nested] 15+ messages in thread

* [PATCH 4/6] make side_effects a vec<rtx>
  2016-06-20 10:15 [PATCH 0/6] remove some usage of rtx_{insn,expr}_list tbsaunde+gcc
  2016-06-20 10:15 ` [PATCH 2/6] remove unused loads rtx_insn_list tbsaunde+gcc
  2016-06-20 10:15 ` [PATCH 3/6] make stores rtx_insn_list a vec tbsaunde+gcc
@ 2016-06-20 10:16 ` tbsaunde+gcc
  2016-06-20 10:16 ` [PATCH 1/6] make antic_stores a vec<rtx_insn *> tbsaunde+gcc
                   ` (3 subsequent siblings)
  6 siblings, 0 replies; 15+ messages in thread
From: tbsaunde+gcc @ 2016-06-20 10:16 UTC (permalink / raw)
  To: gcc-patches

From: Trevor Saunders <tbsaunde+gcc@tbsaunde.org>

gcc/ChangeLog:

2016-06-20  Trevor Saunders  <tbsaunde+gcc@tbsaunde.org>

	* var-tracking.c (struct adjust_mem_data): Make side_effects a vector.
	(adjust_mems): Adjust.
	(adjust_insn): Likewise.
	(prepare_call_arguments): Likewise.
---
 gcc/var-tracking.c | 33 ++++++++++++++-------------------
 1 file changed, 14 insertions(+), 19 deletions(-)

diff --git a/gcc/var-tracking.c b/gcc/var-tracking.c
index 9f09d30..5d09879 100644
--- a/gcc/var-tracking.c
+++ b/gcc/var-tracking.c
@@ -926,7 +926,7 @@ struct adjust_mem_data
   bool store;
   machine_mode mem_mode;
   HOST_WIDE_INT stack_adjust;
-  rtx_expr_list *side_effects;
+  auto_vec<rtx> side_effects;
 };
 
 /* Helper for adjust_mems.  Return true if X is suitable for
@@ -1072,9 +1072,7 @@ adjust_mems (rtx loc, const_rtx old_rtx, void *data)
       amd->store = false;
       tem = simplify_replace_fn_rtx (tem, old_rtx, adjust_mems, data);
       amd->store = store_save;
-      amd->side_effects = alloc_EXPR_LIST (0,
-					   gen_rtx_SET (XEXP (loc, 0), tem),
-					   amd->side_effects);
+      amd->side_effects.safe_push (gen_rtx_SET (XEXP (loc, 0), tem));
       return addr;
     case PRE_MODIFY:
       addr = XEXP (loc, 1);
@@ -1088,9 +1086,7 @@ adjust_mems (rtx loc, const_rtx old_rtx, void *data)
       tem = simplify_replace_fn_rtx (XEXP (loc, 1), old_rtx,
 				     adjust_mems, data);
       amd->store = store_save;
-      amd->side_effects = alloc_EXPR_LIST (0,
-					   gen_rtx_SET (XEXP (loc, 0), tem),
-					   amd->side_effects);
+      amd->side_effects.safe_push (gen_rtx_SET (XEXP (loc, 0), tem));
       return addr;
     case SUBREG:
       /* First try without delegitimization of whole MEMs and
@@ -1184,7 +1180,6 @@ adjust_mem_stores (rtx loc, const_rtx expr, void *data)
 static void
 adjust_insn (basic_block bb, rtx_insn *insn)
 {
-  struct adjust_mem_data amd;
   rtx set;
 
 #ifdef HAVE_window_save
@@ -1213,9 +1208,9 @@ adjust_insn (basic_block bb, rtx_insn *insn)
     }
 #endif
 
+  adjust_mem_data amd;
   amd.mem_mode = VOIDmode;
   amd.stack_adjust = -VTI (bb)->out.stack_adjust;
-  amd.side_effects = NULL;
 
   amd.store = true;
   note_stores (PATTERN (insn), adjust_mem_stores, &amd);
@@ -1281,10 +1276,10 @@ adjust_insn (basic_block bb, rtx_insn *insn)
 	validate_change (NULL_RTX, &SET_SRC (set), XEXP (note, 0), true);
     }
 
-  if (amd.side_effects)
+  if (!amd.side_effects.is_empty ())
     {
-      rtx *pat, new_pat, s;
-      int i, oldn, newn;
+      rtx *pat, new_pat;
+      int i, oldn;
 
       pat = &PATTERN (insn);
       if (GET_CODE (*pat) == COND_EXEC)
@@ -1293,17 +1288,18 @@ adjust_insn (basic_block bb, rtx_insn *insn)
 	oldn = XVECLEN (*pat, 0);
       else
 	oldn = 1;
-      for (s = amd.side_effects, newn = 0; s; newn++)
-	s = XEXP (s, 1);
+      unsigned int newn = amd.side_effects.length ();
       new_pat = gen_rtx_PARALLEL (VOIDmode, rtvec_alloc (oldn + newn));
       if (GET_CODE (*pat) == PARALLEL)
 	for (i = 0; i < oldn; i++)
 	  XVECEXP (new_pat, 0, i) = XVECEXP (*pat, 0, i);
       else
 	XVECEXP (new_pat, 0, 0) = *pat;
-      for (s = amd.side_effects, i = oldn; i < oldn + newn; i++, s = XEXP (s, 1))
-	XVECEXP (new_pat, 0, i) = XEXP (s, 0);
-      free_EXPR_LIST_list (&amd.side_effects);
+
+      rtx effect;
+      unsigned int j;
+      FOR_EACH_VEC_ELT_REVERSE (amd.side_effects, j, effect)
+	XVECEXP (new_pat, 0, j + oldn) = effect;
       validate_change (NULL_RTX, pat, new_pat, true);
     }
 }
@@ -6335,11 +6331,10 @@ prepare_call_arguments (basic_block bb, rtx_insn *insn)
 		struct adjust_mem_data amd;
 		amd.mem_mode = VOIDmode;
 		amd.stack_adjust = -VTI (bb)->out.stack_adjust;
-		amd.side_effects = NULL;
 		amd.store = true;
 		mem = simplify_replace_fn_rtx (mem, NULL_RTX, adjust_mems,
 					       &amd);
-		gcc_assert (amd.side_effects == NULL_RTX);
+		gcc_assert (amd.side_effects.is_empty ());
 	      }
 	    val = cselib_lookup (mem, GET_MODE (mem), 0, VOIDmode);
 	    if (val && cselib_preserved_value_p (val))
-- 
2.7.4

^ permalink raw reply	[flat|nested] 15+ messages in thread

* [PATCH 6/6] loop-iv.c: make cond_list a vec
  2016-06-20 10:15 [PATCH 0/6] remove some usage of rtx_{insn,expr}_list tbsaunde+gcc
                   ` (3 preceding siblings ...)
  2016-06-20 10:16 ` [PATCH 1/6] make antic_stores a vec<rtx_insn *> tbsaunde+gcc
@ 2016-06-20 10:16 ` tbsaunde+gcc
  2016-06-20 20:11   ` Richard Sandiford
  2016-06-20 10:16 ` [PATCH 5/6] make pattern_regs " tbsaunde+gcc
  2016-06-20 16:52 ` [PATCH 0/6] remove some usage of rtx_{insn,expr}_list Bernd Schmidt
  6 siblings, 1 reply; 15+ messages in thread
From: tbsaunde+gcc @ 2016-06-20 10:16 UTC (permalink / raw)
  To: gcc-patches

From: Trevor Saunders <tbsaunde+gcc@tbsaunde.org>

gcc/ChangeLog:

2016-06-20  Trevor Saunders  <tbsaunde+gcc@tbsaunde.org>

	* loop-iv.c (simplify_using_initial_values): Make cond_list a vector.
---
 gcc/loop-iv.c | 55 ++++++++++++++++++-------------------------------------
 1 file changed, 18 insertions(+), 37 deletions(-)

diff --git a/gcc/loop-iv.c b/gcc/loop-iv.c
index 57fb8c1..21c3180 100644
--- a/gcc/loop-iv.c
+++ b/gcc/loop-iv.c
@@ -1860,7 +1860,6 @@ simplify_using_initial_values (struct loop *loop, enum rtx_code op, rtx *expr)
 {
   bool expression_valid;
   rtx head, tail, last_valid_expr;
-  rtx_expr_list *cond_list;
   rtx_insn *insn;
   rtx neutral, aggr;
   regset altered, this_altered;
@@ -1936,7 +1935,7 @@ simplify_using_initial_values (struct loop *loop, enum rtx_code op, rtx *expr)
 
   expression_valid = true;
   last_valid_expr = *expr;
-  cond_list = NULL;
+  auto_vec<rtx> cond_list;
   while (1)
     {
       insn = BB_END (e->src);
@@ -1952,17 +1951,18 @@ simplify_using_initial_values (struct loop *loop, enum rtx_code op, rtx *expr)
 	      simplify_using_condition (cond, expr, altered);
 	      if (old != *expr)
 		{
-		  rtx note;
 		  if (CONSTANT_P (*expr))
 		    goto out;
-		  for (note = cond_list; note; note = XEXP (note, 1))
+
+		  unsigned int len = cond_list.length ();
+		  for (unsigned int i = len - 1; i < len; i--)
 		    {
-		      simplify_using_condition (XEXP (note, 0), expr, altered);
+		      simplify_using_condition (cond_list[i], expr, altered);
 		      if (CONSTANT_P (*expr))
 			goto out;
 		    }
 		}
-	      cond_list = alloc_EXPR_LIST (0, cond, cond_list);
+	      cond_list.safe_push (cond);
 	    }
 	}
 
@@ -1988,39 +1988,30 @@ simplify_using_initial_values (struct loop *loop, enum rtx_code op, rtx *expr)
 
 	  if (suitable_set_for_replacement (insn, &dest, &src))
 	    {
-	      rtx_expr_list **pnote, **pnote_next;
-
 	      replace_in_expr (expr, dest, src);
 	      if (CONSTANT_P (*expr))
 		goto out;
 
-	      for (pnote = &cond_list; *pnote; pnote = pnote_next)
+	      unsigned int len = cond_list.length ();
+	      for (unsigned int i = len - 1; i < len; i--)
 		{
-		  rtx_expr_list *note = *pnote;
-		  rtx old_cond = XEXP (note, 0);
+		  rtx old_cond = cond_list[i];
 
-		  pnote_next = (rtx_expr_list **)&XEXP (note, 1);
-		  replace_in_expr (&XEXP (note, 0), dest, src);
+		  replace_in_expr (&cond_list[i], dest, src);
 
 		  /* We can no longer use a condition that has been simplified
 		     to a constant, and simplify_using_condition will abort if
 		     we try.  */
-		  if (CONSTANT_P (XEXP (note, 0)))
-		    {
-		      *pnote = *pnote_next;
-		      pnote_next = pnote;
-		      free_EXPR_LIST_node (note);
-		    }
+		  if (CONSTANT_P (cond_list[i]))
+		    cond_list.ordered_remove (i);
 		  /* Retry simplifications with this condition if either the
 		     expression or the condition changed.  */
-		  else if (old_cond != XEXP (note, 0) || old != *expr)
-		    simplify_using_condition (XEXP (note, 0), expr, altered);
+		  else if (old_cond != cond_list[i] || old != *expr)
+		    simplify_using_condition (cond_list[i], expr, altered);
 		}
 	    }
 	  else
 	    {
-	      rtx_expr_list **pnote, **pnote_next;
-
 	      /* 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.  */
@@ -2028,19 +2019,10 @@ simplify_using_initial_values (struct loop *loop, enum rtx_code op, rtx *expr)
 		goto out;
 
 	      /* Likewise for the conditions.  */
-	      for (pnote = &cond_list; *pnote; pnote = pnote_next)
-		{
-		  rtx_expr_list *note = *pnote;
-		  rtx old_cond = XEXP (note, 0);
-
-		  pnote_next = (rtx_expr_list **)&XEXP (note, 1);
-		  if (altered_reg_used (old_cond, this_altered))
-		    {
-		      *pnote = *pnote_next;
-		      pnote_next = pnote;
-		      free_EXPR_LIST_node (note);
-		    }
-		}
+	      unsigned int len = cond_list.length ();
+	      for (unsigned int i = len - 1; i < len; i--)
+		if (altered_reg_used (cond_list[i], this_altered))
+		  cond_list.ordered_remove (i);
 	    }
 
 	  if (CONSTANT_P (*expr))
@@ -2065,7 +2047,6 @@ simplify_using_initial_values (struct loop *loop, enum rtx_code op, rtx *expr)
     }
 
  out:
-  free_EXPR_LIST_list (&cond_list);
   if (!CONSTANT_P (*expr))
     *expr = last_valid_expr;
   FREE_REG_SET (altered);
-- 
2.7.4

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCH 0/6] remove some usage of rtx_{insn,expr}_list
  2016-06-20 10:15 [PATCH 0/6] remove some usage of rtx_{insn,expr}_list tbsaunde+gcc
                   ` (5 preceding siblings ...)
  2016-06-20 10:16 ` [PATCH 5/6] make pattern_regs " tbsaunde+gcc
@ 2016-06-20 16:52 ` Bernd Schmidt
  2016-06-21 14:43   ` Trevor Saunders
  6 siblings, 1 reply; 15+ messages in thread
From: Bernd Schmidt @ 2016-06-20 16:52 UTC (permalink / raw)
  To: tbsaunde+gcc, gcc-patches

On 06/20/2016 12:22 PM, tbsaunde+gcc@tbsaunde.org wrote:
> In theory I would expect if anything this helps performance since it isn't
> necessary to malloc every time a node is added, however the data is less clear.

Well, we have alloc pools for these lists, so a malloc is not needed for 
every node.

> fold const O2 new
> real    0m5.034s
> user    0m3.408s
> sys     0m0.364s
>
> fold const O2 old
> real    0m4.012s
> user    0m3.420s
> sys     0m0.340s

So that's a second more in real time - was the machine very busy at the 
time you ran these tests so that these aren't meaningful, or is there a 
need to investigate this?

> So a couple got about .3s slower, and others got about .1 faster, I'm not
> really sure but inclined to say any change is too small to easily measure.
>
> bootstrapped + regtested patches individually on x86_64-linux-gnu, ok?

Modulo the question about compile times I think patches 1-4 are ok, In 5 
and 6 I see explicit for loops instead of FOR_EACH macros; I'm curious 
as to the reason.


Bernd

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCH 5/6] make pattern_regs a vec
  2016-06-20 10:16 ` [PATCH 5/6] make pattern_regs " tbsaunde+gcc
@ 2016-06-20 20:04   ` Richard Sandiford
  0 siblings, 0 replies; 15+ messages in thread
From: Richard Sandiford @ 2016-06-20 20:04 UTC (permalink / raw)
  To: tbsaunde+gcc; +Cc: gcc-patches

tbsaunde+gcc@tbsaunde.org writes:
> @@ -265,18 +261,16 @@ store_ops_ok (const_rtx x, int *regs_set)
>  /* Returns a list of registers mentioned in X.
>     FIXME: A regset would be prettier and less expensive.  */
>
> -static rtx_expr_list *
> -extract_mentioned_regs (rtx x)
> +static void
> +extract_mentioned_regs (rtx x, vec<rtx> *mentioned_regs)
>  {
> -  rtx_expr_list *mentioned_regs = NULL;
>    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);
> +	mentioned_regs->safe_push (x);
>      }
> -  return mentioned_regs;
>  }

The function comment needs to be updated.

Thanks,
Richard

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCH 6/6] loop-iv.c: make cond_list a vec
  2016-06-20 10:16 ` [PATCH 6/6] loop-iv.c: make cond_list a vec tbsaunde+gcc
@ 2016-06-20 20:11   ` Richard Sandiford
  2016-06-21 14:46     ` Trevor Saunders
  0 siblings, 1 reply; 15+ messages in thread
From: Richard Sandiford @ 2016-06-20 20:11 UTC (permalink / raw)
  To: tbsaunde+gcc; +Cc: gcc-patches

tbsaunde+gcc@tbsaunde.org writes:
> diff --git a/gcc/loop-iv.c b/gcc/loop-iv.c
> index 57fb8c1..21c3180 100644
> --- a/gcc/loop-iv.c
> +++ b/gcc/loop-iv.c
> @@ -1860,7 +1860,6 @@ simplify_using_initial_values (struct loop *loop, enum rtx_code op, rtx *expr)
>  {
>    bool expression_valid;
>    rtx head, tail, last_valid_expr;
> -  rtx_expr_list *cond_list;
>    rtx_insn *insn;
>    rtx neutral, aggr;
>    regset altered, this_altered;
> @@ -1936,7 +1935,7 @@ simplify_using_initial_values (struct loop *loop, enum rtx_code op, rtx *expr)
>  
>    expression_valid = true;
>    last_valid_expr = *expr;
> -  cond_list = NULL;
> +  auto_vec<rtx> cond_list;
>    while (1)
>      {
>        insn = BB_END (e->src);

How about using "auto_vec<rtx, N>" for some small N, since we expect
cond_list to be used fairly often?

Wish I knew whether there was supposed to be a space before "<"...

> @@ -1988,39 +1988,30 @@ simplify_using_initial_values (struct loop *loop, enum rtx_code op, rtx *expr)
>  
>  	  if (suitable_set_for_replacement (insn, &dest, &src))
>  	    {
> -	      rtx_expr_list **pnote, **pnote_next;
> -
>  	      replace_in_expr (expr, dest, src);
>  	      if (CONSTANT_P (*expr))
>  		goto out;
>  
> -	      for (pnote = &cond_list; *pnote; pnote = pnote_next)
> +	      unsigned int len = cond_list.length ();
> +	      for (unsigned int i = len - 1; i < len; i--)
>  		{
> -		  rtx_expr_list *note = *pnote;
> -		  rtx old_cond = XEXP (note, 0);
> +		  rtx old_cond = cond_list[i];
>  
> -		  pnote_next = (rtx_expr_list **)&XEXP (note, 1);
> -		  replace_in_expr (&XEXP (note, 0), dest, src);
> +		  replace_in_expr (&cond_list[i], dest, src);
>  
>  		  /* We can no longer use a condition that has been simplified
>  		     to a constant, and simplify_using_condition will abort if
>  		     we try.  */
> -		  if (CONSTANT_P (XEXP (note, 0)))
> -		    {
> -		      *pnote = *pnote_next;
> -		      pnote_next = pnote;
> -		      free_EXPR_LIST_node (note);
> -		    }
> +		  if (CONSTANT_P (cond_list[i]))
> +		    cond_list.ordered_remove (i);

Do we really need ordered removes here and below?  Obviously it turns
the original O(1) operation into O(n), and it wasn't obvious from first
glance that the order of the conditions was relevant.

Thanks,
Richard

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCH 0/6] remove some usage of rtx_{insn,expr}_list
  2016-06-20 16:52 ` [PATCH 0/6] remove some usage of rtx_{insn,expr}_list Bernd Schmidt
@ 2016-06-21 14:43   ` Trevor Saunders
  2016-06-21 14:59     ` Jeff Law
  2016-07-01 13:35     ` Bernd Schmidt
  0 siblings, 2 replies; 15+ messages in thread
From: Trevor Saunders @ 2016-06-21 14:43 UTC (permalink / raw)
  To: Bernd Schmidt; +Cc: tbsaunde+gcc, gcc-patches

On Mon, Jun 20, 2016 at 06:52:35PM +0200, Bernd Schmidt wrote:
> On 06/20/2016 12:22 PM, tbsaunde+gcc@tbsaunde.org wrote:
> > In theory I would expect if anything this helps performance since it isn't
> > necessary to malloc every time a node is added, however the data is less clear.
> 
> Well, we have alloc pools for these lists, so a malloc is not needed for
> every node.

its true, and lists.c has its own special cache, but still it is more
than storing a pointer and incrementing the length I expect.

> > fold const O2 new
> > real    0m5.034s
> > user    0m3.408s
> > sys     0m0.364s
> > 
> > fold const O2 old
> > real    0m4.012s
> > user    0m3.420s
> > sys     0m0.340s
> 
> So that's a second more in real time - was the machine very busy at the time
> you ran these tests so that these aren't meaningful, or is there a need to
> investigate this?

Well, it was on my laptop which was running a web browser and stuff.  I
wasn't aware of it being busy, but it also wasn't a extra stable
machine.  I also noticed a bit of variance within the same configuration
so I'm not terribly concerned, but it is odd.

> > So a couple got about .3s slower, and others got about .1 faster, I'm not
> > really sure but inclined to say any change is too small to easily measure.
> > 
> > bootstrapped + regtested patches individually on x86_64-linux-gnu, ok?
> 
> Modulo the question about compile times I think patches 1-4 are ok, In 5 and
> 6 I see explicit for loops instead of FOR_EACH macros; I'm curious as to the
> reason.

uh, I suck and wasn't careful enough checking I fixed everything and I
guess it was easy to forget since I've been destracted by other stuff.
Sorry about that.

Trev

> 
> Bernd
> 

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCH 6/6] loop-iv.c: make cond_list a vec
  2016-06-20 20:11   ` Richard Sandiford
@ 2016-06-21 14:46     ` Trevor Saunders
  0 siblings, 0 replies; 15+ messages in thread
From: Trevor Saunders @ 2016-06-21 14:46 UTC (permalink / raw)
  To: tbsaunde+gcc, gcc-patches, rdsandiford

On Mon, Jun 20, 2016 at 09:11:21PM +0100, Richard Sandiford wrote:
> tbsaunde+gcc@tbsaunde.org writes:
> > diff --git a/gcc/loop-iv.c b/gcc/loop-iv.c
> > index 57fb8c1..21c3180 100644
> > --- a/gcc/loop-iv.c
> > +++ b/gcc/loop-iv.c
> > @@ -1860,7 +1860,6 @@ simplify_using_initial_values (struct loop *loop, enum rtx_code op, rtx *expr)
> >  {
> >    bool expression_valid;
> >    rtx head, tail, last_valid_expr;
> > -  rtx_expr_list *cond_list;
> >    rtx_insn *insn;
> >    rtx neutral, aggr;
> >    regset altered, this_altered;
> > @@ -1936,7 +1935,7 @@ simplify_using_initial_values (struct loop *loop, enum rtx_code op, rtx *expr)
> >  
> >    expression_valid = true;
> >    last_valid_expr = *expr;
> > -  cond_list = NULL;
> > +  auto_vec<rtx> cond_list;
> >    while (1)
> >      {
> >        insn = BB_END (e->src);
> 
> How about using "auto_vec<rtx, N>" for some small N, since we expect
> cond_list to be used fairly often?

sure, why not?

> > @@ -1988,39 +1988,30 @@ simplify_using_initial_values (struct loop *loop, enum rtx_code op, rtx *expr)
> >  
> >  	  if (suitable_set_for_replacement (insn, &dest, &src))
> >  	    {
> > -	      rtx_expr_list **pnote, **pnote_next;
> > -
> >  	      replace_in_expr (expr, dest, src);
> >  	      if (CONSTANT_P (*expr))
> >  		goto out;
> >  
> > -	      for (pnote = &cond_list; *pnote; pnote = pnote_next)
> > +	      unsigned int len = cond_list.length ();
> > +	      for (unsigned int i = len - 1; i < len; i--)
> >  		{
> > -		  rtx_expr_list *note = *pnote;
> > -		  rtx old_cond = XEXP (note, 0);
> > +		  rtx old_cond = cond_list[i];
> >  
> > -		  pnote_next = (rtx_expr_list **)&XEXP (note, 1);
> > -		  replace_in_expr (&XEXP (note, 0), dest, src);
> > +		  replace_in_expr (&cond_list[i], dest, src);
> >  
> >  		  /* We can no longer use a condition that has been simplified
> >  		     to a constant, and simplify_using_condition will abort if
> >  		     we try.  */
> > -		  if (CONSTANT_P (XEXP (note, 0)))
> > -		    {
> > -		      *pnote = *pnote_next;
> > -		      pnote_next = pnote;
> > -		      free_EXPR_LIST_node (note);
> > -		    }
> > +		  if (CONSTANT_P (cond_list[i]))
> > +		    cond_list.ordered_remove (i);
> 
> Do we really need ordered removes here and below?  Obviously it turns
> the original O(1) operation into O(n), and it wasn't obvious from first
> glance that the order of the conditions was relevant.

I'm not sure, but I certainly don't know that we don't need them.  I
kind of ment to not send this patch because of that question but then
forgot.  I'm not really sure what to do with these, I don't know that I
know what's going on well enough to prove unordered removes are fine,
but I guess I can try.

Trev

> 
> Thanks,
> Richard

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCH 0/6] remove some usage of rtx_{insn,expr}_list
  2016-06-21 14:43   ` Trevor Saunders
@ 2016-06-21 14:59     ` Jeff Law
  2016-07-01 13:35     ` Bernd Schmidt
  1 sibling, 0 replies; 15+ messages in thread
From: Jeff Law @ 2016-06-21 14:59 UTC (permalink / raw)
  To: Trevor Saunders, Bernd Schmidt; +Cc: tbsaunde+gcc, gcc-patches

On 06/21/2016 08:47 AM, Trevor Saunders wrote:
> On Mon, Jun 20, 2016 at 06:52:35PM +0200, Bernd Schmidt wrote:
>> On 06/20/2016 12:22 PM, tbsaunde+gcc@tbsaunde.org wrote:
>>> In theory I would expect if anything this helps performance since it isn't
>>> necessary to malloc every time a node is added, however the data is less clear.
>>
>> Well, we have alloc pools for these lists, so a malloc is not needed for
>> every node.
>
> its true, and lists.c has its own special cache, but still it is more
> than storing a pointer and incrementing the length I expect.
Yea, it's got that cache.  IIRC that was a fairly minor optimization.  I 
wouldn't lose any sleep if it went away.  IIRC it was to reduce the cost 
of the lists we build up and tear down gcse.c.  It may have even been 
limited to load/store motion, in which case I really don't think the 
cache is all that important.



jeff

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCH 0/6] remove some usage of rtx_{insn,expr}_list
  2016-06-21 14:43   ` Trevor Saunders
  2016-06-21 14:59     ` Jeff Law
@ 2016-07-01 13:35     ` Bernd Schmidt
  2016-07-06 23:54       ` Trevor Saunders
  1 sibling, 1 reply; 15+ messages in thread
From: Bernd Schmidt @ 2016-07-01 13:35 UTC (permalink / raw)
  To: Trevor Saunders; +Cc: tbsaunde+gcc, gcc-patches

On 06/21/2016 04:47 PM, Trevor Saunders wrote:
> On Mon, Jun 20, 2016 at 06:52:35PM +0200, Bernd Schmidt wrote:

>> So that's a second more in real time - was the machine very busy at the time
>> you ran these tests so that these aren't meaningful, or is there a need to
>> investigate this?
>
> Well, it was on my laptop which was running a web browser and stuff.  I
> wasn't aware of it being busy, but it also wasn't a extra stable
> machine.  I also noticed a bit of variance within the same configuration
> so I'm not terribly concerned, but it is odd.

I ran some tests myself, and while they were somewhat inconclusive, they 
also didn't give me reason to think there's something to worry about. So 
ok for the first four patches.


Bernd

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCH 0/6] remove some usage of rtx_{insn,expr}_list
  2016-07-01 13:35     ` Bernd Schmidt
@ 2016-07-06 23:54       ` Trevor Saunders
  0 siblings, 0 replies; 15+ messages in thread
From: Trevor Saunders @ 2016-07-06 23:54 UTC (permalink / raw)
  To: Bernd Schmidt; +Cc: tbsaunde+gcc, gcc-patches

On Fri, Jul 01, 2016 at 03:34:51PM +0200, Bernd Schmidt wrote:
> On 06/21/2016 04:47 PM, Trevor Saunders wrote:
> > On Mon, Jun 20, 2016 at 06:52:35PM +0200, Bernd Schmidt wrote:
> 
> > > So that's a second more in real time - was the machine very busy at the time
> > > you ran these tests so that these aren't meaningful, or is there a need to
> > > investigate this?
> > 
> > Well, it was on my laptop which was running a web browser and stuff.  I
> > wasn't aware of it being busy, but it also wasn't a extra stable
> > machine.  I also noticed a bit of variance within the same configuration
> > so I'm not terribly concerned, but it is odd.
> 
> I ran some tests myself, and while they were somewhat inconclusive, they
> also didn't give me reason to think there's something to worry about. So ok
> for the first four patches.

Ok, thanks! committed now.

Trev

^ permalink raw reply	[flat|nested] 15+ messages in thread

end of thread, other threads:[~2016-07-06 23:54 UTC | newest]

Thread overview: 15+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2016-06-20 10:15 [PATCH 0/6] remove some usage of rtx_{insn,expr}_list tbsaunde+gcc
2016-06-20 10:15 ` [PATCH 2/6] remove unused loads rtx_insn_list tbsaunde+gcc
2016-06-20 10:15 ` [PATCH 3/6] make stores rtx_insn_list a vec tbsaunde+gcc
2016-06-20 10:16 ` [PATCH 4/6] make side_effects a vec<rtx> tbsaunde+gcc
2016-06-20 10:16 ` [PATCH 1/6] make antic_stores a vec<rtx_insn *> tbsaunde+gcc
2016-06-20 10:16 ` [PATCH 6/6] loop-iv.c: make cond_list a vec tbsaunde+gcc
2016-06-20 20:11   ` Richard Sandiford
2016-06-21 14:46     ` Trevor Saunders
2016-06-20 10:16 ` [PATCH 5/6] make pattern_regs " tbsaunde+gcc
2016-06-20 20:04   ` Richard Sandiford
2016-06-20 16:52 ` [PATCH 0/6] remove some usage of rtx_{insn,expr}_list Bernd Schmidt
2016-06-21 14:43   ` Trevor Saunders
2016-06-21 14:59     ` Jeff Law
2016-07-01 13:35     ` Bernd Schmidt
2016-07-06 23:54       ` Trevor Saunders

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).