Re: [PATCH 3/3] make stack_slot_list a vec

[Bernd Schmidt]

On 08/26/2016 06:03 PM, Trevor Saunders wrote:

yeah, it was just from being overly careful to not change behavior, but
that's silly we can always revert if it turns out to break something.


Not silly at all - it's a good mindset to have. But in cases where the 
order doesn't matter I think it's best to go for the simplest method so 
as to not puzzle future readers.



Bernd

Re: [PATCH 3/3] make stack_slot_list a vec

[Trevor Saunders]

On Thu, Aug 25, 2016 at 10:46:14AM +0200, Bernd Schmidt wrote:
> On 08/25/2016 08:50 AM, tbsaunde+...@tbsaunde.org wrote:
> > @@ -2626,8 +2626,10 @@ unshare_all_rtl_1 (rtx_insn *insn)
> >   This special care is necessary when the stack slot MEM does not
> >   actually appear in the insn chain.  If it does appear, its address
> >   is unshared from all else at that point.  */
> > -  stack_slot_list = safe_as_a  (
> > - copy_rtx_if_shared (stack_slot_list));
> > +  unsigned int i;
> > +  rtx temp;
> > +  FOR_EACH_VEC_SAFE_ELT_REVERSE (stack_slot_list, i, temp)
> > +(*stack_slot_list)[i] = copy_rtx_if_shared (temp);
> >  }
> > 
> >  /* Go through all the RTL insn bodies and copy any invalid shared
> > @@ -2656,7 +2658,10 @@ unshare_all_rtl_again (rtx_insn *insn)
> >for (decl = DECL_ARGUMENTS (cfun->decl); decl; decl = DECL_CHAIN (decl))
> >  set_used_flags (DECL_RTL (decl));
> > 
> > -  reset_used_flags (stack_slot_list);
> > +  rtx temp;
> > +  unsigned int i;
> > +  FOR_EACH_VEC_SAFE_ELT_REVERSE (stack_slot_list, i, temp)
> > +reset_used_flags (temp);
> 
> Same here, there doesn't seem to be a reason to go reverse in either of the
> two.

yeah, it was just from being overly careful to not change behavior, but
that's silly we can always revert if it turns out to break something.
Tests passed for this and the other patch, so I'll commit without the
reverse iteration later today.  Thanks for the reviews.

Trev

> 
> 
> Bernd

Re: [PATCH 3/3] make stack_slot_list a vec

[Bernd Schmidt]

On 08/25/2016 08:50 AM, tbsaunde+...@tbsaunde.org wrote:

@@ -2626,8 +2626,10 @@ unshare_all_rtl_1 (rtx_insn *insn)
  This special care is necessary when the stack slot MEM does not
  actually appear in the insn chain.  If it does appear, its address
  is unshared from all else at that point.  */
-  stack_slot_list = safe_as_a  (
- copy_rtx_if_shared (stack_slot_list));
+  unsigned int i;
+  rtx temp;
+  FOR_EACH_VEC_SAFE_ELT_REVERSE (stack_slot_list, i, temp)
+(*stack_slot_list)[i] = copy_rtx_if_shared (temp);
 }

 /* Go through all the RTL insn bodies and copy any invalid shared
@@ -2656,7 +2658,10 @@ unshare_all_rtl_again (rtx_insn *insn)
   for (decl = DECL_ARGUMENTS (cfun->decl); decl; decl = DECL_CHAIN (decl))
 set_used_flags (DECL_RTL (decl));

-  reset_used_flags (stack_slot_list);
+  rtx temp;
+  unsigned int i;
+  FOR_EACH_VEC_SAFE_ELT_REVERSE (stack_slot_list, i, temp)
+reset_used_flags (temp);


Same here, there doesn't seem to be a reason to go reverse in either of 
the two.



Bernd

[PATCH 3/3] make stack_slot_list a vec

[tbsaunde+gcc]

From: Trevor Saunders 

gcc/ChangeLog:

2016-08-25  Trevor Saunders  

* emit-rtl.h (struct rtl_data): Make stack_slot_list a vector.
* emit-rtl.c (unshare_all_rtl_1): Adjust.
(unshare_all_rtl_again): Likewise.
* function.c (assign_stack_local_1): Likewise.
(assign_stack_temp_for_type): Likewise.
---
 gcc/emit-rtl.c | 11 ---
 gcc/emit-rtl.h |  2 +-
 gcc/function.c |  8 +++-
 3 files changed, 12 insertions(+), 9 deletions(-)

diff --git a/gcc/emit-rtl.c b/gcc/emit-rtl.c
index 99f052d..c1051eb 100644
--- a/gcc/emit-rtl.c
+++ b/gcc/emit-rtl.c
@@ -2626,8 +2626,10 @@ unshare_all_rtl_1 (rtx_insn *insn)
  This special care is necessary when the stack slot MEM does not
  actually appear in the insn chain.  If it does appear, its address
  is unshared from all else at that point.  */
-  stack_slot_list = safe_as_a  (
- copy_rtx_if_shared (stack_slot_list));
+  unsigned int i;
+  rtx temp;
+  FOR_EACH_VEC_SAFE_ELT_REVERSE (stack_slot_list, i, temp)
+(*stack_slot_list)[i] = copy_rtx_if_shared (temp);
 }
 
 /* Go through all the RTL insn bodies and copy any invalid shared
@@ -2656,7 +2658,10 @@ unshare_all_rtl_again (rtx_insn *insn)
   for (decl = DECL_ARGUMENTS (cfun->decl); decl; decl = DECL_CHAIN (decl))
 set_used_flags (DECL_RTL (decl));
 
-  reset_used_flags (stack_slot_list);
+  rtx temp;
+  unsigned int i;
+  FOR_EACH_VEC_SAFE_ELT_REVERSE (stack_slot_list, i, temp)
+reset_used_flags (temp);
 
   unshare_all_rtl_1 (insn);
 }
diff --git a/gcc/emit-rtl.h b/gcc/emit-rtl.h
index 39dfce9..52c72b1 100644
--- a/gcc/emit-rtl.h
+++ b/gcc/emit-rtl.h
@@ -104,7 +104,7 @@ struct GTY(()) rtl_data {
 
   /* List (chain of EXPR_LISTs) of all stack slots in this function.
  Made for the sake of unshare_all_rtl.  */
-  rtx_expr_list *x_stack_slot_list;
+  vec *x_stack_slot_list;
 
   /* List of empty areas in the stack frame.  */
   struct frame_space *frame_space_list;
diff --git a/gcc/function.c b/gcc/function.c
index bba0705..53bad87 100644
--- a/gcc/function.c
+++ b/gcc/function.c
@@ -499,8 +499,7 @@ assign_stack_local_1 (machine_mode mode, HOST_WIDE_INT size,
   set_mem_align (x, alignment_in_bits);
   MEM_NOTRAP_P (x) = 1;
 
-  stack_slot_list
-= gen_rtx_EXPR_LIST (VOIDmode, x, stack_slot_list);
+  vec_safe_push (stack_slot_list, x);
 
   if (frame_offset_overflow (frame_offset, current_function_decl))
 frame_offset = 0;
@@ -829,8 +828,7 @@ assign_stack_temp_for_type (machine_mode mode, 
HOST_WIDE_INT size,
  p->type = best_p->type;
  insert_slot_to_list (p, _temp_slots);
 
- stack_slot_list = gen_rtx_EXPR_LIST (VOIDmode, p->slot,
-  stack_slot_list);
+ vec_safe_push (stack_slot_list, p->slot);
 
  best_p->size = rounded_size;
  best_p->full_size = rounded_size;
@@ -902,7 +900,7 @@ assign_stack_temp_for_type (machine_mode mode, 
HOST_WIDE_INT size,
 
   /* Create a new MEM rtx to avoid clobbering MEM flags of old slots.  */
   slot = gen_rtx_MEM (mode, XEXP (p->slot, 0));
-  stack_slot_list = gen_rtx_EXPR_LIST (VOIDmode, slot, stack_slot_list);
+  vec_safe_push (stack_slot_list, slot);
 
   /* If we know the alias set for the memory that will be used, use
  it.  If there's no TYPE, then we don't know anything about the
-- 
2.9.0