[PATCH 2/2] add static typed insn_deleted_p

[PATCH 2/2] add static typed insn_deleted_p

[tsaunders]

From: Trevor Saunders <tsaunders@mozilla.com>

Hi,
Same idea as the last patch.

also bootstrapped and regtested on x86_64-unknown-linux-gnu with config-list.mk
ongoing ok?


Trev

gcc/

	* cfgrtl.c, combine.c, config/arc/arc.c, config/mcore/mcore.c,
	config/rs6000/rs6000.c, config/sh/sh.c, cprop.c, dwarf2out.c,
	emit-rtl.c, final.c, function.c, gcse.c, jump.c, reg-stack.c,
	reload1.c, reorg.c, resource.c, sel-sched-ir.c: Replace INSN_DELETED_P
	macro with staticly checked member functions.
	* rtl.h (rtx_insn::deleted): New method.
	(rtx_insn::set_deleted): Likewise.
	(rtx_insn::set_undeleted): Likewise.
---
 gcc/cfgrtl.c               |  8 ++++----
 gcc/combine.c              |  2 +-
 gcc/config/arc/arc.c       |  4 ++--
 gcc/config/mcore/mcore.c   |  2 +-
 gcc/config/rs6000/rs6000.c |  2 +-
 gcc/config/sh/sh.c         |  8 ++++----
 gcc/cprop.c                |  6 +++---
 gcc/dwarf2out.c            |  2 +-
 gcc/emit-rtl.c             |  6 +++---
 gcc/final.c                |  4 ++--
 gcc/function.c             |  2 +-
 gcc/gcse.c                 |  2 +-
 gcc/jump.c                 | 20 ++++++++++----------
 gcc/reg-stack.c            |  2 +-
 gcc/reload1.c              |  2 +-
 gcc/reorg.c                | 12 ++++++------
 gcc/resource.c             |  3 +--
 gcc/rtl.h                  | 13 +++++++++++++
 gcc/sel-sched-ir.c         |  4 ++--
 19 files changed, 58 insertions(+), 46 deletions(-)

diff --git a/gcc/cfgrtl.c b/gcc/cfgrtl.c
index bc6c965..49779fc 100644
--- a/gcc/cfgrtl.c
+++ b/gcc/cfgrtl.c
@@ -165,11 +165,11 @@ delete_insn (rtx uncast_insn)
   if (really_delete)
     {
       /* If this insn has already been deleted, something is very wrong.  */
-      gcc_assert (!INSN_DELETED_P (insn));
+      gcc_assert (!insn->deleted ());
       if (INSN_P (insn))
 	df_insn_delete (insn);
       remove_insn (insn);
-      INSN_DELETED_P (insn) = 1;
+      insn->set_deleted ();
     }
 
   /* If deleting a jump, decrement the use count of the label.  Deleting
@@ -254,7 +254,7 @@ delete_insn_chain (rtx start, rtx finish, bool clear_bb)
       else
 	delete_insn (current);
 
-      if (clear_bb && !INSN_DELETED_P (current))
+      if (clear_bb && !current->deleted ())
 	set_block_for_insn (current, NULL);
 
       if (current == start)
@@ -3278,7 +3278,7 @@ fixup_abnormal_edges (void)
 		      if (GET_CODE (PATTERN (insn)) != USE)
 			{
 			  /* We're not deleting it, we're moving it.  */
-			  INSN_DELETED_P (insn) = 0;
+			  insn->set_undeleted ();
 			  SET_PREV_INSN (insn) = NULL_RTX;
 			  SET_NEXT_INSN (insn) = NULL_RTX;
 
diff --git a/gcc/combine.c b/gcc/combine.c
index 60524b5..67fd5b1 100644
--- a/gcc/combine.c
+++ b/gcc/combine.c
@@ -1238,7 +1238,7 @@ combine_instructions (rtx_insn *f, unsigned int nregs)
 	    continue;
 
 	  while (last_combined_insn
-		 && INSN_DELETED_P (last_combined_insn))
+		 && last_combined_insn->deleted ())
 	    last_combined_insn = PREV_INSN (last_combined_insn);
 	  if (last_combined_insn == NULL_RTX
 	      || BARRIER_P (last_combined_insn)
diff --git a/gcc/config/arc/arc.c b/gcc/config/arc/arc.c
index 7cddab7..89d0c11 100644
--- a/gcc/config/arc/arc.c
+++ b/gcc/config/arc/arc.c
@@ -8624,7 +8624,7 @@ arc_unalign_branch_p (rtx branch)
     return 0;
   /* Do not do this if we have a filled delay slot.  */
   if (get_attr_delay_slot_filled (branch) == DELAY_SLOT_FILLED_YES
-      && !INSN_DELETED_P (NEXT_INSN (branch)))
+      && !NEXT_INSN (branch)->deleted ())
     return 0;
   note = find_reg_note (branch, REG_BR_PROB, 0);
   return (!note
@@ -8705,7 +8705,7 @@ arc_pad_return (void)
 	  rtx save_pred = current_insn_predicate;
 	  final_scan_insn (prev, asm_out_file, optimize, 1, NULL);
 	  cfun->machine->force_short_suffix = -1;
-	  INSN_DELETED_P (prev) = 1;
+	  prev->set_deleted ();
 	  current_output_insn = insn;
 	  current_insn_predicate = save_pred;
 	}
diff --git a/gcc/config/mcore/mcore.c b/gcc/config/mcore/mcore.c
index aad0f1a..e2208b7 100644
--- a/gcc/config/mcore/mcore.c
+++ b/gcc/config/mcore/mcore.c
@@ -2536,7 +2536,7 @@ conditionalize_block (rtx_insn *first)
     {
       rtx_insn *newinsn;
 
-      if (INSN_DELETED_P (insn))
+      if (insn->deleted ())
 	continue;
       
       /* Try to form a conditional variant of the instruction and emit it.  */
diff --git a/gcc/config/rs6000/rs6000.c b/gcc/config/rs6000/rs6000.c
index 6370304..28e47d2 100644
--- a/gcc/config/rs6000/rs6000.c
+++ b/gcc/config/rs6000/rs6000.c
@@ -34184,7 +34184,7 @@ replace_swap_with_copy (swap_web_entry *insn_entry, unsigned i)
 
   df_insn_delete (insn);
   remove_insn (insn);
-  INSN_DELETED_P (insn) = 1;
+  insn->set_deleted ();
 }
 
 /* Dump the swap table to DUMP_FILE.  */
diff --git a/gcc/config/sh/sh.c b/gcc/config/sh/sh.c
index e85ca57..acadf78 100644
--- a/gcc/config/sh/sh.c
+++ b/gcc/config/sh/sh.c
@@ -5572,7 +5572,7 @@ gen_block_redirect (rtx_insn *jump, int addr, int need_block)
   rtx dest;
 
   /* First, check if we already have an instruction that satisfies our need.  */
-  if (prev && NONJUMP_INSN_P (prev) && ! INSN_DELETED_P (prev))
+  if (prev && NONJUMP_INSN_P (prev) && ! prev->deleted ())
     {
       if (INSN_CODE (prev) == CODE_FOR_indirect_jump_scratch)
 	return prev;
@@ -5615,7 +5615,7 @@ gen_block_redirect (rtx_insn *jump, int addr, int need_block)
 	{
 	  enum rtx_code code;
 
-	  if (INSN_DELETED_P (scan))
+	  if (scan->deleted ())
 	    continue;
 	  code = GET_CODE (scan);
 	  if (code == CODE_LABEL || code == JUMP_INSN)
@@ -5634,7 +5634,7 @@ gen_block_redirect (rtx_insn *jump, int addr, int need_block)
 	{
 	  enum rtx_code code;
 
-	  if (INSN_DELETED_P (scan))
+	  if (scan->deleted ())
 	    continue;
 	  code = GET_CODE (scan);
 	  if (INSN_P (scan))
@@ -6494,7 +6494,7 @@ split_branches (rtx_insn *first)
   for (insn = first; insn; insn = NEXT_INSN (insn))
     if (! INSN_P (insn))
       continue;
-    else if (INSN_DELETED_P (insn))
+    else if (insn->deleted ())
       {
 	/* Shorten_branches would split this instruction again,
 	   so transform it into a note.  */
diff --git a/gcc/cprop.c b/gcc/cprop.c
index fa77faa..3711508 100644
--- a/gcc/cprop.c
+++ b/gcc/cprop.c
@@ -1071,7 +1071,7 @@ retry:
 		  print_rtl (dump_file, src);
 		  fprintf (dump_file, "\n");
 		}
-	      if (INSN_DELETED_P (insn))
+	      if (insn->deleted ())
 		return 1;
 	    }
 	}
@@ -1257,7 +1257,7 @@ local_cprop_pass (void)
 			  break;
 			}
 		    }
-		  if (INSN_DELETED_P (insn))
+		  if (insn->deleted ())
 		    break;
 		}
 	      while (i < reg_use_count);
@@ -1854,7 +1854,7 @@ one_cprop_pass (void)
 		/* ??? Need to be careful w.r.t. mods done to INSN.
 		       Don't call mark_oprs_set if we turned the
 		       insn into a NOTE, or deleted the insn.  */
-		if (! NOTE_P (insn) && ! INSN_DELETED_P (insn))
+		if (! NOTE_P (insn) && ! insn->deleted ())
 		  mark_oprs_set (insn);
 	      }
 	}
diff --git a/gcc/dwarf2out.c b/gcc/dwarf2out.c
index 23a80d8..f9327d6 100644
--- a/gcc/dwarf2out.c
+++ b/gcc/dwarf2out.c
@@ -21328,7 +21328,7 @@ dwarf2out_var_location (rtx_insn *loc_note)
 
   next_note = NEXT_INSN (loc_note);
   if (! next_note
-      || INSN_DELETED_P (next_note)
+      || next_note->deleted ()
       || ! NOTE_P (next_note)
       || (NOTE_KIND (next_note) != NOTE_INSN_VAR_LOCATION
 	  && NOTE_KIND (next_note) != NOTE_INSN_CALL_ARG_LOCATION))
diff --git a/gcc/emit-rtl.c b/gcc/emit-rtl.c
index e3df826..9a20e0e 100644
--- a/gcc/emit-rtl.c
+++ b/gcc/emit-rtl.c
@@ -3796,7 +3796,7 @@ try_split (rtx pat, rtx uncast_trial, int last)
      We can't use next_active_insn here since AFTER may be a note.
      Ignore deleted insns, which can be occur if not optimizing.  */
   for (tem = NEXT_INSN (before); tem != after; tem = NEXT_INSN (tem))
-    if (! INSN_DELETED_P (tem) && INSN_P (tem))
+    if (! tem->deleted () && INSN_P (tem))
       tem = try_split (PATTERN (tem), tem, 1);
 
   /* Return either the first or the last insn, depending on which was
@@ -3973,7 +3973,7 @@ add_insn_after_nobb (rtx_insn *insn, rtx_insn *after)
 {
   rtx_insn *next = NEXT_INSN (after);
 
-  gcc_assert (!optimize || !INSN_DELETED_P (after));
+  gcc_assert (!optimize || !after->deleted ());
 
   link_insn_into_chain (insn, after, next);
 
@@ -4002,7 +4002,7 @@ add_insn_before_nobb (rtx_insn *insn, rtx_insn *before)
 {
   rtx_insn *prev = PREV_INSN (before);
 
-  gcc_assert (!optimize || !INSN_DELETED_P (before));
+  gcc_assert (!optimize || !before->deleted ());
 
   link_insn_into_chain (insn, prev, before);
 
diff --git a/gcc/final.c b/gcc/final.c
index 6469f40..114e83d 100644
--- a/gcc/final.c
+++ b/gcc/final.c
@@ -1133,7 +1133,7 @@ shorten_branches (rtx_insn *first)
       if (NOTE_P (insn) || BARRIER_P (insn)
 	  || LABEL_P (insn) || DEBUG_INSN_P (insn))
 	continue;
-      if (INSN_DELETED_P (insn))
+      if (insn->deleted ())
 	continue;
 
       body = PATTERN (insn);
@@ -2185,7 +2185,7 @@ final_scan_insn (rtx uncast_insn, FILE *file, int optimize_p ATTRIBUTE_UNUSED,
 
   /* Ignore deleted insns.  These can occur when we split insns (due to a
      template of "#") while not optimizing.  */
-  if (INSN_DELETED_P (insn))
+  if (insn->deleted ())
     return NEXT_INSN (insn);
 
   switch (GET_CODE (insn))
diff --git a/gcc/function.c b/gcc/function.c
index 8b125ae..8428f8a 100644
--- a/gcc/function.c
+++ b/gcc/function.c
@@ -1933,7 +1933,7 @@ instantiate_virtual_regs (void)
 	else
 	  instantiate_virtual_regs_in_insn (insn);
 
-	if (INSN_DELETED_P (insn))
+	if (insn->deleted ())
 	  continue;
 
 	instantiate_virtual_regs_in_rtx (&REG_NOTES (insn));
diff --git a/gcc/gcse.c b/gcc/gcse.c
index 4a8fe50..15b8bca 100644
--- a/gcc/gcse.c
+++ b/gcc/gcse.c
@@ -2486,7 +2486,7 @@ pre_insert_copies (void)
 		  continue;
 
 		/* Don't handle this one if it's a redundant one.  */
-		if (INSN_DELETED_P (insn))
+		if (insn->deleted ())
 		  continue;
 
 		/* Or if the expression doesn't reach the deleted one.  */
diff --git a/gcc/jump.c b/gcc/jump.c
index 12edd92..300cae5 100644
--- a/gcc/jump.c
+++ b/gcc/jump.c
@@ -291,7 +291,7 @@ mark_all_labels (rtx_insn *f)
 	     handled by other optimizers using better algorithms.  */
 	  FOR_BB_INSNS (bb, insn)
 	    {
-	      gcc_assert (! INSN_DELETED_P (insn));
+	      gcc_assert (! insn->deleted ());
 	      if (NONDEBUG_INSN_P (insn))
 	        mark_jump_label (PATTERN (insn), insn, 0);
 	    }
@@ -312,7 +312,7 @@ mark_all_labels (rtx_insn *f)
       rtx_insn *prev_nonjump_insn = NULL;
       for (insn = f; insn; insn = NEXT_INSN (insn))
 	{
-	  if (INSN_DELETED_P (insn))
+	  if (insn->deleted ())
 	    ;
 	  else if (LABEL_P (insn))
 	    prev_nonjump_insn = NULL;
@@ -1254,11 +1254,11 @@ delete_related_insns (rtx uncast_insn)
   rtx note;
   rtx_insn *next = NEXT_INSN (insn), *prev = PREV_INSN (insn);
 
-  while (next && INSN_DELETED_P (next))
+  while (next && next->deleted ())
     next = NEXT_INSN (next);
 
   /* This insn is already deleted => return first following nondeleted.  */
-  if (INSN_DELETED_P (insn))
+  if (insn->deleted ())
     return next;
 
   delete_insn (insn);
@@ -1279,7 +1279,7 @@ delete_related_insns (rtx uncast_insn)
     {
       rtx_insn *p;
 
-      for (p = next && INSN_DELETED_P (next) ? NEXT_INSN (next) : next;
+      for (p = next && next->deleted () ? NEXT_INSN (next) : next;
 	   p && NOTE_P (p);
 	   p = NEXT_INSN (p))
 	if (NOTE_KIND (p) == NOTE_INSN_CALL_ARG_LOCATION)
@@ -1323,7 +1323,7 @@ delete_related_insns (rtx uncast_insn)
       for (i = 0; i < len; i++)
 	if (LABEL_NUSES (XEXP (XVECEXP (pat, diff_vec_p, i), 0)) == 0)
 	  delete_related_insns (XEXP (XVECEXP (pat, diff_vec_p, i), 0));
-      while (next && INSN_DELETED_P (next))
+      while (next && next->deleted ())
 	next = NEXT_INSN (next);
       return next;
     }
@@ -1339,7 +1339,7 @@ delete_related_insns (rtx uncast_insn)
 	if (LABEL_NUSES (XEXP (note, 0)) == 0)
 	  delete_related_insns (XEXP (note, 0));
 
-  while (prev && (INSN_DELETED_P (prev) || NOTE_P (prev)))
+  while (prev && (prev->deleted () || NOTE_P (prev)))
     prev = PREV_INSN (prev);
 
   /* If INSN was a label and a dispatch table follows it,
@@ -1362,7 +1362,7 @@ delete_related_insns (rtx uncast_insn)
 	  if (code == NOTE)
 	    next = NEXT_INSN (next);
 	  /* Keep going past other deleted labels to delete what follows.  */
-	  else if (code == CODE_LABEL && INSN_DELETED_P (next))
+	  else if (code == CODE_LABEL && next->deleted ())
 	    next = NEXT_INSN (next);
 	  /* Keep the (use (insn))s created by dbr_schedule, which needs
 	     them in order to track liveness relative to a previous
@@ -1386,7 +1386,7 @@ delete_related_insns (rtx uncast_insn)
      but I see no clean and sure alternative way
      to find the first insn after INSN that is not now deleted.
      I hope this works.  */
-  while (next && INSN_DELETED_P (next))
+  while (next && next->deleted ())
     next = NEXT_INSN (next);
   return next;
 }
@@ -1408,7 +1408,7 @@ delete_for_peephole (rtx_insn *from, rtx_insn *to)
 
       if (!NOTE_P (insn))
 	{
-	  INSN_DELETED_P (insn) = 1;
+	  insn->set_deleted();
 
 	  /* Patch this insn out of the chain.  */
 	  /* We don't do this all at once, because we
diff --git a/gcc/reg-stack.c b/gcc/reg-stack.c
index af8e3cd..8c0a5c8 100644
--- a/gcc/reg-stack.c
+++ b/gcc/reg-stack.c
@@ -2341,7 +2341,7 @@ subst_stack_regs (rtx_insn *insn, stack_ptr regstack)
   /* subst_stack_regs_pat may have deleted a no-op insn.  If so, any
      REG_UNUSED will already have been dealt with, so just return.  */
 
-  if (NOTE_P (insn) || INSN_DELETED_P (insn))
+  if (NOTE_P (insn) || insn->deleted ())
     return control_flow_insn_deleted;
 
   /* If this a noreturn call, we can't insert pop insns after it.
diff --git a/gcc/reload1.c b/gcc/reload1.c
index c18ee67..f258a2b 100644
--- a/gcc/reload1.c
+++ b/gcc/reload1.c
@@ -8847,7 +8847,7 @@ delete_output_reload (rtx_insn *insn, int j, int last_reload_reg,
 
   /* It is possible that this reload has been only used to set another reload
      we eliminated earlier and thus deleted this instruction too.  */
-  if (INSN_DELETED_P (output_reload_insn))
+  if (output_reload_insn->deleted ())
     return;
 
   /* Get the raw pseudo-register referred to.  */
diff --git a/gcc/reorg.c b/gcc/reorg.c
index 18717a7..35d24d1 100644
--- a/gcc/reorg.c
+++ b/gcc/reorg.c
@@ -535,7 +535,7 @@ emit_delay_sequence (rtx_insn *insn, rtx_insn_list *list, int length)
       rtx note, next;
 
       /* Show that this copy of the insn isn't deleted.  */
-      INSN_DELETED_P (tem) = 0;
+      tem->set_undeleted ();
 
       /* Unlink insn from its original place, and re-emit it into
 	 the sequence.  */
@@ -1425,7 +1425,7 @@ try_merge_delay_insns (rtx insn, rtx_insn *thread)
 
 		  update_block (dtrial, thread);
 		  new_rtx = delete_from_delay_slot (dtrial);
-	          if (INSN_DELETED_P (thread))
+	          if (thread->deleted ())
 		    thread = new_rtx;
 		  INSN_FROM_TARGET_P (next_to_match) = 0;
 		}
@@ -1463,7 +1463,7 @@ try_merge_delay_insns (rtx insn, rtx_insn *thread)
 
 	      update_block (merged_insns->insn (), thread);
 	      new_rtx = delete_from_delay_slot (merged_insns->insn ());
-	      if (INSN_DELETED_P (thread))
+	      if (thread->deleted ())
 		thread = new_rtx;
 	    }
 	  else
@@ -1946,7 +1946,7 @@ fill_simple_delay_slots (int non_jumps_p)
 
       insn = unfilled_slots_base[i];
       if (insn == 0
-	  || INSN_DELETED_P (insn)
+	  || insn->deleted ()
 	  || (NONJUMP_INSN_P (insn)
 	      && GET_CODE (PATTERN (insn)) == SEQUENCE)
 	  || (JUMP_P (insn) && non_jumps_p)
@@ -2860,7 +2860,7 @@ fill_eager_delay_slots (void)
 
       insn = unfilled_slots_base[i];
       if (insn == 0
-	  || INSN_DELETED_P (insn)
+	  || insn->deleted ()
 	  || !JUMP_P (insn)
 	  || ! (condjump_p (insn) || condjump_in_parallel_p (insn)))
 	continue;
@@ -3831,7 +3831,7 @@ dbr_schedule (rtx_insn *first)
       memset (total_annul_slots, 0, sizeof total_annul_slots);
       for (insn = first; insn; insn = NEXT_INSN (insn))
 	{
-	  if (! INSN_DELETED_P (insn)
+	  if (! insn->deleted ()
 	      && NONJUMP_INSN_P (insn)
 	      && GET_CODE (PATTERN (insn)) != USE
 	      && GET_CODE (PATTERN (insn)) != CLOBBER)
diff --git a/gcc/resource.c b/gcc/resource.c
index 5528831..6035c49 100644
--- a/gcc/resource.c
+++ b/gcc/resource.c
@@ -930,8 +930,7 @@ mark_target_live_regs (rtx_insn *insns, rtx target_maybe_return, struct resource
 	 information, we can get it from there unless the insn at the
 	 start of the basic block has been deleted.  */
       if (tinfo && tinfo->block != -1
-	  && ! INSN_DELETED_P (BB_HEAD (BASIC_BLOCK_FOR_FN (cfun,
-							    tinfo->block))))
+	  && ! BB_HEAD (BASIC_BLOCK_FOR_FN (cfun, tinfo->block))->deleted ())
 	b = tinfo->block;
     }
 
diff --git a/gcc/rtl.h b/gcc/rtl.h
index 2d66a89..e364698 100644
--- a/gcc/rtl.h
+++ b/gcc/rtl.h
@@ -490,6 +490,7 @@ is_a_helper <const rtx_sequence *>::test (const_rtx rt)
 
 class GTY(()) rtx_insn : public rtx_def
 {
+public:
   /* No extra fields, but adds the invariant:
 
      (INSN_P (X)
@@ -505,6 +506,18 @@ class GTY(()) rtx_insn : public rtx_def
     i.e. we have an rtx that has an INSN_UID field and can be part of
     a linked list of insns.
   */
+
+  /* Returns true if this insn has been deleted.  */
+
+  bool deleted () const { return volatil; }
+
+  /* Mark this insn as deleted.  */
+
+  void set_deleted () { volatil = true; }
+
+  /* Mark this insn as not deleted.  */
+
+  void set_undeleted () { volatil = false; }
 };
 
 /* Subclasses of rtx_insn.  */
diff --git a/gcc/sel-sched-ir.c b/gcc/sel-sched-ir.c
index 02dc8f2..0a6c52c 100644
--- a/gcc/sel-sched-ir.c
+++ b/gcc/sel-sched-ir.c
@@ -1069,7 +1069,7 @@ return_nop_to_pool (insn_t nop, bool full_tidying)
   sel_remove_insn (nop, false, full_tidying);
 
   /* We'll recycle this nop.  */
-  INSN_DELETED_P (nop) = 0;
+  nop->set_undeleted ();
 
   if (nop_pool.n == nop_pool.s)
     nop_pool.v = XRESIZEVEC (rtx_insn *, nop_pool.v,
@@ -1404,7 +1404,7 @@ sel_gen_insn_from_expr_after (expr_t expr, vinsn_t vinsn, int seqno,
 
   /* The insn may come from the transformation cache, which may hold already
      deleted insns, so mark it as not deleted.  */
-  INSN_DELETED_P (insn) = 0;
+  insn->set_undeleted ();
 
   add_insn_after (insn, after, BLOCK_FOR_INSN (insn));
 
-- 
2.1.0

[PATCH 1/2] add staticly checked label_nuses accessors

[tsaunders]

From: Trevor Saunders <tsaunders@mozilla.com>

Hi,

 Doing this means we get to remove a fair chunk of runtime checking.

bootstrapped + regtested on x86_64-unknown-linux-gnu with no regressions.
config-list.mk with this and the next patch is ongoing. ok?

Trev

gcc/

	* config/i386/i386.c, config/i386/i386.md, config/mep/mep.c,
	config/mips/mips.c, config/nios2/nios2.c, config/s390/s390.c,
	config/sh/sh.c, reorg.c: Use rtx_code_label::nuses and
	rtx_code_label::set_nuses where possible.
	* rtl.h (rtx_code_label::nuses): New member function.
	(rtx_code_label::set_nuses): Likewise.
---
 gcc/config/i386/i386.c   | 56 ++++++++++++++++++++++++------------------------
 gcc/config/i386/i386.md  | 14 ++++++------
 gcc/config/mep/mep.c     |  2 +-
 gcc/config/mips/mips.c   |  2 +-
 gcc/config/nios2/nios2.c |  4 ++--
 gcc/config/s390/s390.c   |  4 ++--
 gcc/config/sh/sh.c       |  2 +-
 gcc/reorg.c              |  6 +++---
 gcc/rtl.h                |  9 ++++++++
 9 files changed, 54 insertions(+), 45 deletions(-)

diff --git a/gcc/config/i386/i386.c b/gcc/config/i386/i386.c
index a117718..e2b4a00 100644
--- a/gcc/config/i386/i386.c
+++ b/gcc/config/i386/i386.c
@@ -12136,7 +12136,7 @@ ix86_expand_split_stack_prologue (void)
     }
 
   emit_label (label);
-  LABEL_NUSES (label) = 1;
+  label->set_nuses (1);
 
   /* If this function calls va_start, we now have to set the scratch
      register for the case where we do not call __morestack.  In this
@@ -12148,7 +12148,7 @@ ix86_expand_split_stack_prologue (void)
 					    GEN_INT (UNITS_PER_WORD))));
 
       emit_label (varargs_label);
-      LABEL_NUSES (varargs_label) = 1;
+      varargs_label->set_nuses (1);
     }
 }
 
@@ -23129,7 +23129,7 @@ expand_movmem_epilogue (rtx destmem, rtx srcmem,
 	  dest = change_address (destmem, SImode, destptr);
 	  emit_insn (gen_strmov (destptr, dest, srcptr, src));
 	  emit_label (label);
-	  LABEL_NUSES (label) = 1;
+	  label->set_nuses (1);
 	}
       if (max_size > 2)
 	{
@@ -23138,7 +23138,7 @@ expand_movmem_epilogue (rtx destmem, rtx srcmem,
 	  dest = change_address (destmem, HImode, destptr);
 	  emit_insn (gen_strmov (destptr, dest, srcptr, src));
 	  emit_label (label);
-	  LABEL_NUSES (label) = 1;
+	  label->set_nuses (1);
 	}
       if (max_size > 1)
 	{
@@ -23147,7 +23147,7 @@ expand_movmem_epilogue (rtx destmem, rtx srcmem,
 	  dest = change_address (destmem, QImode, destptr);
 	  emit_insn (gen_strmov (destptr, dest, srcptr, src));
 	  emit_label (label);
-	  LABEL_NUSES (label) = 1;
+	  label->set_nuses (1);
 	}
     }
   else
@@ -23166,7 +23166,7 @@ expand_movmem_epilogue (rtx destmem, rtx srcmem,
 	  if (tmp != offset)
 	    emit_move_insn (offset, tmp);
 	  emit_label (label);
-	  LABEL_NUSES (label) = 1;
+	  label->set_nuses (1);
 	}
       if (max_size > 2)
 	{
@@ -23181,7 +23181,7 @@ expand_movmem_epilogue (rtx destmem, rtx srcmem,
 	  if (tmp != offset)
 	    emit_move_insn (offset, tmp);
 	  emit_label (label);
-	  LABEL_NUSES (label) = 1;
+	  label->set_nuses (1);
 	}
       if (max_size > 1)
 	{
@@ -23192,7 +23192,7 @@ expand_movmem_epilogue (rtx destmem, rtx srcmem,
 	  dest = change_address (destmem, QImode, tmp);
 	  emit_move_insn (dest, src);
 	  emit_label (label);
-	  LABEL_NUSES (label) = 1;
+	  label->set_nuses (1);
 	}
     }
 }
@@ -23321,7 +23321,7 @@ expand_setmem_epilogue (rtx destmem, rtx destptr, rtx value, rtx vec_value,
 	  emit_insn (gen_strset (destptr, dest, value));
 	}
       emit_label (label);
-      LABEL_NUSES (label) = 1;
+      label->set_nuses (1);
     }
   if (max_size > 8)
     {
@@ -23339,7 +23339,7 @@ expand_setmem_epilogue (rtx destmem, rtx destptr, rtx value, rtx vec_value,
 	  emit_insn (gen_strset (destptr, dest, value));
 	}
       emit_label (label);
-      LABEL_NUSES (label) = 1;
+      label->set_nuses (1);
     }
   if (max_size > 4)
     {
@@ -23347,7 +23347,7 @@ expand_setmem_epilogue (rtx destmem, rtx destptr, rtx value, rtx vec_value,
       dest = change_address (destmem, SImode, destptr);
       emit_insn (gen_strset (destptr, dest, gen_lowpart (SImode, value)));
       emit_label (label);
-      LABEL_NUSES (label) = 1;
+      label->set_nuses (1);
     }
   if (max_size > 2)
     {
@@ -23355,7 +23355,7 @@ expand_setmem_epilogue (rtx destmem, rtx destptr, rtx value, rtx vec_value,
       dest = change_address (destmem, HImode, destptr);
       emit_insn (gen_strset (destptr, dest, gen_lowpart (HImode, value)));
       emit_label (label);
-      LABEL_NUSES (label) = 1;
+      label->set_nuses (1);
     }
   if (max_size > 1)
     {
@@ -23363,7 +23363,7 @@ expand_setmem_epilogue (rtx destmem, rtx destptr, rtx value, rtx vec_value,
       dest = change_address (destmem, QImode, destptr);
       emit_insn (gen_strset (destptr, dest, gen_lowpart (QImode, value)));
       emit_label (label);
-      LABEL_NUSES (label) = 1;
+      label->set_nuses (1);
     }
 }
 
@@ -23395,7 +23395,7 @@ expand_set_or_movmem_prologue (rtx destmem, rtx srcmem,
 	    destmem = emit_memmov (destmem, &srcmem, destptr, srcptr, i);
 	  ix86_adjust_counter (count, i);
 	  emit_label (label);
-	  LABEL_NUSES (label) = 1;
+	  label->set_nuses (1);
 	  set_mem_align (destmem, i * 2 * BITS_PER_UNIT);
 	}
     }
@@ -23478,7 +23478,7 @@ expand_small_movmem_or_setmem (rtx destmem, rtx srcmem,
   emit_barrier ();
 
   emit_label (label);
-  LABEL_NUSES (label) = 1;
+  label->set_nuses (1);
 }
 
 /* Handle small memcpy (up to SIZE that is supposed to be small power of 2.
@@ -23608,7 +23608,7 @@ expand_set_or_movmem_prologue_epilogue_by_misaligned_moves (rtx destmem, rtx src
 	}
 
       emit_label (label);
-      LABEL_NUSES (label) = 1;
+      label->set_nuses (1);
       emit_jump_insn (gen_jump (*done_label));
       emit_barrier ();
     }
@@ -23620,7 +23620,7 @@ expand_set_or_movmem_prologue_epilogue_by_misaligned_moves (rtx destmem, rtx src
   if (loop_label)
     {
        emit_label (loop_label);
-       LABEL_NUSES (loop_label) = 1;
+       loop_label->set_nuses (1);
     }
 
   /* Copy first desired_align bytes.  */
@@ -24505,7 +24505,7 @@ ix86_expand_set_or_movmem (rtx dst, rtx src, rtx count_exp, rtx val_exp,
   if (label && size_needed == 1)
     {
       emit_label (label);
-      LABEL_NUSES (label) = 1;
+      label->set_nuses (1);
       label = NULL;
       epilogue_size_needed = 1;
       if (issetmem)
@@ -24576,7 +24576,7 @@ ix86_expand_set_or_movmem (rtx dst, rtx src, rtx count_exp, rtx val_exp,
 	    emit_move_insn (count_exp, tmp);
 	}
       emit_label (label);
-      LABEL_NUSES (label) = 1;
+      label->set_nuses (1);
     }
 
   if (count_exp != const0_rtx && epilogue_size_needed > 1)
@@ -41490,7 +41490,7 @@ void ix86_emit_i387_round (rtx op0, rtx op1)
   emit_insn (gen_neg (res, res));
 
   emit_label (jump_label);
-  LABEL_NUSES (jump_label) = 1;
+  jump_label->set_nuses (1);
 
   emit_move_insn (op0, res);
 }
@@ -41935,7 +41935,7 @@ ix86_expand_lfloorceil (rtx op0, rtx op1, bool do_floor)
   emit_move_insn (ireg, tmp);
 
   emit_label (label);
-  LABEL_NUSES (label) = 1;
+  label->set_nuses (1);
 
   emit_move_insn (op0, ireg);
 }
@@ -41972,7 +41972,7 @@ ix86_expand_rint (rtx operand0, rtx operand1)
   ix86_sse_copysign_to_positive (res, xa, res, mask);
 
   emit_label (label);
-  LABEL_NUSES (label) = 1;
+  label->set_nuses (1);
 
   emit_move_insn (operand0, res);
 }
@@ -42035,7 +42035,7 @@ ix86_expand_floorceildf_32 (rtx operand0, rtx operand1, bool do_floor)
   emit_move_insn (res, tmp);
 
   emit_label (label);
-  LABEL_NUSES (label) = 1;
+  label->set_nuses (1);
 
   emit_move_insn (operand0, res);
 }
@@ -42097,7 +42097,7 @@ ix86_expand_floorceil (rtx operand0, rtx operand1, bool do_floor)
     ix86_sse_copysign_to_positive (res, res, force_reg (mode, operand1), mask);
 
   emit_label (label);
-  LABEL_NUSES (label) = 1;
+  label->set_nuses (1);
 
   emit_move_insn (operand0, res);
 }
@@ -42171,7 +42171,7 @@ ix86_expand_rounddf_32 (rtx operand0, rtx operand1)
   ix86_sse_copysign_to_positive (res, xa2, force_reg (mode, operand1), mask);
 
   emit_label (label);
-  LABEL_NUSES (label) = 1;
+  label->set_nuses (1);
 
   emit_move_insn (operand0, res);
 }
@@ -42216,7 +42216,7 @@ ix86_expand_trunc (rtx operand0, rtx operand1)
     ix86_sse_copysign_to_positive (res, res, force_reg (mode, operand1), mask);
 
   emit_label (label);
-  LABEL_NUSES (label) = 1;
+  label->set_nuses (1);
 
   emit_move_insn (operand0, res);
 }
@@ -42275,7 +42275,7 @@ ix86_expand_truncdf_32 (rtx operand0, rtx operand1)
   ix86_sse_copysign_to_positive (res, res, force_reg (mode, operand1), smask);
 
   emit_label (label);
-  LABEL_NUSES (label) = 1;
+  label->set_nuses (1);
 
   emit_move_insn (operand0, res);
 }
@@ -42325,7 +42325,7 @@ ix86_expand_round (rtx operand0, rtx operand1)
   ix86_sse_copysign_to_positive (res, xa, force_reg (mode, operand1), mask);
 
   emit_label (label);
-  LABEL_NUSES (label) = 1;
+  label->set_nuses (1);
 
   emit_move_insn (operand0, res);
 }
diff --git a/gcc/config/i386/i386.md b/gcc/config/i386/i386.md
index d5588c8..9b11e9c 100644
--- a/gcc/config/i386/i386.md
+++ b/gcc/config/i386/i386.md
@@ -9191,7 +9191,7 @@
   ix86_expand_clear (operands[1]);
 
   emit_label (label);
-  LABEL_NUSES (label) = 1;
+  label->set_nuses (1);
 
   DONE;
 })
@@ -9854,7 +9854,7 @@
   emit_insn (gen_ashr<mode>3_cvt (operands[1], operands[1],
 				  GEN_INT (GET_MODE_BITSIZE (<MODE>mode)-1)));
   emit_label (label);
-  LABEL_NUSES (label) = 1;
+  label->set_nuses (1);
 
   DONE;
 })
@@ -13855,7 +13855,7 @@
   emit_label (label);
   emit_insn (gen_fpremxf4_i387 (op1, op2, op1, op2));
   ix86_emit_fp_unordered_jump (label);
-  LABEL_NUSES (label) = 1;
+  label->set_nuses (1);
 
   emit_move_insn (operands[0], op1);
   DONE;
@@ -13880,7 +13880,7 @@
   emit_label (label);
   emit_insn (gen_fpremxf4_i387 (op1, op2, op1, op2));
   ix86_emit_fp_unordered_jump (label);
-  LABEL_NUSES (label) = 1;
+  label->set_nuses (1);
 
   /* Truncate the result properly for strict SSE math.  */
   if (SSE_FLOAT_MODE_P (<MODE>mode) && TARGET_SSE_MATH
@@ -13926,7 +13926,7 @@
   emit_label (label);
   emit_insn (gen_fprem1xf4_i387 (op1, op2, op1, op2));
   ix86_emit_fp_unordered_jump (label);
-  LABEL_NUSES (label) = 1;
+  label->set_nuses (1);
 
   emit_move_insn (operands[0], op1);
   DONE;
@@ -13952,7 +13952,7 @@
 
   emit_insn (gen_fprem1xf4_i387 (op1, op2, op1, op2));
   ix86_emit_fp_unordered_jump (label);
-  LABEL_NUSES (label) = 1;
+  label->set_nuses (1);
 
   /* Truncate the result properly for strict SSE math.  */
   if (SSE_FLOAT_MODE_P (<MODE>mode) && TARGET_SSE_MATH
@@ -18542,7 +18542,7 @@
   emit_jump_insn (gen_xbegin_1 (ax_reg, label));
 
   emit_label (label);
-  LABEL_NUSES (label) = 1;
+  label->set_nuses (1);
 
   emit_move_insn (operands[0], ax_reg);
 
diff --git a/gcc/config/mep/mep.c b/gcc/config/mep/mep.c
index fcd5e0a..0f36a62 100644
--- a/gcc/config/mep/mep.c
+++ b/gcc/config/mep/mep.c
@@ -5579,7 +5579,7 @@ mep_reorg_erepeat (rtx_insn *insns)
 
 		/* Insert the erepeat after INSN's target label.  */
 		x = gen_erepeat (gen_rtx_LABEL_REF (VOIDmode, l));
-		LABEL_NUSES (l)++;
+		l->set_nuses (l->nuses () + 1);
 		emit_insn_after (x, prev);
 
 		/* Insert the erepeat label.  */
diff --git a/gcc/config/mips/mips.c b/gcc/config/mips/mips.c
index 3e77491..76c0b33 100644
--- a/gcc/config/mips/mips.c
+++ b/gcc/config/mips/mips.c
@@ -14892,7 +14892,7 @@ mips16_lay_out_constants (bool split_p)
 
 	      jump = emit_jump_insn_before (gen_jump (label), insn);
 	      JUMP_LABEL (jump) = label;
-	      LABEL_NUSES (label) = 1;
+	      label->set_nuses (1);
 	      barrier = emit_barrier_after (jump);
 
 	      emit_label_after (label, barrier);
diff --git a/gcc/config/nios2/nios2.c b/gcc/config/nios2/nios2.c
index 1ab74f9..be173a6 100644
--- a/gcc/config/nios2/nios2.c
+++ b/gcc/config/nios2/nios2.c
@@ -1342,7 +1342,7 @@ nios2_emit_expensive_div (rtx *operands, enum machine_mode mode)
   emit_barrier ();
 
   emit_label (lab3);
-  LABEL_NUSES (lab3) = 1;
+  lab3->set_nuses (1);
 
   start_sequence ();
   final_result = emit_library_call_value (libfunc, NULL_RTX,
@@ -1356,7 +1356,7 @@ nios2_emit_expensive_div (rtx *operands, enum machine_mode mode)
                       gen_rtx_DIV (SImode, operands[1], operands[2]));
 
   emit_label (lab1);
-  LABEL_NUSES (lab1) = 1;
+  lab1->set_nuses (1);
 }
 
 \f
diff --git a/gcc/config/s390/s390.c b/gcc/config/s390/s390.c
index 9e910cc..7b7652a 100644
--- a/gcc/config/s390/s390.c
+++ b/gcc/config/s390/s390.c
@@ -9895,7 +9895,7 @@ s390_expand_tbegin (rtx dest, rtx tdb, rtx retry, bool clobber_fprs_p)
 			     gen_rtx_EQ (VOIDmode,
 			       gen_rtx_REG (CCRAWmode, CC_REGNUM),
 			       gen_rtx_CONST_INT (VOIDmode, CC0 | CC1 | CC3)));
-      LABEL_NUSES (leave_label) = 1;
+      leave_label->set_nuses (1);
 
       /* CC2 - transient failure. Perform retry with ppa.  */
       emit_move_insn (count, retry_plus_two);
@@ -9905,7 +9905,7 @@ s390_expand_tbegin (rtx dest, rtx tdb, rtx retry, bool clobber_fprs_p)
 					      retry_reg,
 					      retry_reg));
       JUMP_LABEL (jump) = retry_label;
-      LABEL_NUSES (retry_label) = 1;
+      retry_label->set_nuses (1);
       emit_label (leave_label);
     }
 }
diff --git a/gcc/config/sh/sh.c b/gcc/config/sh/sh.c
index 849867a..e85ca57 100644
--- a/gcc/config/sh/sh.c
+++ b/gcc/config/sh/sh.c
@@ -5355,7 +5355,7 @@ find_barrier (int num_mova, rtx_insn *mova, rtx_insn *from)
 
       from = emit_jump_insn_after (gen_jump (label), from);
       JUMP_LABEL (from) = label;
-      LABEL_NUSES (label) = 1;
+      label->set_nuses (1);
       found_barrier = emit_barrier_after (from);
       emit_label_after (label, found_barrier);
     }
diff --git a/gcc/reorg.c b/gcc/reorg.c
index 89d500d..18717a7 100644
--- a/gcc/reorg.c
+++ b/gcc/reorg.c
@@ -427,7 +427,7 @@ find_end_label (rtx kind)
     {
       rtx_insn *temp = PREV_INSN (PREV_INSN (insn));
       rtx_code_label *label = gen_label_rtx ();
-      LABEL_NUSES (label) = 0;
+      label->set_nuses (0);
 
       /* Put the label before any USE insns that may precede the RETURN
 	 insn.  */
@@ -443,7 +443,7 @@ find_end_label (rtx kind)
   else
     {
       rtx_code_label *label = gen_label_rtx ();
-      LABEL_NUSES (label) = 0;
+      label->set_nuses (0);
       /* If the basic block reorder pass moves the return insn to
 	 some other place try to locate it again and put our
 	 function_return_label there.  */
@@ -496,7 +496,7 @@ find_end_label (rtx kind)
 
   /* Show one additional use for this label so it won't go away until
      we are done.  */
-  ++LABEL_NUSES (*plabel);
+  (*plabel)->set_nuses ((*plabel)->nuses () + 1);
 
   return *plabel;
 }
diff --git a/gcc/rtl.h b/gcc/rtl.h
index cd2f2e7..2d66a89 100644
--- a/gcc/rtl.h
+++ b/gcc/rtl.h
@@ -595,6 +595,7 @@ class GTY(()) rtx_barrier : public rtx_insn
 
 class GTY(()) rtx_code_label : public rtx_insn
 {
+public:
   /* No extra fields, but adds the invariant:
        LABEL_P (X) aka (GET_CODE (X) == CODE_LABEL)
      i.e. a label in the assembler.
@@ -602,6 +603,14 @@ class GTY(()) rtx_code_label : public rtx_insn
      This is an instance of:
        DEF_RTL_EXPR(CODE_LABEL, "code_label", "uuB00is", RTX_EXTRA)
      from rtl.def.  */
+
+  /* The number of times this label is used.  */
+
+  int nuses () const { return u.fld[4].rt_int; }
+
+  /* Set the number of times this label is used.  */
+
+  void set_nuses (int n) { u.fld[4].rt_int = n; }
 };
 
 class GTY(()) rtx_note : public rtx_insn
-- 
2.1.0

Re: [PATCH 1/2] add staticly checked label_nuses accessors

[Jeff Law]

On 09/05/14 09:32, tsaunders@mozilla.com wrote:
> From: Trevor Saunders <tsaunders@mozilla.com>
>
> Hi,
>
>   Doing this means we get to remove a fair chunk of runtime checking.
>
> bootstrapped + regtested on x86_64-unknown-linux-gnu with no regressions.
> config-list.mk with this and the next patch is ongoing. ok?
>
> Trev
>
> gcc/
>
> 	* config/i386/i386.c, config/i386/i386.md, config/mep/mep.c,
> 	config/mips/mips.c, config/nios2/nios2.c, config/s390/s390.c,
> 	config/sh/sh.c, reorg.c: Use rtx_code_label::nuses and
> 	rtx_code_label::set_nuses where possible.
> 	* rtl.h (rtx_code_label::nuses): New member function.
> 	(rtx_code_label::set_nuses): Likewise.
Is there some reason why we don't fix every reference to LABEL_NUSES to 
use the new member functions?  My concern here is that we've now got two 
ways actively used to get at that information, the old LABEL_NUSES and 
the new member functions.  Obviously we really just want one blessed way 
to get/set that information.  If this is a short term situation, then 
I'd be inclined to say OK, but I don't see further patches which sort 
out the ~200 or so LABEL_NUSES users.

THe approach David has taken for similar situations has been to first 
introduce the get/set methods, convert everything to set the get/set 
method, dealing with the type fallout that occurrs around that, then 
collapsing back to the old macro.  Is there some reason we can not or 
should not do that here?

jeff

Re: [PATCH 2/2] add static typed insn_deleted_p

[Jeff Law]

On 09/05/14 09:32, tsaunders@mozilla.com wrote:
> From: Trevor Saunders <tsaunders@mozilla.com>
>
> Hi,
> Same idea as the last patch.
>
> also bootstrapped and regtested on x86_64-unknown-linux-gnu with config-list.mk
> ongoing ok?
>
>
> Trev
>
> gcc/
>
> 	* cfgrtl.c, combine.c, config/arc/arc.c, config/mcore/mcore.c,
> 	config/rs6000/rs6000.c, config/sh/sh.c, cprop.c, dwarf2out.c,
> 	emit-rtl.c, final.c, function.c, gcse.c, jump.c, reg-stack.c,
> 	reload1.c, reorg.c, resource.c, sel-sched-ir.c: Replace INSN_DELETED_P
> 	macro with staticly checked member functions.
> 	* rtl.h (rtx_insn::deleted): New method.
> 	(rtx_insn::set_deleted): Likewise.
> 	(rtx_insn::set_undeleted): Likewise.
So in a similar manner, is there some reason that we continue to have 
INSN_DELETED_P as a macro?  Can we get to a point where either we don't 
have the macro because everything has been converted?  What's the plan 
for getting there?


jeff

Re: [PATCH 1/2] add staticly checked label_nuses accessors

[Trevor Saunders]

On Fri, Sep 05, 2014 at 10:43:45AM -0600, Jeff Law wrote:
> On 09/05/14 09:32, tsaunders@mozilla.com wrote:
> >From: Trevor Saunders <tsaunders@mozilla.com>
> >
> >Hi,
> >
> >  Doing this means we get to remove a fair chunk of runtime checking.
> >
> >bootstrapped + regtested on x86_64-unknown-linux-gnu with no regressions.
> >config-list.mk with this and the next patch is ongoing. ok?
> >
> >Trev
> >
> >gcc/
> >
> >	* config/i386/i386.c, config/i386/i386.md, config/mep/mep.c,
> >	config/mips/mips.c, config/nios2/nios2.c, config/s390/s390.c,
> >	config/sh/sh.c, reorg.c: Use rtx_code_label::nuses and
> >	rtx_code_label::set_nuses where possible.
> >	* rtl.h (rtx_code_label::nuses): New member function.
> >	(rtx_code_label::set_nuses): Likewise.
> Is there some reason why we don't fix every reference to LABEL_NUSES to use
> the new member functions?  My concern here is that we've now got two ways

 The reason I left some as is was because we didn't already have an
 rtx_code_label * there, so I was going to handle those later.  Though
 actually I guess we could adjust the macro to be

 #define LABEL_NUSES(l) as_a<rtx_code_label *> (l)->nuses ()

Whcih also has the advantage it doesn't compile if you pass in an
rtx_code_label *, and so you need to change to the newer form.

> actively used to get at that information, the old LABEL_NUSES and the new
> member functions.  Obviously we really just want one blessed way to get/set
> that information.  If this is a short term situation, then I'd be inclined
> to say OK, but I don't see further patches which sort out the ~200 or so
> LABEL_NUSES users.

because they haven't been written yet :) but I'd expect as types are
made more specific that situation will get better.

> THe approach David has taken for similar situations has been to first
> introduce the get/set methods, convert everything to set the get/set method,
> dealing with the type fallout that occurrs around that, then collapsing back
> to the old macro.  Is there some reason we can not or should not do that
> here?

I'd say we're doing the first parts of that here, but its not complete
yet.  That is I added the getters, and converted some but not all users
over.  There's certainly more patches to write though.

yes, basically the same thing is true for the other patch though we may
be much closer to finishing that conversion.

Trev

> 
> jeff
> 

Re: [PATCH 1/2] add staticly checked label_nuses accessors

[David Malcolm]

On Fri, 2014-09-05 at 10:43 -0600, Jeff Law wrote:
> On 09/05/14 09:32, tsaunders@mozilla.com wrote:
> > From: Trevor Saunders <tsaunders@mozilla.com>
> >
> > Hi,
> >
> >   Doing this means we get to remove a fair chunk of runtime checking.
> >
> > bootstrapped + regtested on x86_64-unknown-linux-gnu with no regressions.
> > config-list.mk with this and the next patch is ongoing. ok?
> >
> > Trev
> >
> > gcc/
> >
> > 	* config/i386/i386.c, config/i386/i386.md, config/mep/mep.c,
> > 	config/mips/mips.c, config/nios2/nios2.c, config/s390/s390.c,
> > 	config/sh/sh.c, reorg.c: Use rtx_code_label::nuses and
> > 	rtx_code_label::set_nuses where possible.
> > 	* rtl.h (rtx_code_label::nuses): New member function.
> > 	(rtx_code_label::set_nuses): Likewise.
> Is there some reason why we don't fix every reference to LABEL_NUSES to 
> use the new member functions?  My concern here is that we've now got two 
> ways actively used to get at that information, the old LABEL_NUSES and 
> the new member functions.  Obviously we really just want one blessed way 
> to get/set that information.  If this is a short term situation, then 
> I'd be inclined to say OK, but I don't see further patches which sort 
> out the ~200 or so LABEL_NUSES users.
> 
> THe approach David has taken for similar situations has been to first 
> introduce the get/set methods, convert everything to set the get/set 
> method, dealing with the type fallout that occurrs around that, then 
> collapsing back to the old macro.  Is there some reason we can not or 
> should not do that here?

If it's a macro that's simply a direct field access into a struct (e.g.
BB_HEADER), then yes, but for those that are wrappers around e.g. XEXP
I've been keeping them as inline functions, so that the type system
detects incorrect node types as compile-time errors.

One other aspect of my approach is that (believe it or not) I'm trying
to minimize the size of the changes, to avoid introducing pain when
backporting bugfixes from trunk to the branches.

My goal here is type-safety, with readability as a secondary benefit.  I
think it's a good idea for us to add methods that let us replace e.g.
XEXP (x, 0) accessors with descriptive names, and have been doing so,
and I prefer doing this as methods for new code.  However, when the
accessor already has a descriptive name, like LABEL_NUSES, I think it's
enough to convert them to inline functions and tighten up the params and
return type to express things.  I'm not sure the cost/benefit of
*additionally* converting them to be methods is worth it, given that it
means changing the spelling at every callsite.

Dave

Re: [PATCH 1/2] add staticly checked label_nuses accessors

[Trevor Saunders]

On Fri, Sep 05, 2014 at 05:57:13PM -0400, David Malcolm wrote:
> On Fri, 2014-09-05 at 10:43 -0600, Jeff Law wrote:
> > On 09/05/14 09:32, tsaunders@mozilla.com wrote:
> > > From: Trevor Saunders <tsaunders@mozilla.com>
> > >
> > > Hi,
> > >
> > >   Doing this means we get to remove a fair chunk of runtime checking.
> > >
> > > bootstrapped + regtested on x86_64-unknown-linux-gnu with no regressions.
> > > config-list.mk with this and the next patch is ongoing. ok?
> > >
> > > Trev
> > >
> > > gcc/
> > >
> > > 	* config/i386/i386.c, config/i386/i386.md, config/mep/mep.c,
> > > 	config/mips/mips.c, config/nios2/nios2.c, config/s390/s390.c,
> > > 	config/sh/sh.c, reorg.c: Use rtx_code_label::nuses and
> > > 	rtx_code_label::set_nuses where possible.
> > > 	* rtl.h (rtx_code_label::nuses): New member function.
> > > 	(rtx_code_label::set_nuses): Likewise.
> > Is there some reason why we don't fix every reference to LABEL_NUSES to 
> > use the new member functions?  My concern here is that we've now got two 
> > ways actively used to get at that information, the old LABEL_NUSES and 
> > the new member functions.  Obviously we really just want one blessed way 
> > to get/set that information.  If this is a short term situation, then 
> > I'd be inclined to say OK, but I don't see further patches which sort 
> > out the ~200 or so LABEL_NUSES users.
> > 
> > THe approach David has taken for similar situations has been to first 
> > introduce the get/set methods, convert everything to set the get/set 
> > method, dealing with the type fallout that occurrs around that, then 
> > collapsing back to the old macro.  Is there some reason we can not or 
> > should not do that here?
> 
> If it's a macro that's simply a direct field access into a struct (e.g.
> BB_HEADER), then yes, but for those that are wrappers around e.g. XEXP
> I've been keeping them as inline functions, so that the type system
> detects incorrect node types as compile-time errors.
> 
> One other aspect of my approach is that (believe it or not) I'm trying
> to minimize the size of the changes, to avoid introducing pain when
> backporting bugfixes from trunk to the branches.
> 
> My goal here is type-safety, with readability as a secondary benefit.  I
> think it's a good idea for us to add methods that let us replace e.g.
> XEXP (x, 0) accessors with descriptive names, and have been doing so,
> and I prefer doing this as methods for new code.  However, when the
> accessor already has a descriptive name, like LABEL_NUSES, I think it's
> enough to convert them to inline functions and tighten up the params and
> return type to express things.  I'm not sure the cost/benefit of
> *additionally* converting them to be methods is worth it, given that it
> means changing the spelling at every callsite.

So, in this case it seems to me you basically have two options

A. change the macro to an inline function, and fix up all the callers to
pass the right type.  Then rebase that into some sort of reasonable
patch series.

b. add a function with a different name and convert users piece meal,
and then remove the old macro (I guess you can then rename the new
function to the old macro if you like).

of those I prefer b because it means you can convert call sites one at a
time and its easy to know which have been delt with.

I also do think the advantages of using members outways the cost.

For one thing functions with all caps names are just weird.  I think the
more important reason though is that it will help make rtx_insn be a
separate class sometime in the far future, since at some point we can
make its inheritance from rtx_def protected to chase down things that
try to convert from rtx_insn to rtx.  There's also the argument that its
inconsistant to have some things be members and others be functions.

Trev

> 
> Dave
> 

Re: [PATCH 1/2] add staticly checked label_nuses accessors

[Richard Sandiford]

Trevor Saunders <tsaunders@mozilla.com> writes:
> I also do think the advantages of using members outways the cost.
>
> For one thing functions with all caps names are just weird.  I think the
> more important reason though is that it will help make rtx_insn be a
> separate class sometime in the far future, since at some point we can
> make its inheritance from rtx_def protected to chase down things that
> try to convert from rtx_insn to rtx.  There's also the argument that its
> inconsistant to have some things be members and others be functions.

+1 FWIW.  I think after conversion all rtx fields should be accessed
using the same style, rather than a mixture of caps-functions for some
fields and member functions for others.

Thanks,
Richard

Re: [PATCH 1/2] add staticly checked label_nuses accessors

[Jeff Law]

On 09/05/14 16:38, Trevor Saunders wrote:
>
> So, in this case it seems to me you basically have two options
>
> A. change the macro to an inline function, and fix up all the callers to
> pass the right type.  Then rebase that into some sort of reasonable
> patch series.
>
> b. add a function with a different name and convert users piece meal,
> and then remove the old macro (I guess you can then rename the new
> function to the old macro if you like).
>
> of those I prefer b because it means you can convert call sites one at a
> time and its easy to know which have been delt with.
My worry with piecemeal is obviously an incomplete transition.  While we 
have that to some extent with David's patches, we don't generally have 
two ways to get at the same hunk of data.




>
> I also do think the advantages of using members outways the cost.
I don't think it's always the case, but my general preference will be to 
go to member functions.


>
> For one thing functions with all caps names are just weird.  I think the
> more important reason though is that it will help make rtx_insn be a
> separate class sometime in the far future, since at some point we can
> make its inheritance from rtx_def protected to chase down things that
> try to convert from rtx_insn to rtx.  There's also the argument that its
> inconsistant to have some things be members and others be functions.
Yes, the all caps is weird.  There was a time when the all caps 
designated that we were using a macro and it was expected to be very 
efficient.  When calling a function we were supposed to use lowercase to 
signify the likely high overhead.

I'd tend to lean towards #1.  If we want to convert to methods, I think 
that can be a follow-up.  Type safety is my focus right now, not pushing 
towards member functions.

Jeff

Re: [PATCH 1/2] add staticly checked label_nuses accessors

[Jeff Law]

On 09/05/14 15:57, David Malcolm wrote:
> One other aspect of my approach is that (believe it or not) I'm trying
> to minimize the size of the changes, to avoid introducing pain when
> backporting bugfixes from trunk to the branches.
Right.  I believe and know you're trying to avoid unnecessary pain :-)

>
> My goal here is type-safety, with readability as a secondary benefit.
Agreed.  However, consistency with access is also important.  But, yes, 
we're really focused on moving on type safety here.



   I
> think it's a good idea for us to add methods that let us replace e.g.
> XEXP (x, 0) accessors with descriptive names, and have been doing so,
> and I prefer doing this as methods for new code.
As you undoubtly know, there's some resistance to that ;-)  But in cases 
where we can carve off things easily (like the list stuff) converting to 
members, at least IMHO, is much cleaner.  But I think we're a long way 
from converting RTL as a whole.



  However, when the
> accessor already has a descriptive name, like LABEL_NUSES, I think it's
> enough to convert them to inline functions and tighten up the params and
> return type to express things.  I'm not sure the cost/benefit of
> *additionally* converting them to be methods is worth it, given that it
> means changing the spelling at every callsite.
My feelings as well.

jeff

Re: [PATCH 1/2] add staticly checked label_nuses accessors

[Trevor Saunders]

On Thu, Sep 11, 2014 at 02:55:43PM -0600, Jeff Law wrote:
> On 09/05/14 16:38, Trevor Saunders wrote:
> >
> >So, in this case it seems to me you basically have two options
> >
> >A. change the macro to an inline function, and fix up all the callers to
> >pass the right type.  Then rebase that into some sort of reasonable
> >patch series.
> >
> >b. add a function with a different name and convert users piece meal,
> >and then remove the old macro (I guess you can then rename the new
> >function to the old macro if you like).
> >
> >of those I prefer b because it means you can convert call sites one at a
> >time and its easy to know which have been delt with.
> My worry with piecemeal is obviously an incomplete transition.  While we
> have that to some extent with David's patches, we don't generally have two
> ways to get at the same hunk of data.

yeah, I'd expect there's enough incentive for conversions to be
finished, but really I should probably know better ;)

fortunately I have a patch that just needs a ChangeLog finishing the
conversion of INSN_DELETED_P (though to a member function).  LABEL_NUSES
is harder in large part due to JUMP_LABEL, so that will probably have to
wait.

> >I also do think the advantages of using members outways the cost.
> I don't think it's always the case, but my general preference will be to go
> to member functions.
> 
> 
> >
> >For one thing functions with all caps names are just weird.  I think the
> >more important reason though is that it will help make rtx_insn be a
> >separate class sometime in the far future, since at some point we can
> >make its inheritance from rtx_def protected to chase down things that
> >try to convert from rtx_insn to rtx.  There's also the argument that its
> >inconsistant to have some things be members and others be functions.
> Yes, the all caps is weird.  There was a time when the all caps designated
> that we were using a macro and it was expected to be very efficient.  When
> calling a function we were supposed to use lowercase to signify the likely
> high overhead.

I actually care because if I suspect its a macro I'm going to grep for
'define XXX' and if its a function '^xxx', but I guess I should just
remember to use ctags :p

> I'd tend to lean towards #1.  If we want to convert to methods, I think that
> can be a follow-up.  Type safety is my focus right now, not pushing towards
> member functions.

yeah, if you don't want to change things incrementally I think there's
less incentive to change this at the same time.

Trev

> 
> Jeff