public inbox for gcc-patches@gcc.gnu.org
 help / color / mirror / Atom feed
* [PATCH 0/8] make next_*_insn take rtx_insn * arguments
@ 2016-09-14 19:13 tbsaunde+gcc
  2016-09-14 19:13 ` [PATCH 1/8] change a few rtx_insn * to rtx_jump_insn * tbsaunde+gcc
                   ` (8 more replies)
  0 siblings, 9 replies; 38+ messages in thread
From: tbsaunde+gcc @ 2016-09-14 19:13 UTC (permalink / raw)
  To: gcc-patches

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

Hi,

Basically $subject.  First change variable's type to rtx_insn * where possible.
Then change the functions and fixup callers where it is still necessary to
cast.

patches bootstrapped and regtested individually on x86_64-linux-gnu, and the
series was run through config-list.mk, ok?

Trev

Trevor Saunders (8):
  change a few rtx_insn * to rtx_jump_insn *
  use rtx_insn * more
  make next/prev _nonnote_insn take rtx_insn *
  make next/prev nondebug_insn take rtx_insn *
  make prev_real_insn take rtx_insn *
  make next/prev nonnote_nondebug_insn take rtx_insn *
  make next/prev active_insn and active_insn_p take rtx_insn *
  make next_cc0_user take rtx_insn *

 gcc/bb-reorder.c                |  5 ++--
 gcc/cfgcleanup.c                |  2 +-
 gcc/cfgloop.h                   |  2 +-
 gcc/config/arc/arc-protos.h     |  2 +-
 gcc/config/arc/arc.c            |  2 +-
 gcc/config/arc/arc.md           | 43 +++++++++++++++++++---------
 gcc/config/arm/arm.c            |  2 +-
 gcc/config/bfin/bfin.c          |  2 +-
 gcc/config/c6x/c6x.c            | 19 ++++++-------
 gcc/config/cris/cris-protos.h   |  4 +--
 gcc/config/cris/cris.c          | 14 ++++-----
 gcc/config/h8300/h8300-protos.h |  4 +--
 gcc/config/h8300/h8300.c        |  4 +--
 gcc/config/m32r/m32r.c          |  2 +-
 gcc/config/nds32/nds32-protos.h |  2 +-
 gcc/config/nds32/nds32.c        |  2 +-
 gcc/config/pa/pa.c              |  6 ++--
 gcc/config/rl78/rl78.c          | 11 +++----
 gcc/config/s390/s390-protos.h   |  2 +-
 gcc/config/s390/s390.c          |  2 +-
 gcc/config/sh/sh.c              | 10 +++----
 gcc/config/sh/sh.md             |  3 +-
 gcc/config/sparc/sparc-protos.h |  2 +-
 gcc/config/sparc/sparc.c        |  8 ++----
 gcc/cse.c                       |  2 +-
 gcc/dwarf2out.c                 |  2 +-
 gcc/emit-rtl.c                  | 41 +++++++--------------------
 gcc/jump.c                      | 18 ++++++------
 gcc/loop-doloop.c               |  2 +-
 gcc/reorg.c                     | 63 ++++++++++++++++++++++-------------------
 gcc/resource.c                  |  2 +-
 gcc/rtl.h                       | 26 ++++++++---------
 gcc/simplify-rtx.c              |  8 +++---
 33 files changed, 160 insertions(+), 159 deletions(-)

-- 
2.9.3

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

* [PATCH 1/8] change a few rtx_insn * to rtx_jump_insn *
  2016-09-14 19:13 [PATCH 0/8] make next_*_insn take rtx_insn * arguments tbsaunde+gcc
@ 2016-09-14 19:13 ` tbsaunde+gcc
  2016-09-15  4:33   ` Kaz Kojima
  2016-09-14 19:14 ` [PATCH 7/8] make next/prev active_insn and active_insn_p take rtx_insn * tbsaunde+gcc
                   ` (7 subsequent siblings)
  8 siblings, 1 reply; 38+ messages in thread
From: tbsaunde+gcc @ 2016-09-14 19:13 UTC (permalink / raw)
  To: gcc-patches

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

gcc/ChangeLog:

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

	* bb-reorder.c (fix_crossing_unconditional_branches): Make type
	of jump_insn rtx_jump_insn *.
	* reorg.c (steal_delay_list_from_target): Make type of insn
	rtx_jump_insn *.
	(follow_jumps): Make type of jump rtx_jump_insn *.
---
 gcc/bb-reorder.c |  5 ++---
 gcc/reorg.c      | 12 ++++++------
 2 files changed, 8 insertions(+), 9 deletions(-)

diff --git a/gcc/bb-reorder.c b/gcc/bb-reorder.c
index bb8435f..b26c041 100644
--- a/gcc/bb-reorder.c
+++ b/gcc/bb-reorder.c
@@ -2146,7 +2146,7 @@ fix_crossing_unconditional_branches (void)
   rtx label;
   rtx label_addr;
   rtx_insn *indirect_jump_sequence;
-  rtx_insn *jump_insn = NULL;
+  rtx_jump_insn *jump_insn = NULL;
   rtx new_reg;
   rtx_insn *cur_insn;
   edge succ;
@@ -2201,8 +2201,7 @@ fix_crossing_unconditional_branches (void)
 		{
 		  if (!BARRIER_P (cur_insn))
 		    BLOCK_FOR_INSN (cur_insn) = cur_bb;
-		  if (JUMP_P (cur_insn))
-		    jump_insn = cur_insn;
+		  jump_insn = dyn_cast<rtx_jump_insn *> (cur_insn);
 		}
 
 	      /* Insert the new (indirect) jump sequence immediately before
diff --git a/gcc/reorg.c b/gcc/reorg.c
index c58d608..d798c6a 100644
--- a/gcc/reorg.c
+++ b/gcc/reorg.c
@@ -205,7 +205,7 @@ static int redirect_with_delay_slots_safe_p (rtx_insn *, rtx, rtx);
 static int redirect_with_delay_list_safe_p (rtx_insn *, rtx,
 					    const vec<rtx_insn *> &);
 static int check_annul_list_true_false (int, const vec<rtx_insn *> &);
-static void steal_delay_list_from_target (rtx_insn *, rtx, rtx_sequence *,
+static void steal_delay_list_from_target (rtx_jump_insn *, rtx, rtx_sequence *,
 					  vec<rtx_insn *> *,
 					  struct resources *,
 					  struct resources *,
@@ -1033,10 +1033,10 @@ check_annul_list_true_false (int annul_true_p,
    execution should continue.  */
 
 static void
-steal_delay_list_from_target (rtx_insn *insn, rtx condition, rtx_sequence *seq,
-			      vec<rtx_insn *> *delay_list, resources *sets,
-			      struct resources *needed,
-			      struct resources *other_needed,
+steal_delay_list_from_target (rtx_jump_insn *insn, rtx condition,
+			      rtx_sequence *seq, vec<rtx_insn *> *delay_list,
+			      resources *sets, resources *needed,
+			      resources *other_needed,
 			      int slots_to_fill, int *pslots_filled,
 			      int *pannul_p, rtx *pnew_thread)
 {
@@ -2265,7 +2265,7 @@ fill_simple_delay_slots (int non_jumps_p)
    set *CROSSING to true, otherwise set it to false.  */
 
 static rtx
-follow_jumps (rtx label, rtx_insn *jump, bool *crossing)
+follow_jumps (rtx label, rtx_jump_insn *jump, bool *crossing)
 {
   rtx_insn *insn;
   rtx_insn *next;
-- 
2.9.3

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

* [PATCH 2/8] use rtx_insn * more
  2016-09-14 19:13 [PATCH 0/8] make next_*_insn take rtx_insn * arguments tbsaunde+gcc
                   ` (4 preceding siblings ...)
  2016-09-14 19:14 ` [PATCH 3/8] make next/prev _nonnote_insn " tbsaunde+gcc
@ 2016-09-14 19:14 ` tbsaunde+gcc
  2016-09-16 15:25   ` Alexander Monakov
  2016-09-14 19:19 ` [PATCH 6/8] make next/prev nonnote_nondebug_insn take rtx_insn * tbsaunde+gcc
                   ` (2 subsequent siblings)
  8 siblings, 1 reply; 38+ messages in thread
From: tbsaunde+gcc @ 2016-09-14 19:14 UTC (permalink / raw)
  To: gcc-patches

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

gcc/ChangeLog:

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

	* config/arc/arc-protos.h (arc_label_align): Change type of
	variables from rtx to rtx_insn *.
	* config/arc/arc.c (arc_label_align): Likewise.
	* config/arm/arm.c (any_sibcall_could_use_r3): Likewise.
	* config/bfin/bfin.c (workaround_speculation): Likewise.
	* config/c6x/c6x.c (find_next_cycle_insn): Likewise.
	(find_last_same_clock): Likewise.
	(reorg_split_calls): Likewise.
	* config/cris/cris-protos.h (cris_cc0_user_requires_cmp): Likewise.
	* config/cris/cris.c (cris_cc0_user_requires_cmp): Likewise.
	* config/h8300/h8300-protos.h (same_cmp_preceding_p): Likewise.
	(same_cmp_following_p): Likewise.
	* config/h8300/h8300.c (same_cmp_preceding_p): Likewise.
	(same_cmp_following_p): Likwise.
	* config/m32r/m32r.c (m32r_expand_epilogue): Likewise.
	* config/nds32/nds32-protos.h (nds32_target_alignment): Likewise.
	* config/nds32/nds32.c (nds32_target_alignment): Likewise.
	* config/rl78/rl78.c (rl78_alloc_physical_registers_op2):
	* Likewise.
	(rl78_alloc_physical_registers_cmp): Likewise.
	(rl78_alloc_physical_registers_umul): Likewise.
	(rl78_calculate_death_notes): Likewise.
	* config/s390/s390-protos.h (s390_label_align): Likewise.
	* config/s390/s390.c (s390_label_align): Likewise.
	* config/sh/sh.c (barrier_align): Likewise.
	* config/sparc/sparc-protos.h (emit_cbcond_nop): Likewise.
	* config/sparc/sparc.c (sparc_asm_function_epilogue): Likewise.
	(emit_cbcond_nop): Likewise.
---
 gcc/config/arc/arc-protos.h     |  2 +-
 gcc/config/arc/arc.c            |  2 +-
 gcc/config/arm/arm.c            |  2 +-
 gcc/config/bfin/bfin.c          |  2 +-
 gcc/config/c6x/c6x.c            | 19 +++++++++----------
 gcc/config/cris/cris-protos.h   |  2 +-
 gcc/config/cris/cris.c          |  2 +-
 gcc/config/h8300/h8300-protos.h |  4 ++--
 gcc/config/h8300/h8300.c        |  4 ++--
 gcc/config/m32r/m32r.c          |  2 +-
 gcc/config/nds32/nds32-protos.h |  2 +-
 gcc/config/nds32/nds32.c        |  2 +-
 gcc/config/rl78/rl78.c          | 11 ++++++-----
 gcc/config/s390/s390-protos.h   |  2 +-
 gcc/config/s390/s390.c          |  2 +-
 gcc/config/sh/sh.c              |  2 +-
 gcc/config/sparc/sparc-protos.h |  2 +-
 gcc/config/sparc/sparc.c        |  8 +++-----
 18 files changed, 35 insertions(+), 37 deletions(-)

diff --git a/gcc/config/arc/arc-protos.h b/gcc/config/arc/arc-protos.h
index 8630e2d..73aacbc 100644
--- a/gcc/config/arc/arc-protos.h
+++ b/gcc/config/arc/arc-protos.h
@@ -109,7 +109,7 @@ extern rtx arc_regno_use_in (unsigned int, rtx);
 extern int arc_attr_type (rtx_insn *);
 extern bool arc_scheduling_not_expected (void);
 extern bool arc_sets_cc_p (rtx_insn *insn);
-extern int arc_label_align (rtx label);
+extern int arc_label_align (rtx_insn *label);
 extern bool arc_need_delay (rtx_insn *insn);
 extern bool arc_text_label (rtx_insn *insn);
 
diff --git a/gcc/config/arc/arc.c b/gcc/config/arc/arc.c
index c0aa075..2b25b0b 100644
--- a/gcc/config/arc/arc.c
+++ b/gcc/config/arc/arc.c
@@ -9243,7 +9243,7 @@ arc_scheduling_not_expected (void)
    long.)  */
 
 int
-arc_label_align (rtx label)
+arc_label_align (rtx_insn *label)
 {
   int loop_align = LOOP_ALIGN (LABEL);
 
diff --git a/gcc/config/arm/arm.c b/gcc/config/arm/arm.c
index 43a832e..63d0067 100644
--- a/gcc/config/arm/arm.c
+++ b/gcc/config/arm/arm.c
@@ -20860,7 +20860,7 @@ any_sibcall_could_use_r3 (void)
   FOR_EACH_EDGE (e, ei, EXIT_BLOCK_PTR_FOR_FN (cfun)->preds)
     if (e->flags & EDGE_SIBCALL)
       {
-	rtx call = BB_END (e->src);
+	rtx_insn *call = BB_END (e->src);
 	if (!CALL_P (call))
 	  call = prev_nonnote_nondebug_insn (call);
 	gcc_assert (CALL_P (call) && SIBLING_CALL_P (call));
diff --git a/gcc/config/bfin/bfin.c b/gcc/config/bfin/bfin.c
index 086d548..4683a28 100644
--- a/gcc/config/bfin/bfin.c
+++ b/gcc/config/bfin/bfin.c
@@ -4452,7 +4452,7 @@ workaround_speculation (void)
 	      || cbranch_predicted_taken_p (insn)))
 	{
 	  rtx_insn *target = JUMP_LABEL_AS_INSN (insn);
-	  rtx label = target;
+	  rtx_insn *label = target;
 	  rtx_insn *next_tgt;
 
 	  cycles_since_jump = 0;
diff --git a/gcc/config/c6x/c6x.c b/gcc/config/c6x/c6x.c
index d759482..c250f8a 100644
--- a/gcc/config/c6x/c6x.c
+++ b/gcc/config/c6x/c6x.c
@@ -4807,10 +4807,10 @@ convert_to_callp (rtx_insn *insn)
 /* Scan forwards from INSN until we find the next insn that has mode TImode
    (indicating it starts a new cycle), and occurs in cycle CLOCK.
    Return it if we find such an insn, NULL_RTX otherwise.  */
-static rtx
-find_next_cycle_insn (rtx insn, int clock)
+static rtx_insn *
+find_next_cycle_insn (rtx_insn *insn, int clock)
 {
-  rtx t = insn;
+  rtx_insn *t = insn;
   if (GET_MODE (t) == TImode)
     t = next_real_insn (t);
   while (t && GET_MODE (t) != TImode)
@@ -4818,7 +4818,7 @@ find_next_cycle_insn (rtx insn, int clock)
 
   if (t && insn_get_clock (t) == clock)
     return t;
-  return NULL_RTX;
+  return NULL;
 }
 
 /* If COND_INSN has a COND_EXEC condition, wrap the same condition
@@ -4836,10 +4836,10 @@ duplicate_cond (rtx pat, rtx cond_insn)
 
 /* Walk forward from INSN to find the last insn that issues in the same clock
    cycle.  */
-static rtx
-find_last_same_clock (rtx insn)
+static rtx_insn *
+find_last_same_clock (rtx_insn *insn)
 {
-  rtx retval = insn;
+  rtx_insn *retval = insn;
   rtx_insn *t = next_real_insn (insn);
 
   while (t && GET_MODE (t) != TImode)
@@ -4935,12 +4935,11 @@ reorg_split_calls (rtx *call_labels)
 		 no insn setting/using B3 is scheduled in the delay slots of
 		 a call.  */
 	      int this_clock = insn_get_clock (insn);
-	      rtx last_same_clock;
-	      rtx after1;
+	      rtx_insn *after1;
 
 	      call_labels[INSN_UID (insn)] = label;
 
-	      last_same_clock = find_last_same_clock (insn);
+	      rtx_insn *last_same_clock = find_last_same_clock (insn);
 
 	      if (can_use_callp (insn))
 		{
diff --git a/gcc/config/cris/cris-protos.h b/gcc/config/cris/cris-protos.h
index 9068166..5e0ae61 100644
--- a/gcc/config/cris/cris-protos.h
+++ b/gcc/config/cris/cris-protos.h
@@ -27,7 +27,7 @@ extern void cris_notice_update_cc (rtx, rtx_insn *);
 extern bool cris_reload_address_legitimized (rtx, machine_mode, int, int, int);
 extern int cris_side_effect_mode_ok (enum rtx_code, rtx *, int, int,
                                      int, int, int);
-extern bool cris_cc0_user_requires_cmp (rtx);
+extern bool cris_cc0_user_requires_cmp (rtx_insn *);
 extern rtx cris_return_addr_rtx (int, rtx);
 extern rtx cris_split_movdx (rtx *);
 extern int cris_legitimate_pic_operand (rtx);
diff --git a/gcc/config/cris/cris.c b/gcc/config/cris/cris.c
index 971d07f..39118f0 100644
--- a/gcc/config/cris/cris.c
+++ b/gcc/config/cris/cris.c
@@ -2407,7 +2407,7 @@ cris_side_effect_mode_ok (enum rtx_code code, rtx *ops,
    insn for other reasons.  */
 
 bool
-cris_cc0_user_requires_cmp (rtx insn)
+cris_cc0_user_requires_cmp (rtx_insn *insn)
 {
   rtx_insn *cc0_user = NULL;
   rtx body;
diff --git a/gcc/config/h8300/h8300-protos.h b/gcc/config/h8300/h8300-protos.h
index 721b6c7..a4895ff 100644
--- a/gcc/config/h8300/h8300-protos.h
+++ b/gcc/config/h8300/h8300-protos.h
@@ -56,8 +56,8 @@ extern void split_adds_subs (machine_mode, rtx[]);
 extern int h8300_eightbit_constant_address_p (rtx);
 extern int h8300_tiny_constant_address_p (rtx);
 extern int byte_accesses_mergeable_p (rtx, rtx);
-extern int same_cmp_preceding_p (rtx);
-extern int same_cmp_following_p (rtx);
+extern int same_cmp_preceding_p (rtx_insn *);
+extern int same_cmp_following_p (rtx_insn *);
 
 /* Used in builtins.c */
 extern rtx h8300_return_addr_rtx (int, rtx);
diff --git a/gcc/config/h8300/h8300.c b/gcc/config/h8300/h8300.c
index 1818684..d22d822 100644
--- a/gcc/config/h8300/h8300.c
+++ b/gcc/config/h8300/h8300.c
@@ -5730,7 +5730,7 @@ byte_accesses_mergeable_p (rtx addr1, rtx addr2)
    before I3.  I3 is assumed to be a comparison insn.  */
 
 int
-same_cmp_preceding_p (rtx i3)
+same_cmp_preceding_p (rtx_insn *i3)
 {
   rtx_insn *i1, *i2;
 
@@ -5750,7 +5750,7 @@ same_cmp_preceding_p (rtx i3)
    after I1.  I1 is assumed to be a comparison insn.  */
 
 int
-same_cmp_following_p (rtx i1)
+same_cmp_following_p (rtx_insn *i1)
 {
   rtx_insn *i2, *i3;
 
diff --git a/gcc/config/m32r/m32r.c b/gcc/config/m32r/m32r.c
index b40c53c..7ef59c1 100644
--- a/gcc/config/m32r/m32r.c
+++ b/gcc/config/m32r/m32r.c
@@ -1786,7 +1786,7 @@ m32r_expand_epilogue (void)
 
   if (total_size == 0)
     {
-      rtx insn = get_last_insn ();
+      rtx_insn *insn = get_last_insn ();
 
       /* If the last insn was a BARRIER, we don't have to write any code
 	 because a jump (aka return) was put there.  */
diff --git a/gcc/config/nds32/nds32-protos.h b/gcc/config/nds32/nds32-protos.h
index d66749d..aef022b 100644
--- a/gcc/config/nds32/nds32-protos.h
+++ b/gcc/config/nds32/nds32-protos.h
@@ -126,7 +126,7 @@ extern int nds32_can_use_return_insn (void);
 
 /* Auxiliary functions to decide output alignment or not.  */
 
-extern int nds32_target_alignment (rtx);
+extern int nds32_target_alignment (rtx_insn *);
 
 /* Auxiliary functions to expand builtin functions.  */
 
diff --git a/gcc/config/nds32/nds32.c b/gcc/config/nds32/nds32.c
index c47c122..1c9ae88 100644
--- a/gcc/config/nds32/nds32.c
+++ b/gcc/config/nds32/nds32.c
@@ -3489,7 +3489,7 @@ nds32_adjust_insn_length (rtx_insn *insn, int length)
 
 /* Return align 2 (log base 2) if the next instruction of LABEL is 4 byte.  */
 int
-nds32_target_alignment (rtx label)
+nds32_target_alignment (rtx_insn *label)
 {
   rtx_insn *insn;
 
diff --git a/gcc/config/rl78/rl78.c b/gcc/config/rl78/rl78.c
index 3ddb198..c4ae901 100644
--- a/gcc/config/rl78/rl78.c
+++ b/gcc/config/rl78/rl78.c
@@ -3150,8 +3150,8 @@ has_constraint (unsigned int opnum, enum constraint_num constraint)
 static void
 rl78_alloc_physical_registers_op2 (rtx_insn * insn)
 {
-  rtx prev;
-  rtx first;
+  rtx_insn *prev;
+  rtx_insn *first;
   bool hl_used;
   int tmp_id;
   rtx saved_op1;
@@ -3329,7 +3329,7 @@ rl78_alloc_physical_registers_cmp (rtx_insn * insn)
   int tmp_id;
   rtx saved_op1;
   rtx_insn *prev = prev_nonnote_nondebug_insn (insn);
-  rtx first;
+  rtx_insn *first;
 
   OP (1) = transcode_memory_rtx (OP (1), DE, insn);
   OP (2) = transcode_memory_rtx (OP (2), HL, insn);
@@ -3420,7 +3420,7 @@ static void
 rl78_alloc_physical_registers_umul (rtx_insn * insn)
 {
   rtx_insn *prev = prev_nonnote_nondebug_insn (insn);
-  rtx first;
+  rtx_insn *first;
   int tmp_id;
   rtx saved_op1;
 
@@ -3790,7 +3790,8 @@ static void
 rl78_calculate_death_notes (void)
 {
   char dead[FIRST_PSEUDO_REGISTER];
-  rtx insn, p, s, d;
+  rtx p, s, d;
+rtx_insn *insn;
   int i;
 
   memset (dead, 0, sizeof (dead));
diff --git a/gcc/config/s390/s390-protos.h b/gcc/config/s390/s390-protos.h
index 2ccf0bb..91dc0ff 100644
--- a/gcc/config/s390/s390-protos.h
+++ b/gcc/config/s390/s390-protos.h
@@ -133,7 +133,7 @@ extern void s390_split_access_reg (rtx, rtx *, rtx *);
 extern void print_operand_address (FILE *, rtx);
 extern void print_operand (FILE *, rtx, int);
 extern void s390_output_pool_entry (rtx, machine_mode, unsigned int);
-extern int s390_label_align (rtx);
+extern int s390_label_align (rtx_insn *);
 extern int s390_agen_dep_p (rtx_insn *, rtx_insn *);
 extern rtx_insn *s390_load_got (void);
 extern rtx s390_get_thread_pointer (void);
diff --git a/gcc/config/s390/s390.c b/gcc/config/s390/s390.c
index 0a457fc..60e2070 100644
--- a/gcc/config/s390/s390.c
+++ b/gcc/config/s390/s390.c
@@ -1134,7 +1134,7 @@ static const struct attribute_spec s390_attribute_table[] = {
 /* Return the alignment for LABEL.  We default to the -falign-labels
    value except for the literal pool base label.  */
 int
-s390_label_align (rtx label)
+s390_label_align (rtx_insn *label)
 {
   rtx_insn *prev_insn = prev_active_insn (label);
   rtx set, src;
diff --git a/gcc/config/sh/sh.c b/gcc/config/sh/sh.c
index 10b5a28..b3e949e 100644
--- a/gcc/config/sh/sh.c
+++ b/gcc/config/sh/sh.c
@@ -5694,7 +5694,7 @@ barrier_align (rtx_insn *barrier_or_label)
 	      ? 1 : align_jumps_log);
     }
 
-  rtx next = next_active_insn (barrier_or_label);
+  rtx_insn *next = next_active_insn (barrier_or_label);
 
   if (! next)
     return 0;
diff --git a/gcc/config/sparc/sparc-protos.h b/gcc/config/sparc/sparc-protos.h
index 4a93cc2..2308b91 100644
--- a/gcc/config/sparc/sparc-protos.h
+++ b/gcc/config/sparc/sparc-protos.h
@@ -84,7 +84,7 @@ extern int registers_ok_for_ldd_peep (rtx, rtx);
 extern int mems_ok_for_ldd_peep (rtx, rtx, rtx);
 extern rtx widen_mem_for_ldd_peep (rtx, rtx, machine_mode);
 extern int empty_delay_slot (rtx_insn *);
-extern int emit_cbcond_nop (rtx);
+extern int emit_cbcond_nop (rtx_insn *);
 extern int eligible_for_call_delay (rtx_insn *);
 extern int eligible_for_return_delay (rtx_insn *);
 extern int eligible_for_sibcall_delay (rtx_insn *);
diff --git a/gcc/config/sparc/sparc.c b/gcc/config/sparc/sparc.c
index 254b722..ec7513e 100644
--- a/gcc/config/sparc/sparc.c
+++ b/gcc/config/sparc/sparc.c
@@ -3485,7 +3485,7 @@ empty_delay_slot (rtx_insn *insn)
    situation.  */
 
 int
-emit_cbcond_nop (rtx insn)
+emit_cbcond_nop (rtx_insn *insn)
 {
   rtx next = next_active_insn (insn);
 
@@ -5870,11 +5870,9 @@ sparc_asm_function_epilogue (FILE *file, HOST_WIDE_INT size ATTRIBUTE_UNUSED)
      backtraces in such cases.  This is pointless for sibling calls since
      the return address is explicitly adjusted.  */
 
-  rtx insn, last_real_insn;
+  rtx_insn *insn = get_last_insn ();
 
-  insn = get_last_insn ();
-
-  last_real_insn = prev_real_insn (insn);
+  rtx last_real_insn = prev_real_insn (insn);
   if (last_real_insn
       && NONJUMP_INSN_P (last_real_insn)
       && GET_CODE (PATTERN (last_real_insn)) == SEQUENCE)
-- 
2.9.3

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

* [PATCH 7/8] make next/prev active_insn and active_insn_p take rtx_insn *
  2016-09-14 19:13 [PATCH 0/8] make next_*_insn take rtx_insn * arguments tbsaunde+gcc
  2016-09-14 19:13 ` [PATCH 1/8] change a few rtx_insn * to rtx_jump_insn * tbsaunde+gcc
@ 2016-09-14 19:14 ` tbsaunde+gcc
  2016-09-19 21:26   ` Jeff Law
  2016-09-14 19:14 ` [PATCH 4/8] make next/prev nondebug_insn " tbsaunde+gcc
                   ` (6 subsequent siblings)
  8 siblings, 1 reply; 38+ messages in thread
From: tbsaunde+gcc @ 2016-09-14 19:14 UTC (permalink / raw)
  To: gcc-patches

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

gcc/ChangeLog:

2016-09-14  Trevor Saunders  <tbsaunde+gcc@tbsaunde.org>

	* emit-rtl.c (next_active_insn): Change argument type to
	rtx_insn *.
	(prev_active_insn): Likewise.
	(active_insn_p): Likewise.
	* rtl.h: Adjust prototypes.
	* cfgcleanup.c (merge_blocks_move_successor_nojumps): Adjust.
	* config/arc/arc.md: Likewise.
	* config/pa/pa.c (branch_to_delay_slot_p): Likewise.
	(branch_needs_nop_p): Likewise.
	(use_skip_p): Likewise.
	* config/sh/sh.c (gen_block_redirect): Likewise.
	(split_branches): Likewise.
	* reorg.c (optimize_skip): Likewise.
	(fill_simple_delay_slots): Likewise.
	(fill_slots_from_thread): Likewise.
	(relax_delay_slots): Likewise.
	* resource.c (mark_target_live_regs): Likewise.
---
 gcc/cfgcleanup.c      |  2 +-
 gcc/config/arc/arc.md | 33 +++++++++++++++++++++++----------
 gcc/config/pa/pa.c    |  6 +++---
 gcc/config/sh/sh.c    |  8 ++++----
 gcc/emit-rtl.c        | 10 +++-------
 gcc/reorg.c           | 29 +++++++++++++++++------------
 gcc/resource.c        |  2 +-
 gcc/rtl.h             |  6 +++---
 8 files changed, 55 insertions(+), 41 deletions(-)

diff --git a/gcc/cfgcleanup.c b/gcc/cfgcleanup.c
index 023b9d2..2e2a635 100644
--- a/gcc/cfgcleanup.c
+++ b/gcc/cfgcleanup.c
@@ -708,7 +708,7 @@ merge_blocks_move_successor_nojumps (basic_block a, basic_block b)
   /* If there is a jump table following block B temporarily add the jump table
      to block B so that it will also be moved to the correct location.  */
   if (tablejump_p (BB_END (b), &label, &table)
-      && prev_active_insn (label) == BB_END (b))
+      && prev_active_insn (as_a<rtx_insn *> (label)) == BB_END (b))
     {
       BB_END (b) = table;
     }
diff --git a/gcc/config/arc/arc.md b/gcc/config/arc/arc.md
index 22fdbba..ac7346b 100644
--- a/gcc/config/arc/arc.md
+++ b/gcc/config/arc/arc.md
@@ -5122,16 +5122,29 @@
 		  scan = as_a <rtx_insn *> (XEXP (SET_SRC (PATTERN (scan)), 0));
 		  continue;
 		}
-	      if (JUMP_LABEL (scan)
-		  /* JUMP_LABEL might be simple_return instead if an insn.  */
-		  && (!INSN_P (JUMP_LABEL (scan))
-		      || (!next_active_insn (JUMP_LABEL (scan))
-			  || (recog_memoized (next_active_insn (JUMP_LABEL (scan)))
-			      != CODE_FOR_doloop_begin_i)))
-		  && (!next_active_insn (NEXT_INSN (PREV_INSN (scan)))
-		      || (recog_memoized
-			   (next_active_insn (NEXT_INSN (PREV_INSN (scan))))
-			  != CODE_FOR_doloop_begin_i)))
+
+	      rtx lab = JUMP_LABEL (scan);
+	      if (!lab)
+		break;
+
+	      rtx_insn *next_scan
+		= next_active_insn (NEXT_INSN (PREV_INSN (scan)));
+	      if (next_scan
+		  && recog_memoized (next_scan) != CODE_FOR_doloop_begin_i)
+		break;
+
+	      /* JUMP_LABEL might be simple_return instead if an insn.  */
+	      if (!INSN_P (lab))
+		{
+		  n_insns++;
+		  break;
+		}
+
+	      rtx_insn *next_lab = next_active_insn (as_a<rtx_insn *> (lab));
+	      if (next_lab
+		  && recog_memoized (next_lab) != CODE_FOR_doloop_begin_i)
+		break;
+
 		n_insns++;
 	    }
 	  break;
diff --git a/gcc/config/pa/pa.c b/gcc/config/pa/pa.c
index 251c1ad..9c15504 100644
--- a/gcc/config/pa/pa.c
+++ b/gcc/config/pa/pa.c
@@ -6442,7 +6442,7 @@ branch_to_delay_slot_p (rtx_insn *insn)
   if (dbr_sequence_length ())
     return FALSE;
 
-  jump_insn = next_active_insn (JUMP_LABEL (insn));
+  jump_insn = next_active_insn (JUMP_LABEL_AS_INSN (insn));
   while (insn)
     {
       insn = next_active_insn (insn);
@@ -6476,7 +6476,7 @@ branch_needs_nop_p (rtx_insn *insn)
   if (dbr_sequence_length ())
     return FALSE;
 
-  jump_insn = next_active_insn (JUMP_LABEL (insn));
+  jump_insn = next_active_insn (JUMP_LABEL_AS_INSN (insn));
   while (insn)
     {
       insn = next_active_insn (insn);
@@ -6499,7 +6499,7 @@ branch_needs_nop_p (rtx_insn *insn)
 static bool
 use_skip_p (rtx_insn *insn)
 {
-  rtx_insn *jump_insn = next_active_insn (JUMP_LABEL (insn));
+  rtx_insn *jump_insn = next_active_insn (JUMP_LABEL_AS_INSN (insn));
 
   while (insn)
     {
diff --git a/gcc/config/sh/sh.c b/gcc/config/sh/sh.c
index b3e949e..a9b5a14 100644
--- a/gcc/config/sh/sh.c
+++ b/gcc/config/sh/sh.c
@@ -5503,7 +5503,8 @@ gen_block_redirect (rtx_insn *jump, int addr, int need_block)
 
   else if (optimize && need_block >= 0)
     {
-      rtx_insn *next = next_active_insn (next_active_insn (dest));
+      rtx_insn *next = next_active_insn (as_a<rtx_insn *> (dest));
+      next = next_active_insn (next);
       if (next && JUMP_P (next)
 	  && GET_CODE (PATTERN (next)) == SET
 	  && recog_memoized (next) == CODE_FOR_jump_compact)
@@ -6395,9 +6396,8 @@ split_branches (rtx_insn *first)
 		/* We can't use JUMP_LABEL here because it might be undefined
 		   when not optimizing.  */
 		/* A syntax error might cause beyond to be NULL_RTX.  */
-		beyond
-		  = next_active_insn (XEXP (XEXP (SET_SRC (PATTERN (insn)), 1),
-					    0));
+		rtx temp = XEXP (XEXP (SET_SRC (PATTERN (insn)), 1), 0);
+		beyond = next_active_insn (as_a<rtx_insn *> (temp));
 
 		if (beyond
 		    && (JUMP_P (beyond)
diff --git a/gcc/emit-rtl.c b/gcc/emit-rtl.c
index 20d8250..e794613 100644
--- a/gcc/emit-rtl.c
+++ b/gcc/emit-rtl.c
@@ -3490,7 +3490,7 @@ last_call_insn (void)
    standalone USE and CLOBBER insn.  */
 
 int
-active_insn_p (const_rtx insn)
+active_insn_p (const rtx_insn *insn)
 {
   return (CALL_P (insn) || JUMP_P (insn)
 	  || JUMP_TABLE_DATA_P (insn) /* FIXME */
@@ -3501,10 +3501,8 @@ active_insn_p (const_rtx insn)
 }
 
 rtx_insn *
-next_active_insn (rtx uncast_insn)
+next_active_insn (rtx_insn *insn)
 {
-  rtx_insn *insn = safe_as_a <rtx_insn *> (uncast_insn);
-
   while (insn)
     {
       insn = NEXT_INSN (insn);
@@ -3520,10 +3518,8 @@ next_active_insn (rtx uncast_insn)
    standalone USE and CLOBBER insn.  */
 
 rtx_insn *
-prev_active_insn (rtx uncast_insn)
+prev_active_insn (rtx_insn *insn)
 {
-  rtx_insn *insn = safe_as_a <rtx_insn *> (uncast_insn);
-
   while (insn)
     {
       insn = PREV_INSN (insn);
diff --git a/gcc/reorg.c b/gcc/reorg.c
index 44cbf9f..d15cdbd 100644
--- a/gcc/reorg.c
+++ b/gcc/reorg.c
@@ -749,7 +749,7 @@ optimize_skip (rtx_jump_insn *insn, vec<rtx_insn *> *delay_list)
      we have one insn followed by a branch to the same label we branch to.
      In both of these cases, inverting the jump and annulling the delay
      slot give the same effect in fewer insns.  */
-  if (next_trial == next_active_insn (JUMP_LABEL (insn))
+  if (next_trial == next_active_insn (JUMP_LABEL_AS_INSN (insn))
       || (next_trial != 0
 	  && simplejump_or_return_p (next_trial)
 	  && JUMP_LABEL (insn) == JUMP_LABEL (next_trial)))
@@ -2198,7 +2198,7 @@ fill_simple_delay_slots (int non_jumps_p)
 	      && trial
 	      && jump_to_label_p (trial)
 	      && simplejump_p (trial)
-	      && (next_trial = next_active_insn (JUMP_LABEL (trial))) != 0
+	      && (next_trial = next_active_insn (JUMP_LABEL_AS_INSN (trial))) != 0
 	      && ! (NONJUMP_INSN_P (next_trial)
 		    && GET_CODE (PATTERN (next_trial)) == SEQUENCE)
 	      && !JUMP_P (next_trial)
@@ -2238,8 +2238,8 @@ fill_simple_delay_slots (int non_jumps_p)
 	  && simplejump_p (jump_insn)
 	  && slots_filled != slots_to_fill)
 	fill_slots_from_thread (jump_insn, const_true_rtx,
-				next_active_insn (JUMP_LABEL (insn)), NULL, 1,
-				1, own_thread_p (JUMP_LABEL (insn),
+				next_active_insn (JUMP_LABEL_AS_INSN (insn)),
+				NULL, 1, 1, own_thread_p (JUMP_LABEL (insn),
 						 JUMP_LABEL (insn), 0),
 				slots_to_fill, &slots_filled, &delay_list);
 
@@ -2575,7 +2575,8 @@ fill_slots_from_thread (rtx_jump_insn *insn, rtx condition,
 			     to call update_block and delete_insn.  */
 			  fix_reg_dead_note (prior_insn, insn);
 			  update_reg_unused_notes (prior_insn, new_thread);
-			  new_thread = next_active_insn (new_thread);
+			  new_thread
+			    = next_active_insn (as_a<rtx_insn *> (new_thread));
 			}
 		      break;
 		    }
@@ -3079,7 +3080,7 @@ delete_jump (rtx_insn *insn)
 }
 
 static rtx_insn *
-label_before_next_insn (rtx x, rtx scan_limit)
+label_before_next_insn (rtx_insn *x, rtx scan_limit)
 {
   rtx_insn *insn = next_active_insn (x);
   while (insn)
@@ -3142,7 +3143,8 @@ relax_delay_slots (rtx_insn *first)
 	  if (ANY_RETURN_P (target_label))
 	    target_label = find_end_label (target_label);
 
-	  if (target_label && next_active_insn (target_label) == next
+	  if (target_label
+	      && next_active_insn (as_a<rtx_insn *> (target_label)) == next
 	      && ! condjump_in_parallel_p (jump_insn)
 	      && ! (next && switch_text_sections_between_p (jump_insn, next)))
 	    {
@@ -3163,7 +3165,8 @@ relax_delay_slots (rtx_insn *first)
 	  if (next && simplejump_or_return_p (next)
 	      && any_condjump_p (jump_insn)
 	      && target_label
-	      && next_active_insn (target_label) == next_active_insn (next)
+	      && (next_active_insn (as_a<rtx_insn *> (target_label))
+		  == next_active_insn (next))
 	      && no_labels_between_p (jump_insn, next)
 	      && targetm.can_follow_jump (jump_insn, next))
 	    {
@@ -3318,7 +3321,7 @@ relax_delay_slots (rtx_insn *first)
 	{
 	  /* Figure out where to emit the special USE insn so we don't
 	     later incorrectly compute register live/death info.  */
-	  rtx_insn *tmp = next_active_insn (trial);
+	  rtx_insn *tmp = next_active_insn (as_a<rtx_insn *> (trial));
 	  if (tmp == 0)
 	    tmp = find_end_label (simple_return_rtx);
 
@@ -3366,7 +3369,7 @@ relax_delay_slots (rtx_insn *first)
       /* See if we have a simple (conditional) jump that is useless.  */
       if (! INSN_ANNULLED_BRANCH_P (delay_jump_insn)
 	  && ! condjump_in_parallel_p (delay_jump_insn)
-	  && prev_active_insn (target_label) == insn
+	  && prev_active_insn (as_a<rtx_insn *> (target_label)) == insn
 	  && ! BARRIER_P (prev_nonnote_insn (as_a<rtx_insn *> (target_label)))
 	  /* If the last insn in the delay slot sets CC0 for some insn,
 	     various code assumes that it is in a delay slot.  We could
@@ -3429,7 +3432,8 @@ relax_delay_slots (rtx_insn *first)
       if (! INSN_ANNULLED_BRANCH_P (delay_jump_insn)
 	  && any_condjump_p (delay_jump_insn)
 	  && next && simplejump_or_return_p (next)
-	  && next_active_insn (target_label) == next_active_insn (next)
+	  && (next_active_insn (as_a<rtx_insn *> (target_label))
+	      == next_active_insn (next))
 	  && no_labels_between_p (insn, next))
 	{
 	  rtx label = JUMP_LABEL (next);
@@ -3480,7 +3484,8 @@ relax_delay_slots (rtx_insn *first)
 	try_merge_delay_insns (insn, next);
       else if (! INSN_FROM_TARGET_P (pat->insn (1))
 	       && own_thread_p (target_label, target_label, 0))
-	try_merge_delay_insns (insn, next_active_insn (target_label));
+	try_merge_delay_insns (insn,
+			       next_active_insn (as_a<rtx_insn *> (target_label)));
 
       /* If we get here, we haven't deleted INSN.  But we may have deleted
 	 NEXT, so recompute it.  */
diff --git a/gcc/resource.c b/gcc/resource.c
index ae2f5d8..1d7ce95 100644
--- a/gcc/resource.c
+++ b/gcc/resource.c
@@ -1122,7 +1122,7 @@ mark_target_live_regs (rtx_insn *insns, rtx target_maybe_return, struct resource
       rtx_insn *stop_insn = next_active_insn (jump_insn);
 
       if (!ANY_RETURN_P (jump_target))
-	jump_target = next_active_insn (jump_target);
+	jump_target = next_active_insn (as_a<rtx_insn *> (jump_target));
       mark_target_live_regs (insns, jump_target, &new_resources);
       CLEAR_RESOURCE (&set);
       CLEAR_RESOURCE (&needed);
diff --git a/gcc/rtl.h b/gcc/rtl.h
index 4ae9a25..22ee2e6 100644
--- a/gcc/rtl.h
+++ b/gcc/rtl.h
@@ -2844,9 +2844,9 @@ extern rtx_insn *prev_nonnote_nondebug_insn (rtx_insn *);
 extern rtx_insn *next_nonnote_nondebug_insn (rtx_insn *);
 extern rtx_insn *prev_real_insn (rtx_insn *);
 extern rtx_insn *next_real_insn (rtx);
-extern rtx_insn *prev_active_insn (rtx);
-extern rtx_insn *next_active_insn (rtx);
-extern int active_insn_p (const_rtx);
+extern rtx_insn *prev_active_insn (rtx_insn *);
+extern rtx_insn *next_active_insn (rtx_insn *);
+extern int active_insn_p (const rtx_insn *);
 extern rtx_insn *next_cc0_user (rtx);
 extern rtx_insn *prev_cc0_setter (rtx_insn *);
 
-- 
2.9.3

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

* [PATCH 4/8] make next/prev nondebug_insn take rtx_insn *
  2016-09-14 19:13 [PATCH 0/8] make next_*_insn take rtx_insn * arguments tbsaunde+gcc
  2016-09-14 19:13 ` [PATCH 1/8] change a few rtx_insn * to rtx_jump_insn * tbsaunde+gcc
  2016-09-14 19:14 ` [PATCH 7/8] make next/prev active_insn and active_insn_p take rtx_insn * tbsaunde+gcc
@ 2016-09-14 19:14 ` tbsaunde+gcc
  2016-09-14 19:14 ` [PATCH 8/8] make next_cc0_user " tbsaunde+gcc
                   ` (5 subsequent siblings)
  8 siblings, 0 replies; 38+ messages in thread
From: tbsaunde+gcc @ 2016-09-14 19:14 UTC (permalink / raw)
  To: gcc-patches

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

gcc/ChangeLog:

2016-09-13  Trevor Saunders  <tbsaunde+gcc@tbsaunde.org>

	* emit-rtl.c (next_nondebug_insn): Change argument type to
	rtx_insn *.
	(prev_nondebug_insn): Likewise.
	* loop-doloop.c (doloop_condition_get): Likewise.
	* rtl.h: Adjust prototype.
	* cfgloop.h: Likewise.
---
 gcc/cfgloop.h     | 2 +-
 gcc/emit-rtl.c    | 8 ++------
 gcc/loop-doloop.c | 2 +-
 gcc/rtl.h         | 4 ++--
 4 files changed, 6 insertions(+), 10 deletions(-)

diff --git a/gcc/cfgloop.h b/gcc/cfgloop.h
index 5c202eb..0448a61 100644
--- a/gcc/cfgloop.h
+++ b/gcc/cfgloop.h
@@ -371,7 +371,7 @@ extern bool just_once_each_iteration_p (const struct loop *, const_basic_block);
 gcov_type expected_loop_iterations_unbounded (const struct loop *,
 					      bool *read_profile_p = NULL);
 extern unsigned expected_loop_iterations (struct loop *);
-extern rtx doloop_condition_get (rtx);
+extern rtx doloop_condition_get (rtx_insn *);
 
 void mark_loop_for_removal (loop_p);
 
diff --git a/gcc/emit-rtl.c b/gcc/emit-rtl.c
index cbea214..6051326 100644
--- a/gcc/emit-rtl.c
+++ b/gcc/emit-rtl.c
@@ -3373,10 +3373,8 @@ prev_nonnote_insn_bb (rtx uncast_insn)
    routine does not look inside SEQUENCEs.  */
 
 rtx_insn *
-next_nondebug_insn (rtx uncast_insn)
+next_nondebug_insn (rtx_insn *insn)
 {
-  rtx_insn *insn = safe_as_a <rtx_insn *> (uncast_insn);
-
   while (insn)
     {
       insn = NEXT_INSN (insn);
@@ -3391,10 +3389,8 @@ next_nondebug_insn (rtx uncast_insn)
    This routine does not look inside SEQUENCEs.  */
 
 rtx_insn *
-prev_nondebug_insn (rtx uncast_insn)
+prev_nondebug_insn (rtx_insn *insn)
 {
-  rtx_insn *insn = safe_as_a <rtx_insn *> (uncast_insn);
-
   while (insn)
     {
       insn = PREV_INSN (insn);
diff --git a/gcc/loop-doloop.c b/gcc/loop-doloop.c
index c311516..17a968f 100644
--- a/gcc/loop-doloop.c
+++ b/gcc/loop-doloop.c
@@ -70,7 +70,7 @@ along with GCC; see the file COPYING3.  If not see
    if it is not a decrement and branch jump insn.  */
 
 rtx
-doloop_condition_get (rtx doloop_pat)
+doloop_condition_get (rtx_insn *doloop_pat)
 {
   rtx cmp;
   rtx inc;
diff --git a/gcc/rtl.h b/gcc/rtl.h
index 0d121bc..b557ffe 100644
--- a/gcc/rtl.h
+++ b/gcc/rtl.h
@@ -2838,8 +2838,8 @@ extern rtx_insn *prev_nonnote_insn (rtx_insn *);
 extern rtx_insn *prev_nonnote_insn_bb (rtx);
 extern rtx_insn *next_nonnote_insn (rtx_insn *);
 extern rtx_insn *next_nonnote_insn_bb (rtx_insn *);
-extern rtx_insn *prev_nondebug_insn (rtx);
-extern rtx_insn *next_nondebug_insn (rtx);
+extern rtx_insn *prev_nondebug_insn (rtx_insn *);
+extern rtx_insn *next_nondebug_insn (rtx_insn *);
 extern rtx_insn *prev_nonnote_nondebug_insn (rtx);
 extern rtx_insn *next_nonnote_nondebug_insn (rtx);
 extern rtx_insn *prev_real_insn (rtx);
-- 
2.9.3

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

* [PATCH 8/8] make next_cc0_user take rtx_insn *
  2016-09-14 19:13 [PATCH 0/8] make next_*_insn take rtx_insn * arguments tbsaunde+gcc
                   ` (2 preceding siblings ...)
  2016-09-14 19:14 ` [PATCH 4/8] make next/prev nondebug_insn " tbsaunde+gcc
@ 2016-09-14 19:14 ` tbsaunde+gcc
  2016-09-14 19:14 ` [PATCH 3/8] make next/prev _nonnote_insn " tbsaunde+gcc
                   ` (4 subsequent siblings)
  8 siblings, 0 replies; 38+ messages in thread
From: tbsaunde+gcc @ 2016-09-14 19:14 UTC (permalink / raw)
  To: gcc-patches

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

gcc/ChangeLog:

2016-09-14  Trevor Saunders  <tbsaunde+gcc@tbsaunde.org>

	* emit-rtl.c (next_cc0_user): Make argument type rtx_insn *.
	* rtl.h: Adjust prototype.
---
 gcc/emit-rtl.c | 4 +---
 gcc/rtl.h      | 2 +-
 2 files changed, 2 insertions(+), 4 deletions(-)

diff --git a/gcc/emit-rtl.c b/gcc/emit-rtl.c
index e794613..1519aa5 100644
--- a/gcc/emit-rtl.c
+++ b/gcc/emit-rtl.c
@@ -3540,10 +3540,8 @@ prev_active_insn (rtx_insn *insn)
    Return 0 if we can't find the insn.  */
 
 rtx_insn *
-next_cc0_user (rtx uncast_insn)
+next_cc0_user (rtx_insn *insn)
 {
-  rtx_insn *insn = safe_as_a <rtx_insn *> (uncast_insn);
-
   rtx note = find_reg_note (insn, REG_CC_USER, NULL_RTX);
 
   if (note)
diff --git a/gcc/rtl.h b/gcc/rtl.h
index 22ee2e6..ce1131b 100644
--- a/gcc/rtl.h
+++ b/gcc/rtl.h
@@ -2847,7 +2847,7 @@ extern rtx_insn *next_real_insn (rtx);
 extern rtx_insn *prev_active_insn (rtx_insn *);
 extern rtx_insn *next_active_insn (rtx_insn *);
 extern int active_insn_p (const rtx_insn *);
-extern rtx_insn *next_cc0_user (rtx);
+extern rtx_insn *next_cc0_user (rtx_insn *);
 extern rtx_insn *prev_cc0_setter (rtx_insn *);
 
 /* In emit-rtl.c  */
-- 
2.9.3

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

* [PATCH 3/8] make next/prev _nonnote_insn take rtx_insn *
  2016-09-14 19:13 [PATCH 0/8] make next_*_insn take rtx_insn * arguments tbsaunde+gcc
                   ` (3 preceding siblings ...)
  2016-09-14 19:14 ` [PATCH 8/8] make next_cc0_user " tbsaunde+gcc
@ 2016-09-14 19:14 ` tbsaunde+gcc
  2016-09-19 21:27   ` Jeff Law
  2016-09-14 19:14 ` [PATCH 2/8] use rtx_insn * more tbsaunde+gcc
                   ` (3 subsequent siblings)
  8 siblings, 1 reply; 38+ messages in thread
From: tbsaunde+gcc @ 2016-09-14 19:14 UTC (permalink / raw)
  To: gcc-patches

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

gcc/ChangeLog:

2016-09-13  Trevor Saunders  <tbsaunde+gcc@tbsaunde.org>

	* emit-rtl.c (next_nonnote_insn): Change argument type to
	rtx_insn *.
	(prev_nonnote_insn): Likewise.
	* jump.c (reversed_comparison_code_parts): Likewise.
	(reversed_comparison): Likewise.
	* rtl.h: Adjust prototypes.
	* config/arc/arc.md: Adjust.
	* cse.c (find_comparison_args): Likewise.
	* reorg.c (redundant_insn): Change return type to rtx_insn *.
	(fix_reg_dead_note): Change argument type to rtx_insn *.
	(delete_prior_computation): Likewise.
	(delete_computation): Likewise.
	(fill_slots_from_thread): Adjust.
	(relax_delay_slots): Likewise.
	* simplify-rtx.c (simplify_unary_operation_1): Likewise.
	(simplify_relational_operation_1): Likewise.
	(simplify_ternary_operation): Likewise.
---
 gcc/config/arc/arc.md | 10 ++++++----
 gcc/cse.c             |  2 +-
 gcc/emit-rtl.c        |  7 ++-----
 gcc/jump.c            | 12 ++++++------
 gcc/reorg.c           | 22 +++++++++++-----------
 gcc/rtl.h             |  8 ++++----
 gcc/simplify-rtx.c    |  8 ++++----
 7 files changed, 34 insertions(+), 35 deletions(-)

diff --git a/gcc/config/arc/arc.md b/gcc/config/arc/arc.md
index 1102c53..22fdbba 100644
--- a/gcc/config/arc/arc.md
+++ b/gcc/config/arc/arc.md
@@ -3879,7 +3879,7 @@
   ""
   "*
 {
-  rtx diff_vec = PATTERN (next_nonnote_insn (operands[3]));
+  rtx diff_vec = PATTERN (next_nonnote_insn (as_a<rtx_insn *> (operands[3])));
 
   if (GET_CODE (diff_vec) != ADDR_DIFF_VEC)
     {
@@ -3907,10 +3907,12 @@
   [(set_attr "type" "load")
    (set_attr_alternative "iscompact"
      [(cond
-	[(ne (symbol_ref "GET_MODE (PATTERN (next_nonnote_insn (operands[3])))")
+	[(ne (symbol_ref "GET_MODE (PATTERN (next_nonnote_insn
+					       (as_a<rtx_insn *> (operands[3]))))")
 	     (symbol_ref "QImode"))
 	 (const_string "false")
-	 (match_test "!ADDR_DIFF_VEC_FLAGS (PATTERN (next_nonnote_insn (operands[3]))).offset_unsigned")
+	 (match_test "!ADDR_DIFF_VEC_FLAGS (PATTERN (next_nonnote_insn
+						       (as_a<rtx_insn *> (operands[3])))).offset_unsigned")
 	 (const_string "false")]
 	(const_string "true"))
       (const_string "false")
@@ -3946,7 +3948,7 @@
   "TARGET_COMPACT_CASESI"
   "*
 {
-  rtx diff_vec = PATTERN (next_nonnote_insn (operands[1]));
+  rtx diff_vec = PATTERN (next_nonnote_insn (as_a<rtx_insn *> (operands[1])));
   int unalign = arc_get_unalign ();
   rtx xop[3];
   const char *s;
diff --git a/gcc/cse.c b/gcc/cse.c
index 0bfd7ff..8637d57 100644
--- a/gcc/cse.c
+++ b/gcc/cse.c
@@ -3054,7 +3054,7 @@ find_comparison_args (enum rtx_code code, rtx *parg1, rtx *parg2,
 	 with floating-point operands.  */
       if (reverse_code)
 	{
-	  enum rtx_code reversed = reversed_comparison_code (x, NULL_RTX);
+	  enum rtx_code reversed = reversed_comparison_code (x, NULL);
 	  if (reversed == UNKNOWN)
 	    break;
 	  else
diff --git a/gcc/emit-rtl.c b/gcc/emit-rtl.c
index a724608..cbea214 100644
--- a/gcc/emit-rtl.c
+++ b/gcc/emit-rtl.c
@@ -3301,9 +3301,8 @@ previous_insn (rtx_insn *insn)
    look inside SEQUENCEs.  */
 
 rtx_insn *
-next_nonnote_insn (rtx uncast_insn)
+next_nonnote_insn (rtx_insn *insn)
 {
-  rtx_insn *insn = safe_as_a <rtx_insn *> (uncast_insn);
   while (insn)
     {
       insn = NEXT_INSN (insn);
@@ -3337,10 +3336,8 @@ next_nonnote_insn_bb (rtx_insn *insn)
    not look inside SEQUENCEs.  */
 
 rtx_insn *
-prev_nonnote_insn (rtx uncast_insn)
+prev_nonnote_insn (rtx_insn *insn)
 {
-  rtx_insn *insn = safe_as_a <rtx_insn *> (uncast_insn);
-
   while (insn)
     {
       insn = PREV_INSN (insn);
diff --git a/gcc/jump.c b/gcc/jump.c
index 22f8a71..87a1a5d 100644
--- a/gcc/jump.c
+++ b/gcc/jump.c
@@ -62,7 +62,7 @@ static void mark_all_labels (rtx_insn *);
 static void mark_jump_label_1 (rtx, rtx_insn *, bool, bool);
 static void mark_jump_label_asm (rtx, rtx_insn *);
 static void redirect_exp_1 (rtx *, rtx, rtx, rtx);
-static int invert_exp_1 (rtx, rtx);
+static int invert_exp_1 (rtx, rtx_insn *);
 \f
 /* Worker for rebuild_jump_labels and rebuild_jump_labels_chain.  */
 static void
@@ -360,7 +360,7 @@ mark_all_labels (rtx_insn *f)
    to help this function avoid overhead in these cases.  */
 enum rtx_code
 reversed_comparison_code_parts (enum rtx_code code, const_rtx arg0,
-				const_rtx arg1, const_rtx insn)
+				const_rtx arg1, const rtx_insn *insn)
 {
   machine_mode mode;
 
@@ -422,7 +422,7 @@ reversed_comparison_code_parts (enum rtx_code code, const_rtx arg0,
       /* These CONST_CAST's are okay because prev_nonnote_insn just
 	 returns its argument and we assign it to a const_rtx
 	 variable.  */
-      for (rtx_insn *prev = prev_nonnote_insn (CONST_CAST_RTX (insn));
+      for (rtx_insn *prev = prev_nonnote_insn (const_cast<rtx_insn *> (insn));
 	   prev != 0 && !LABEL_P (prev);
 	   prev = prev_nonnote_insn (prev))
 	{
@@ -470,7 +470,7 @@ reversed_comparison_code_parts (enum rtx_code code, const_rtx arg0,
 /* A wrapper around the previous function to take COMPARISON as rtx
    expression.  This simplifies many callers.  */
 enum rtx_code
-reversed_comparison_code (const_rtx comparison, const_rtx insn)
+reversed_comparison_code (const_rtx comparison, const rtx_insn *insn)
 {
   if (!COMPARISON_P (comparison))
     return UNKNOWN;
@@ -484,7 +484,7 @@ reversed_comparison_code (const_rtx comparison, const_rtx insn)
 rtx
 reversed_comparison (const_rtx exp, machine_mode mode)
 {
-  enum rtx_code reversed_code = reversed_comparison_code (exp, NULL_RTX);
+  enum rtx_code reversed_code = reversed_comparison_code (exp, NULL);
   if (reversed_code == UNKNOWN)
     return NULL_RTX;
   else
@@ -1623,7 +1623,7 @@ redirect_jump_2 (rtx_jump_insn *jump, rtx olabel, rtx nlabel, int delete_unused,
 /* Invert the jump condition X contained in jump insn INSN.  Accrue the
    modifications into the change group.  Return nonzero for success.  */
 static int
-invert_exp_1 (rtx x, rtx insn)
+invert_exp_1 (rtx x, rtx_insn *insn)
 {
   RTX_CODE code = GET_CODE (x);
 
diff --git a/gcc/reorg.c b/gcc/reorg.c
index d798c6a..44cbf9f 100644
--- a/gcc/reorg.c
+++ b/gcc/reorg.c
@@ -219,12 +219,12 @@ static void steal_delay_list_from_fallthrough (rtx_insn *, rtx, rtx_sequence *,
 					       struct resources *,
 					       int, int *, int *);
 static void try_merge_delay_insns (rtx_insn *, rtx_insn *);
-static rtx redundant_insn (rtx, rtx_insn *, const vec<rtx_insn *> &);
+static rtx_insn *redundant_insn (rtx, rtx_insn *, const vec<rtx_insn *> &);
 static int own_thread_p (rtx, rtx, int);
 static void update_block (rtx_insn *, rtx);
 static int reorg_redirect_jump (rtx_jump_insn *, rtx);
 static void update_reg_dead_notes (rtx_insn *, rtx_insn *);
-static void fix_reg_dead_note (rtx, rtx);
+static void fix_reg_dead_note (rtx_insn *, rtx);
 static void update_reg_unused_notes (rtx, rtx);
 static void fill_simple_delay_slots (int);
 static void fill_slots_from_thread (rtx_jump_insn *, rtx, rtx, rtx,
@@ -1449,7 +1449,7 @@ try_merge_delay_insns (rtx_insn *insn, rtx_insn *thread)
    redundant insn, but the cost of splitting seems greater than the possible
    gain in rare cases.  */
 
-static rtx
+static rtx_insn *
 redundant_insn (rtx insn, rtx_insn *target, const vec<rtx_insn *> &delay_list)
 {
   rtx target_main = target;
@@ -1606,7 +1606,7 @@ redundant_insn (rtx insn, rtx_insn *target, const vec<rtx_insn *> &delay_list)
 	     resource requirements as we go.  */
 	  for (i = seq->len () - 1; i > 0; i--)
 	    {
-	      rtx candidate = seq->element (i);
+	      rtx_insn *candidate = seq->insn (i);
 
 	      /* If an insn will be annulled if the branch is false, it isn't
 		 considered as a possible duplicate insn.  */
@@ -1773,7 +1773,7 @@ update_reg_dead_notes (rtx_insn *insn, rtx_insn *delayed_insn)
    confused into thinking the register is dead.  */
 
 static void
-fix_reg_dead_note (rtx start_insn, rtx stop_insn)
+fix_reg_dead_note (rtx_insn *start_insn, rtx stop_insn)
 {
   rtx link, next;
   rtx_insn *p;
@@ -2417,7 +2417,7 @@ fill_slots_from_thread (rtx_jump_insn *insn, rtx condition,
 			      && (! own_thread || ! sets_cc0_p (pat)))))
 	  && ! can_throw_internal (trial))
 	{
-	  rtx prior_insn;
+	  rtx_insn *prior_insn;
 
 	  /* If TRIAL is redundant with some insn before INSN, we don't
 	     actually need to add it to the delay list; we can merely pretend
@@ -2904,16 +2904,16 @@ fill_eager_delay_slots (void)
     }
 }
 \f
-static void delete_computation (rtx insn);
+static void delete_computation (rtx_insn *insn);
 
 /* Recursively delete prior insns that compute the value (used only by INSN
    which the caller is deleting) stored in the register mentioned by NOTE
    which is a REG_DEAD note associated with INSN.  */
 
 static void
-delete_prior_computation (rtx note, rtx insn)
+delete_prior_computation (rtx note, rtx_insn *insn)
 {
-  rtx our_prev;
+  rtx_insn *our_prev;
   rtx reg = XEXP (note, 0);
 
   for (our_prev = prev_nonnote_insn (insn);
@@ -3025,7 +3025,7 @@ delete_prior_computation (rtx note, rtx insn)
    delete the insn that set it.  */
 
 static void
-delete_computation (rtx insn)
+delete_computation (rtx_insn *insn)
 {
   rtx note, next;
 
@@ -3367,7 +3367,7 @@ relax_delay_slots (rtx_insn *first)
       if (! INSN_ANNULLED_BRANCH_P (delay_jump_insn)
 	  && ! condjump_in_parallel_p (delay_jump_insn)
 	  && prev_active_insn (target_label) == insn
-	  && ! BARRIER_P (prev_nonnote_insn (target_label))
+	  && ! BARRIER_P (prev_nonnote_insn (as_a<rtx_insn *> (target_label)))
 	  /* If the last insn in the delay slot sets CC0 for some insn,
 	     various code assumes that it is in a delay slot.  We could
 	     put it back where it belonged and delete the register notes,
diff --git a/gcc/rtl.h b/gcc/rtl.h
index b531ab7..0d121bc 100644
--- a/gcc/rtl.h
+++ b/gcc/rtl.h
@@ -2834,9 +2834,9 @@ extern void add_function_usage_to (rtx, rtx);
 extern rtx_call_insn *last_call_insn (void);
 extern rtx_insn *previous_insn (rtx_insn *);
 extern rtx_insn *next_insn (rtx_insn *);
-extern rtx_insn *prev_nonnote_insn (rtx);
+extern rtx_insn *prev_nonnote_insn (rtx_insn *);
 extern rtx_insn *prev_nonnote_insn_bb (rtx);
-extern rtx_insn *next_nonnote_insn (rtx);
+extern rtx_insn *next_nonnote_insn (rtx_insn *);
 extern rtx_insn *next_nonnote_insn_bb (rtx_insn *);
 extern rtx_insn *prev_nondebug_insn (rtx);
 extern rtx_insn *next_nondebug_insn (rtx);
@@ -3498,9 +3498,9 @@ extern int redirect_jump (rtx_jump_insn *, rtx, int);
 extern void rebuild_jump_labels (rtx_insn *);
 extern void rebuild_jump_labels_chain (rtx_insn *);
 extern rtx reversed_comparison (const_rtx, machine_mode);
-extern enum rtx_code reversed_comparison_code (const_rtx, const_rtx);
+extern enum rtx_code reversed_comparison_code (const_rtx, const rtx_insn *);
 extern enum rtx_code reversed_comparison_code_parts (enum rtx_code, const_rtx,
-						     const_rtx, const_rtx);
+						     const_rtx, const rtx_insn *);
 extern void delete_for_peephole (rtx_insn *, rtx_insn *);
 extern int condjump_in_parallel_p (const rtx_insn *);
 
diff --git a/gcc/simplify-rtx.c b/gcc/simplify-rtx.c
index 2085b99..6ad7d7e 100644
--- a/gcc/simplify-rtx.c
+++ b/gcc/simplify-rtx.c
@@ -887,7 +887,7 @@ simplify_unary_operation_1 (enum rtx_code code, machine_mode mode, rtx op)
 	 comparison is all ones.   */
       if (COMPARISON_P (op)
 	  && (mode == BImode || STORE_FLAG_VALUE == -1)
-	  && ((reversed = reversed_comparison_code (op, NULL_RTX)) != UNKNOWN))
+	  && ((reversed = reversed_comparison_code (op, NULL)) != UNKNOWN))
 	return simplify_gen_relational (reversed, mode, VOIDmode,
 					XEXP (op, 0), XEXP (op, 1));
 
@@ -1010,7 +1010,7 @@ simplify_unary_operation_1 (enum rtx_code code, machine_mode mode, rtx op)
 	       || (GET_CODE (false_rtx) == NEG
 		   && rtx_equal_p (XEXP (false_rtx, 0), true_rtx)))
 	    {
-	      if (reversed_comparison_code (cond, NULL_RTX) != UNKNOWN)
+	      if (reversed_comparison_code (cond, NULL) != UNKNOWN)
 		temp = reversed_comparison (cond, mode);
 	      else
 		{
@@ -4628,7 +4628,7 @@ simplify_relational_operation_1 (enum rtx_code code, machine_mode mode,
 	}
       else if (code == EQ)
 	{
-	  enum rtx_code new_code = reversed_comparison_code (op0, NULL_RTX);
+	  enum rtx_code new_code = reversed_comparison_code (op0, NULL);
 	  if (new_code != UNKNOWN)
 	    return simplify_gen_relational (new_code, mode, VOIDmode,
 					    XEXP (op0, 0), XEXP (op0, 1));
@@ -5475,7 +5475,7 @@ simplify_ternary_operation (enum rtx_code code, machine_mode mode,
 	      else if (t == 0 && f == STORE_FLAG_VALUE)
 		{
 		  enum rtx_code tmp;
-		  tmp = reversed_comparison_code (op0, NULL_RTX);
+		  tmp = reversed_comparison_code (op0, NULL);
 		  if (tmp == UNKNOWN)
 		    break;
 		  code = tmp;
-- 
2.9.3

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

* [PATCH 6/8] make next/prev nonnote_nondebug_insn take rtx_insn *
  2016-09-14 19:13 [PATCH 0/8] make next_*_insn take rtx_insn * arguments tbsaunde+gcc
                   ` (5 preceding siblings ...)
  2016-09-14 19:14 ` [PATCH 2/8] use rtx_insn * more tbsaunde+gcc
@ 2016-09-14 19:19 ` tbsaunde+gcc
  2016-09-19 21:09   ` Jeff Law
  2016-09-14 19:33 ` [PATCH 5/8] make prev_real_insn " tbsaunde+gcc
  2016-09-15 11:02 ` [PATCH 0/8] make next_*_insn take rtx_insn * arguments Bernd Schmidt
  8 siblings, 1 reply; 38+ messages in thread
From: tbsaunde+gcc @ 2016-09-14 19:19 UTC (permalink / raw)
  To: gcc-patches

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

gcc/ChangeLog:

2016-09-14  Trevor Saunders  <tbsaunde+gcc@tbsaunde.org>

	* config/cris/cris.c (cris_asm_output_case_end): Change argument
type to rtx_insn *.
	* emit-rtl.c (next_nonnote_nondebug_insn): Likewise.
	(prev_nonnote_nondebug_insn): Likewise.
	* config/cris/cris-protos.h: Adjust prototype.
	* rtl.h: Likewise.
	* jump.c (rtx_renumbered_equal_p): Adjust.
---
 gcc/config/cris/cris-protos.h |  2 +-
 gcc/config/cris/cris.c        | 12 ++++++------
 gcc/emit-rtl.c                |  8 ++------
 gcc/jump.c                    |  6 ++++--
 gcc/rtl.h                     |  4 ++--
 5 files changed, 15 insertions(+), 17 deletions(-)

diff --git a/gcc/config/cris/cris-protos.h b/gcc/config/cris/cris-protos.h
index 5e0ae61..028c2b7 100644
--- a/gcc/config/cris/cris-protos.h
+++ b/gcc/config/cris/cris-protos.h
@@ -44,7 +44,7 @@ extern bool cris_store_multiple_op_p (rtx);
 extern bool cris_movem_load_rest_p (rtx, int);
 extern void cris_asm_output_symbol_ref (FILE *, rtx);
 extern int cris_cfun_uses_pic_table (void);
-extern void cris_asm_output_case_end (FILE *, int, rtx);
+extern void cris_asm_output_case_end (FILE *, int, rtx_insn *);
 extern rtx cris_gen_movem_load (rtx, rtx, int);
 extern rtx cris_emit_movem_store (rtx, rtx, int, bool);
 extern void cris_expand_pic_call_address (rtx *, rtx *);
diff --git a/gcc/config/cris/cris.c b/gcc/config/cris/cris.c
index 39118f0..bd49152 100644
--- a/gcc/config/cris/cris.c
+++ b/gcc/config/cris/cris.c
@@ -2586,11 +2586,11 @@ cris_asm_output_ident (const char *string)
 /* The ASM_OUTPUT_CASE_END worker.  */
 
 void
-cris_asm_output_case_end (FILE *stream, int num, rtx table)
+cris_asm_output_case_end (FILE *stream, int num, rtx_insn *table)
 {
   /* Step back, over the label for the table, to the actual casejump and
      assert that we find only what's expected.  */
-  rtx whole_jump_insn = prev_nonnote_nondebug_insn (table);
+  rtx_insn *whole_jump_insn = prev_nonnote_nondebug_insn (table);
   gcc_assert (whole_jump_insn != NULL_RTX && LABEL_P (whole_jump_insn));
   whole_jump_insn = prev_nonnote_nondebug_insn (whole_jump_insn);
   gcc_assert (whole_jump_insn != NULL_RTX
@@ -2598,15 +2598,15 @@ cris_asm_output_case_end (FILE *stream, int num, rtx table)
 		  || (TARGET_V32 && INSN_P (whole_jump_insn)
 		      && GET_CODE (PATTERN (whole_jump_insn)) == SEQUENCE)));
   /* Get the pattern of the casejump, so we can extract the default label.  */
-  whole_jump_insn = PATTERN (whole_jump_insn);
+  rtx whole_jump_pat = PATTERN (whole_jump_insn);
 
   if (TARGET_V32)
     {
       /* This can be a SEQUENCE, meaning the delay-slot of the jump is
 	 filled.  We also output the offset word a little differently.  */
       rtx parallel_jump
-	= (GET_CODE (whole_jump_insn) == SEQUENCE
-	   ? PATTERN (XVECEXP (whole_jump_insn, 0, 0)) : whole_jump_insn);
+	= (GET_CODE (whole_jump_pat) == SEQUENCE
+	   ? PATTERN (XVECEXP (whole_jump_pat, 0, 0)) : whole_jump_pat);
 
       asm_fprintf (stream,
 		   "\t.word %LL%d-.%s\n",
@@ -2621,7 +2621,7 @@ cris_asm_output_case_end (FILE *stream, int num, rtx table)
 	       "\t.word %LL%d-%LL%d%s\n",
 	       CODE_LABEL_NUMBER (XEXP
 				  (XEXP
-				   (XEXP (XVECEXP (whole_jump_insn, 0, 0), 1), 
+				   (XEXP (XVECEXP (whole_jump_pat, 0, 0), 1),
 				    2), 0)),
 	       num,
 	       (TARGET_PDEBUG ? "; default" : ""));
diff --git a/gcc/emit-rtl.c b/gcc/emit-rtl.c
index df5d359..20d8250 100644
--- a/gcc/emit-rtl.c
+++ b/gcc/emit-rtl.c
@@ -3405,10 +3405,8 @@ prev_nondebug_insn (rtx_insn *insn)
    This routine does not look inside SEQUENCEs.  */
 
 rtx_insn *
-next_nonnote_nondebug_insn (rtx uncast_insn)
+next_nonnote_nondebug_insn (rtx_insn *insn)
 {
-  rtx_insn *insn = safe_as_a <rtx_insn *> (uncast_insn);
-
   while (insn)
     {
       insn = NEXT_INSN (insn);
@@ -3423,10 +3421,8 @@ next_nonnote_nondebug_insn (rtx uncast_insn)
    This routine does not look inside SEQUENCEs.  */
 
 rtx_insn *
-prev_nonnote_nondebug_insn (rtx uncast_insn)
+prev_nonnote_nondebug_insn (rtx_insn *insn)
 {
-  rtx_insn *insn = safe_as_a <rtx_insn *> (uncast_insn);
-
   while (insn)
     {
       insn = PREV_INSN (insn);
diff --git a/gcc/jump.c b/gcc/jump.c
index 87a1a5d..2164c3b 100644
--- a/gcc/jump.c
+++ b/gcc/jump.c
@@ -1806,8 +1806,10 @@ rtx_renumbered_equal_p (const_rtx x, const_rtx y)
 	 in the same position in the instruction stream.  */
       else
 	{
-	  rtx_insn *xi = next_nonnote_nondebug_insn (LABEL_REF_LABEL (x));
-	  rtx_insn *yi = next_nonnote_nondebug_insn (LABEL_REF_LABEL (y));
+	  rtx_insn *xi = next_nonnote_nondebug_insn
+	    (as_a<rtx_insn *> (LABEL_REF_LABEL (x)));
+	  rtx_insn *yi = next_nonnote_nondebug_insn
+	    (as_a<rtx_insn *> (LABEL_REF_LABEL (y)));
 	  while (xi && LABEL_P (xi))
 	    xi = next_nonnote_nondebug_insn (xi);
 	  while (yi && LABEL_P (yi))
diff --git a/gcc/rtl.h b/gcc/rtl.h
index c253fda..4ae9a25 100644
--- a/gcc/rtl.h
+++ b/gcc/rtl.h
@@ -2840,8 +2840,8 @@ extern rtx_insn *next_nonnote_insn (rtx_insn *);
 extern rtx_insn *next_nonnote_insn_bb (rtx_insn *);
 extern rtx_insn *prev_nondebug_insn (rtx_insn *);
 extern rtx_insn *next_nondebug_insn (rtx_insn *);
-extern rtx_insn *prev_nonnote_nondebug_insn (rtx);
-extern rtx_insn *next_nonnote_nondebug_insn (rtx);
+extern rtx_insn *prev_nonnote_nondebug_insn (rtx_insn *);
+extern rtx_insn *next_nonnote_nondebug_insn (rtx_insn *);
 extern rtx_insn *prev_real_insn (rtx_insn *);
 extern rtx_insn *next_real_insn (rtx);
 extern rtx_insn *prev_active_insn (rtx);
-- 
2.9.3

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

* [PATCH 5/8] make prev_real_insn take rtx_insn *
  2016-09-14 19:13 [PATCH 0/8] make next_*_insn take rtx_insn * arguments tbsaunde+gcc
                   ` (6 preceding siblings ...)
  2016-09-14 19:19 ` [PATCH 6/8] make next/prev nonnote_nondebug_insn take rtx_insn * tbsaunde+gcc
@ 2016-09-14 19:33 ` tbsaunde+gcc
  2016-09-19 21:08   ` Jeff Law
  2016-09-15 11:02 ` [PATCH 0/8] make next_*_insn take rtx_insn * arguments Bernd Schmidt
  8 siblings, 1 reply; 38+ messages in thread
From: tbsaunde+gcc @ 2016-09-14 19:33 UTC (permalink / raw)
  To: gcc-patches

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

gcc/ChangeLog:

2016-09-13  Trevor Saunders  <tbsaunde+gcc@tbsaunde.org>

	* emit-rtl.c (prev_real_insn): Change argument type to rtx_insn *.
	* rtl.h: Adjust prototype.
	* config/sh/sh.md: Adjust.
	* dwarf2out.c (add_var_loc_to_decl): Likewise.
---
 gcc/config/sh/sh.md | 3 ++-
 gcc/dwarf2out.c     | 2 +-
 gcc/emit-rtl.c      | 4 +---
 gcc/rtl.h           | 2 +-
 4 files changed, 5 insertions(+), 6 deletions(-)

diff --git a/gcc/config/sh/sh.md b/gcc/config/sh/sh.md
index edc4d15..25e03ef 100644
--- a/gcc/config/sh/sh.md
+++ b/gcc/config/sh/sh.md
@@ -7178,7 +7178,8 @@
 		      (label_ref (match_operand 1 "" ""))))
    (use (label_ref (match_operand 2 "" "")))]
   "TARGET_SH2
-   && (! INSN_UID (operands[1]) || prev_real_insn (operands[1]) == insn)"
+   && (! INSN_UID (operands[1])
+       || prev_real_insn (as_a<rtx_insn *> (operands[1])) == insn)"
   "braf	%0%#"
   [(set_attr "needs_delay_slot" "yes")
    (set_attr "type" "jump_ind")])
diff --git a/gcc/dwarf2out.c b/gcc/dwarf2out.c
index 45eb684..fb8ec7d 100644
--- a/gcc/dwarf2out.c
+++ b/gcc/dwarf2out.c
@@ -5408,7 +5408,7 @@ add_var_loc_to_decl (tree decl, rtx loc_note, const char *label)
       && NOTE_VAR_LOCATION_LOC (temp->first->loc)
       && GET_CODE (NOTE_VAR_LOCATION_LOC (temp->first->loc))
 	 == GET_CODE (DECL_INCOMING_RTL (decl))
-      && prev_real_insn (temp->first->loc) == NULL_RTX
+      && prev_real_insn (as_a<rtx_insn *> (temp->first->loc)) == NULL_RTX
       && (bitsize != -1
 	  || !rtx_equal_p (NOTE_VAR_LOCATION_LOC (temp->first->loc),
 			   NOTE_VAR_LOCATION_LOC (loc_note))
diff --git a/gcc/emit-rtl.c b/gcc/emit-rtl.c
index 6051326..df5d359 100644
--- a/gcc/emit-rtl.c
+++ b/gcc/emit-rtl.c
@@ -3461,10 +3461,8 @@ next_real_insn (rtx uncast_insn)
    SEQUENCEs.  */
 
 rtx_insn *
-prev_real_insn (rtx uncast_insn)
+prev_real_insn (rtx_insn *insn)
 {
-  rtx_insn *insn = safe_as_a <rtx_insn *> (uncast_insn);
-
   while (insn)
     {
       insn = PREV_INSN (insn);
diff --git a/gcc/rtl.h b/gcc/rtl.h
index b557ffe..c253fda 100644
--- a/gcc/rtl.h
+++ b/gcc/rtl.h
@@ -2842,7 +2842,7 @@ extern rtx_insn *prev_nondebug_insn (rtx_insn *);
 extern rtx_insn *next_nondebug_insn (rtx_insn *);
 extern rtx_insn *prev_nonnote_nondebug_insn (rtx);
 extern rtx_insn *next_nonnote_nondebug_insn (rtx);
-extern rtx_insn *prev_real_insn (rtx);
+extern rtx_insn *prev_real_insn (rtx_insn *);
 extern rtx_insn *next_real_insn (rtx);
 extern rtx_insn *prev_active_insn (rtx);
 extern rtx_insn *next_active_insn (rtx);
-- 
2.9.3

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

* Re: [PATCH 1/8] change a few rtx_insn * to rtx_jump_insn *
  2016-09-14 19:13 ` [PATCH 1/8] change a few rtx_insn * to rtx_jump_insn * tbsaunde+gcc
@ 2016-09-15  4:33   ` Kaz Kojima
  2016-09-15  5:56     ` Trevor Saunders
  0 siblings, 1 reply; 38+ messages in thread
From: Kaz Kojima @ 2016-09-15  4:33 UTC (permalink / raw)
  To: tbsaunde+gcc; +Cc: gcc-patches

tbsaunde+gcc@tbsaunde.org wrote:
> @@ -2201,8 +2201,7 @@ fix_crossing_unconditional_branches (void)
>  		{
>  		  if (!BARRIER_P (cur_insn))
>  		    BLOCK_FOR_INSN (cur_insn) = cur_bb;
> -		  if (JUMP_P (cur_insn))
> -		    jump_insn = cur_insn;
> +		  jump_insn = dyn_cast<rtx_jump_insn *> (cur_insn);
>  		}

This hunk results several new failures for tree-profile tests on SH.
If the line "if (JUMP_P (cur_insn))" is restored, those failures
go away.

Regards,
	kaz

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

* Re: [PATCH 1/8] change a few rtx_insn * to rtx_jump_insn *
  2016-09-15  4:33   ` Kaz Kojima
@ 2016-09-15  5:56     ` Trevor Saunders
  2016-09-15  7:08       ` Kaz Kojima
  2016-09-15  8:51       ` Richard Biener
  0 siblings, 2 replies; 38+ messages in thread
From: Trevor Saunders @ 2016-09-15  5:56 UTC (permalink / raw)
  To: Kaz Kojima; +Cc: tbsaunde+gcc, gcc-patches

On Thu, Sep 15, 2016 at 01:04:04PM +0900, Kaz Kojima wrote:
> tbsaunde+gcc@tbsaunde.org wrote:
> > @@ -2201,8 +2201,7 @@ fix_crossing_unconditional_branches (void)
> >  		{
> >  		  if (!BARRIER_P (cur_insn))
> >  		    BLOCK_FOR_INSN (cur_insn) = cur_bb;
> > -		  if (JUMP_P (cur_insn))
> > -		    jump_insn = cur_insn;
> > +		  jump_insn = dyn_cast<rtx_jump_insn *> (cur_insn);
> >  		}
> 
> This hunk results several new failures for tree-profile tests on SH.
> If the line "if (JUMP_P (cur_insn))" is restored, those failures
> go away.

That's interesting because dyn_cast should include that check.  What is
the error?

Thanks!

Trev

> 
> Regards,
> 	kaz

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

* Re: [PATCH 1/8] change a few rtx_insn * to rtx_jump_insn *
  2016-09-15  5:56     ` Trevor Saunders
@ 2016-09-15  7:08       ` Kaz Kojima
  2016-09-15  8:51       ` Richard Biener
  1 sibling, 0 replies; 38+ messages in thread
From: Kaz Kojima @ 2016-09-15  7:08 UTC (permalink / raw)
  To: tbsaunde; +Cc: tbsaunde+gcc, gcc-patches

Trevor Saunders <tbsaunde@tbsaunde.org> wrote:
>> This hunk results several new failures for tree-profile tests on SH.
>> If the line "if (JUMP_P (cur_insn))" is restored, those failures
>> go away.
> 
> That's interesting because dyn_cast should include that check.  What is
> the error?

Here is a typical log:

spawn -ignore SIGHUP /home/ldroot/dodes/xsh-gcc/gcc/xgcc -B/home/ldroot/dodes/xsh-gcc/gcc/ /exp/ldroot/dodes/LOCAL/trunk/gcc/testsuite/gcc.dg/tree-prof/20041218-1.c -fno-diagnostics-show-caret -fdiagnostics-color=never -O2 -freorder-blocks-and-partition -fprofile-use -D_PROFILE_USE -lm -o /home/ldroot/dodes/xsh-gcc/gcc/testsuite/gcc/20041218-1.x02
/exp/ldroot/dodes/LOCAL/trunk/gcc/testsuite/gcc.dg/tree-prof/20041218-1.c: In function 'check':
/exp/ldroot/dodes/LOCAL/trunk/gcc/testsuite/gcc.dg/tree-prof/20041218-1.c:55:1: internal compiler error: Segmentation fault
0x86cd24a crash_signal
	../../LOCAL/trunk/gcc/toplev.c:336
0x8b4cf5e fix_crossing_unconditional_branches
	../../LOCAL/trunk/gcc/bb-reorder.c:2214
0x8b4e18a execute
	../../LOCAL/trunk/gcc/bb-reorder.c:2933

Regards,
	kaz

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

* Re: [PATCH 1/8] change a few rtx_insn * to rtx_jump_insn *
  2016-09-15  5:56     ` Trevor Saunders
  2016-09-15  7:08       ` Kaz Kojima
@ 2016-09-15  8:51       ` Richard Biener
  2016-09-15  9:26         ` Trevor Saunders
  1 sibling, 1 reply; 38+ messages in thread
From: Richard Biener @ 2016-09-15  8:51 UTC (permalink / raw)
  To: Trevor Saunders; +Cc: Kaz Kojima, tbsaunde+gcc, GCC Patches

On Thu, Sep 15, 2016 at 6:31 AM, Trevor Saunders <tbsaunde@tbsaunde.org> wrote:
> On Thu, Sep 15, 2016 at 01:04:04PM +0900, Kaz Kojima wrote:
>> tbsaunde+gcc@tbsaunde.org wrote:
>> > @@ -2201,8 +2201,7 @@ fix_crossing_unconditional_branches (void)
>> >             {
>> >               if (!BARRIER_P (cur_insn))
>> >                 BLOCK_FOR_INSN (cur_insn) = cur_bb;
>> > -             if (JUMP_P (cur_insn))
>> > -               jump_insn = cur_insn;
>> > +             jump_insn = dyn_cast<rtx_jump_insn *> (cur_insn);
>> >             }
>>
>> This hunk results several new failures for tree-profile tests on SH.
>> If the line "if (JUMP_P (cur_insn))" is restored, those failures
>> go away.
>
> That's interesting because dyn_cast should include that check.  What is
> the error?

maybe jump_insn is non-NULL before?  in which case you set it to NULL
if ! JUMP_P while before we didn't.

Richard.

> Thanks!
>
> Trev
>
>>
>> Regards,
>>       kaz

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

* Re: [PATCH 1/8] change a few rtx_insn * to rtx_jump_insn *
  2016-09-15  8:51       ` Richard Biener
@ 2016-09-15  9:26         ` Trevor Saunders
  0 siblings, 0 replies; 38+ messages in thread
From: Trevor Saunders @ 2016-09-15  9:26 UTC (permalink / raw)
  To: Richard Biener; +Cc: Kaz Kojima, tbsaunde+gcc, GCC Patches

On Thu, Sep 15, 2016 at 10:28:19AM +0200, Richard Biener wrote:
> On Thu, Sep 15, 2016 at 6:31 AM, Trevor Saunders <tbsaunde@tbsaunde.org> wrote:
> > On Thu, Sep 15, 2016 at 01:04:04PM +0900, Kaz Kojima wrote:
> >> tbsaunde+gcc@tbsaunde.org wrote:
> >> > @@ -2201,8 +2201,7 @@ fix_crossing_unconditional_branches (void)
> >> >             {
> >> >               if (!BARRIER_P (cur_insn))
> >> >                 BLOCK_FOR_INSN (cur_insn) = cur_bb;
> >> > -             if (JUMP_P (cur_insn))
> >> > -               jump_insn = cur_insn;
> >> > +             jump_insn = dyn_cast<rtx_jump_insn *> (cur_insn);
> >> >             }
> >>
> >> This hunk results several new failures for tree-profile tests on SH.
> >> If the line "if (JUMP_P (cur_insn))" is restored, those failures
> >> go away.
> >
> > That's interesting because dyn_cast should include that check.  What is
> > the error?
> 
> maybe jump_insn is non-NULL before?  in which case you set it to NULL
> if ! JUMP_P while before we didn't.

Oh wow, I missed that this  assignment was in a loop not an iff :(
makes sense now and easy enough to fix up.

Trev

> 
> Richard.
> 
> > Thanks!
> >
> > Trev
> >
> >>
> >> Regards,
> >>       kaz

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

* Re: [PATCH 0/8] make next_*_insn take rtx_insn * arguments
  2016-09-14 19:13 [PATCH 0/8] make next_*_insn take rtx_insn * arguments tbsaunde+gcc
                   ` (7 preceding siblings ...)
  2016-09-14 19:33 ` [PATCH 5/8] make prev_real_insn " tbsaunde+gcc
@ 2016-09-15 11:02 ` Bernd Schmidt
  2016-09-15 16:23   ` Trevor Saunders
  8 siblings, 1 reply; 38+ messages in thread
From: Bernd Schmidt @ 2016-09-15 11:02 UTC (permalink / raw)
  To: tbsaunde+gcc, gcc-patches

On 09/14/2016 09:21 PM, tbsaunde+gcc@tbsaunde.org wrote:

> Basically $subject.  First change variable's type to rtx_insn * where possible.
> Then change the functions and fixup callers where it is still necessary to
> cast.

#2, #4 and #8 look good and can be applied if they work independently of 
the others.

Less certain about some of the others which introduce additional casts. 
Maybe LABEL_REF_LABEL needs converting first, and reorg.c has a few 
variables that might have to be made rtx_insn * in patch #7 to avoid casts.


Bernd

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

* Re: [PATCH 0/8] make next_*_insn take rtx_insn * arguments
  2016-09-15 11:02 ` [PATCH 0/8] make next_*_insn take rtx_insn * arguments Bernd Schmidt
@ 2016-09-15 16:23   ` Trevor Saunders
  2016-09-15 16:42     ` Jeff Law
  0 siblings, 1 reply; 38+ messages in thread
From: Trevor Saunders @ 2016-09-15 16:23 UTC (permalink / raw)
  To: Bernd Schmidt; +Cc: tbsaunde+gcc, gcc-patches

On Thu, Sep 15, 2016 at 12:52:59PM +0200, Bernd Schmidt wrote:
> On 09/14/2016 09:21 PM, tbsaunde+gcc@tbsaunde.org wrote:
> 
> > Basically $subject.  First change variable's type to rtx_insn * where possible.
> > Then change the functions and fixup callers where it is still necessary to
> > cast.
> 
> #2, #4 and #8 look good and can be applied if they work independently of the
> others.

at most #2 should depend on #1 so it should be fine and I can check on
the others.

> Less certain about some of the others which introduce additional casts.

yeah, its somewhat unfortunate, though one way or another we will need
to add casts I think the question is just how many we will accept and
where.

> Maybe LABEL_REF_LABEL needs converting first, and reorg.c has a few
> variables that might have to be made rtx_insn * in patch #7 to avoid casts.

LABEL_REF_LABEL might be doable, its a good idea I'll look into.  The
reorg.c stuff around target_label is rather complicated unfortunately.
In the end I of course agree the variables should be rtx_insn *.
However currently things are assigned to that variable that are not
insns.  So we need to break the variable up, but its involved in a lot
of code I don't think I know well enough to really refactor.  For
example it looks like target_label can hold a value between iterations
of the loop, I suspect that would be a bug, but I'm not really sure.

Trev

> 
> 
> Bernd

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

* Re: [PATCH 0/8] make next_*_insn take rtx_insn * arguments
  2016-09-15 16:23   ` Trevor Saunders
@ 2016-09-15 16:42     ` Jeff Law
  2016-09-15 17:42       ` Trevor Saunders
  2016-09-16 10:07       ` Trevor Saunders
  0 siblings, 2 replies; 38+ messages in thread
From: Jeff Law @ 2016-09-15 16:42 UTC (permalink / raw)
  To: Trevor Saunders, Bernd Schmidt; +Cc: tbsaunde+gcc, gcc-patches

On 09/15/2016 10:10 AM, Trevor Saunders wrote:
> On Thu, Sep 15, 2016 at 12:52:59PM +0200, Bernd Schmidt wrote:
>> On 09/14/2016 09:21 PM, tbsaunde+gcc@tbsaunde.org wrote:
>>
>>> Basically $subject.  First change variable's type to rtx_insn * where possible.
>>> Then change the functions and fixup callers where it is still necessary to
>>> cast.
>>
>> #2, #4 and #8 look good and can be applied if they work independently of the
>> others.
>
> at most #2 should depend on #1 so it should be fine and I can check on
> the others.
>
>> Less certain about some of the others which introduce additional casts.
>
> yeah, its somewhat unfortunate, though one way or another we will need
> to add casts I think the question is just how many we will accept and
> where.
In my mind the casts represent the "bounds" of how far we've taken this 
work.  They occur at the boundaries where we haven't converted something 
from an "rtx" to an "rtx_insn *" and allow us to do the work piecemeal 
rather than all-at-once.

What I don't have a sense of is when we'll be able to push rtx_insn * 
far enough that we're able to start removing casts.

And that might be a good way to prioritize the next batch of work.  Pick 
a cast, remove it and deal with the fallout.  :-)


>
>> Maybe LABEL_REF_LABEL needs converting first, and reorg.c has a few
>> variables that might have to be made rtx_insn * in patch #7 to avoid casts.
>
> LABEL_REF_LABEL might be doable, its a good idea I'll look into.  The
> reorg.c stuff around target_label is rather complicated unfortunately.
> In the end I of course agree the variables should be rtx_insn *.
> However currently things are assigned to that variable that are not
> insns.  So we need to break the variable up, but its involved in a lot
> of code I don't think I know well enough to really refactor.  For
> example it looks like target_label can hold a value between iterations
> of the loop, I suspect that would be a bug, but I'm not really sure.
I can probably help with reorg.  Hell, you might even be referring to my 
code!
jeff

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

* Re: [PATCH 0/8] make next_*_insn take rtx_insn * arguments
  2016-09-15 16:42     ` Jeff Law
@ 2016-09-15 17:42       ` Trevor Saunders
  2016-09-16 10:07       ` Trevor Saunders
  1 sibling, 0 replies; 38+ messages in thread
From: Trevor Saunders @ 2016-09-15 17:42 UTC (permalink / raw)
  To: Jeff Law; +Cc: Bernd Schmidt, tbsaunde+gcc, gcc-patches

On Thu, Sep 15, 2016 at 10:27:56AM -0600, Jeff Law wrote:
> On 09/15/2016 10:10 AM, Trevor Saunders wrote:
> > On Thu, Sep 15, 2016 at 12:52:59PM +0200, Bernd Schmidt wrote:
> > > On 09/14/2016 09:21 PM, tbsaunde+gcc@tbsaunde.org wrote:
> > > 
> > > > Basically $subject.  First change variable's type to rtx_insn * where possible.
> > > > Then change the functions and fixup callers where it is still necessary to
> > > > cast.
> > > 
> > > #2, #4 and #8 look good and can be applied if they work independently of the
> > > others.
> > 
> > at most #2 should depend on #1 so it should be fine and I can check on
> > the others.
> > 
> > > Less certain about some of the others which introduce additional casts.
> > 
> > yeah, its somewhat unfortunate, though one way or another we will need
> > to add casts I think the question is just how many we will accept and
> > where.
> In my mind the casts represent the "bounds" of how far we've taken this
> work.  They occur at the boundaries where we haven't converted something
> from an "rtx" to an "rtx_insn *" and allow us to do the work piecemeal
> rather than all-at-once.
> 
> What I don't have a sense of is when we'll be able to push rtx_insn * far
> enough that we're able to start removing casts.

Well, this series does remove a few, though I'm sure it is a net
addition.

> And that might be a good way to prioritize the next batch of work.  Pick a
> cast, remove it and deal with the fallout.  :-)

yeah, that's more or less what I did here.  each of the next_X_insn
methods took a rtx and cast it to rtx_insn *, so I changed it to take a
rtx_insn * and dropped the cast.  Then I fixed up the fallout and moved
the conversion of variable types to the beginning of the series since it
is less objectionable.

> > 
> > > Maybe LABEL_REF_LABEL needs converting first, and reorg.c has a few
> > > variables that might have to be made rtx_insn * in patch #7 to avoid casts.
> > 
> > LABEL_REF_LABEL might be doable, its a good idea I'll look into.  The
> > reorg.c stuff around target_label is rather complicated unfortunately.
> > In the end I of course agree the variables should be rtx_insn *.
> > However currently things are assigned to that variable that are not
> > insns.  So we need to break the variable up, but its involved in a lot
> > of code I don't think I know well enough to really refactor.  For
> > example it looks like target_label can hold a value between iterations
> > of the loop, I suspect that would be a bug, but I'm not really sure.
> I can probably help with reorg.  Hell, you might even be referring to my
> code!

heh, that'd be useful.  Though I'm not really sure how important it is
to clean up code specific to targets with delay slots.

Trev

> jeff

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

* Re: [PATCH 0/8] make next_*_insn take rtx_insn * arguments
  2016-09-15 16:42     ` Jeff Law
  2016-09-15 17:42       ` Trevor Saunders
@ 2016-09-16 10:07       ` Trevor Saunders
  2016-09-16 10:21         ` Bernd Schmidt
  2016-09-19 20:55         ` Jeff Law
  1 sibling, 2 replies; 38+ messages in thread
From: Trevor Saunders @ 2016-09-16 10:07 UTC (permalink / raw)
  To: Jeff Law; +Cc: Bernd Schmidt, tbsaunde+gcc, gcc-patches

On Thu, Sep 15, 2016 at 10:27:56AM -0600, Jeff Law wrote:
> On 09/15/2016 10:10 AM, Trevor Saunders wrote:
> > On Thu, Sep 15, 2016 at 12:52:59PM +0200, Bernd Schmidt wrote:
> > > On 09/14/2016 09:21 PM, tbsaunde+gcc@tbsaunde.org wrote:
> > > 
> > > > Basically $subject.  First change variable's type to rtx_insn * where possible.
> > > > Then change the functions and fixup callers where it is still necessary to
> > > > cast.
> > > 
> > > #2, #4 and #8 look good and can be applied if they work independently of the
> > > others.
> > 
> > at most #2 should depend on #1 so it should be fine and I can check on
> > the others.
> > 
> > > Less certain about some of the others which introduce additional casts.
> > 
> > yeah, its somewhat unfortunate, though one way or another we will need
> > to add casts I think the question is just how many we will accept and
> > where.
> In my mind the casts represent the "bounds" of how far we've taken this
> work.  They occur at the boundaries where we haven't converted something
> from an "rtx" to an "rtx_insn *" and allow us to do the work piecemeal
> rather than all-at-once.
> 
> What I don't have a sense of is when we'll be able to push rtx_insn * far
> enough that we're able to start removing casts.
> 
> And that might be a good way to prioritize the next batch of work.  Pick a
> cast, remove it and deal with the fallout.  :-)
> 
> 
> > 
> > > Maybe LABEL_REF_LABEL needs converting first, and reorg.c has a few
> > > variables that might have to be made rtx_insn * in patch #7 to avoid casts.
> > 
> > LABEL_REF_LABEL might be doable, its a good idea I'll look into.  The
> > reorg.c stuff around target_label is rather complicated unfortunately.
> > In the end I of course agree the variables should be rtx_insn *.
> > However currently things are assigned to that variable that are not
> > insns.  So we need to break the variable up, but its involved in a lot
> > of code I don't think I know well enough to really refactor.  For
> > example it looks like target_label can hold a value between iterations
> > of the loop, I suspect that would be a bug, but I'm not really sure.
> I can probably help with reorg.  Hell, you might even be referring to my
> code!

ok, going through all the casts added here I see the following reasons
for them.

- in md files operands is a array of rtx, we should probably have a
  different way to pass insns to the C code here.  That seems worth
  investigating incrementally.

- JUMP_LABEL can be a return which is not an insn.  That also seems
  like something that should be improved at some point.

- tablejump_p returns a label through a rtx * out argument.  I expect
  that isn't hard to fix at this point.

- sh.c uses XEXP (SET_SRC (PATTERN (jump)) 0) with a comment JUMP_LABEL
  might be undefined when not optimizing.  Its not clear to me if that
  is still true.

- var_loc_node::loc is either an expr list or a note, off hand I'm not
  sure what to do with this.

- in reorg.c variables refer to both a insn and other things I think
  this is more results of JUMP_LABEL some times being a return.

I've locally converted LABEL_REF_LABEL, but that only avoids 2 casts.
However it does seem like an improvement so I'll send that shortly.

Trev

> jeff

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

* Re: [PATCH 0/8] make next_*_insn take rtx_insn * arguments
  2016-09-16 10:07       ` Trevor Saunders
@ 2016-09-16 10:21         ` Bernd Schmidt
  2016-09-16 15:13           ` Trevor Saunders
  2016-09-16 15:29           ` Jeff Law
  2016-09-19 20:55         ` Jeff Law
  1 sibling, 2 replies; 38+ messages in thread
From: Bernd Schmidt @ 2016-09-16 10:21 UTC (permalink / raw)
  To: Trevor Saunders, Jeff Law; +Cc: tbsaunde+gcc, gcc-patches

On 09/16/2016 12:10 PM, Trevor Saunders wrote:
> ok, going through all the casts added here I see the following reasons
> for them.
>
> - in md files operands is a array of rtx, we should probably have a
>   different way to pass insns to the C code here.  That seems worth
>   investigating incrementally.
>
> - JUMP_LABEL can be a return which is not an insn.  That also seems
>   like something that should be improved at some point.

These just show that fundamentally, rtl is just dynamically typed, and 
used as such, which is why I was never massively enthusiastic about the 
rtx -> rtx_insn conversion to begin with.


Bernd

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

* Re: [PATCH 0/8] make next_*_insn take rtx_insn * arguments
  2016-09-16 10:21         ` Bernd Schmidt
@ 2016-09-16 15:13           ` Trevor Saunders
  2016-09-16 15:29           ` Jeff Law
  1 sibling, 0 replies; 38+ messages in thread
From: Trevor Saunders @ 2016-09-16 15:13 UTC (permalink / raw)
  To: Bernd Schmidt; +Cc: Jeff Law, tbsaunde+gcc, gcc-patches

On Fri, Sep 16, 2016 at 12:10:51PM +0200, Bernd Schmidt wrote:
> On 09/16/2016 12:10 PM, Trevor Saunders wrote:
> > ok, going through all the casts added here I see the following reasons
> > for them.
> > 
> > - in md files operands is a array of rtx, we should probably have a
> >   different way to pass insns to the C code here.  That seems worth
> >   investigating incrementally.
> > 
> > - JUMP_LABEL can be a return which is not an insn.  That also seems
> >   like something that should be improved at some point.
> 
> These just show that fundamentally, rtl is just dynamically typed, and used
> as such, which is why I was never massively enthusiastic about the rtx ->
> rtx_insn conversion to begin with.

I would agree that rtl is dynamically typed at the moment, and viewing
rtx_insn as an attempt to change that is certainly reasonable.  I don't
know that any of these things mean rtl has to be dynamically typed.  I
think that effort has already helped some things, I wouldn't want to
think about trying to get rid of rtx_insn_list without it.  Further I
expect it will make it possible to change the data structures here more,
and I suspect there is room for cleverness there that isn't possible
with dynamic typing.  Changing away from dynamic typing ccertainly isn't
easy, but I think there's a good amount of evidence it isn't well suited
to large complicated projects.

Alternatively if you take the view that dynamic typing is just a special
case of static typing with one type then adding more types allows you to
reduce the number of places you need to check the "type" of an object.
If we can enable some of what rtl checking gets us with out the compile
time penalty that certainly seems valuable.

Trev

> 
> 
> Bernd

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

* Re: [PATCH 2/8] use rtx_insn * more
  2016-09-14 19:14 ` [PATCH 2/8] use rtx_insn * more tbsaunde+gcc
@ 2016-09-16 15:25   ` Alexander Monakov
  0 siblings, 0 replies; 38+ messages in thread
From: Alexander Monakov @ 2016-09-16 15:25 UTC (permalink / raw)
  To: tbsaunde+gcc; +Cc: gcc-patches

On Wed, 14 Sep 2016, tbsaunde+gcc@tbsaunde.org wrote:
> @@ -3790,7 +3790,8 @@ static void
>  rl78_calculate_death_notes (void)
>  {
>    char dead[FIRST_PSEUDO_REGISTER];
> -  rtx insn, p, s, d;
> +  rtx p, s, d;
> +rtx_insn *insn;

Minor nit: this hunk seems to be missing indentation preceding 'rtx_insn'.

Alexander

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

* Re: [PATCH 0/8] make next_*_insn take rtx_insn * arguments
  2016-09-16 10:21         ` Bernd Schmidt
  2016-09-16 15:13           ` Trevor Saunders
@ 2016-09-16 15:29           ` Jeff Law
  1 sibling, 0 replies; 38+ messages in thread
From: Jeff Law @ 2016-09-16 15:29 UTC (permalink / raw)
  To: Bernd Schmidt, Trevor Saunders; +Cc: tbsaunde+gcc, gcc-patches

On 09/16/2016 04:10 AM, Bernd Schmidt wrote:
> On 09/16/2016 12:10 PM, Trevor Saunders wrote:
>> ok, going through all the casts added here I see the following reasons
>> for them.
>>
>> - in md files operands is a array of rtx, we should probably have a
>>   different way to pass insns to the C code here.  That seems worth
>>   investigating incrementally.
>>
>> - JUMP_LABEL can be a return which is not an insn.  That also seems
>>   like something that should be improved at some point.
>
> These just show that fundamentally, rtl is just dynamically typed, and
> used as such, which is why I was never massively enthusiastic about the
> rtx -> rtx_insn conversion to begin with.
RTL is dynamically typed, but I think we can start carving off things 
like the insn chain objects -- they don't need to be dynamically typed. 
Essentially in my mind the insn chain doesn't need to be RTL.  It's 
really an RTL container.

It does mean there are things we have to fix up, but so far the things 
we've fixed up, I've considered clear improvements.

Jeff

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

* Re: [PATCH 0/8] make next_*_insn take rtx_insn * arguments
  2016-09-16 10:07       ` Trevor Saunders
  2016-09-16 10:21         ` Bernd Schmidt
@ 2016-09-19 20:55         ` Jeff Law
  2016-09-19 22:02           ` Trevor Saunders
  1 sibling, 1 reply; 38+ messages in thread
From: Jeff Law @ 2016-09-19 20:55 UTC (permalink / raw)
  To: Trevor Saunders; +Cc: Bernd Schmidt, tbsaunde+gcc, gcc-patches

On 09/16/2016 04:10 AM, Trevor Saunders wrote:
> On Thu, Sep 15, 2016 at 10:27:56AM -0600, Jeff Law wrote:
>> On 09/15/2016 10:10 AM, Trevor Saunders wrote:
>>> On Thu, Sep 15, 2016 at 12:52:59PM +0200, Bernd Schmidt wrote:
>>>> On 09/14/2016 09:21 PM, tbsaunde+gcc@tbsaunde.org wrote:
>>>>
>>>>> Basically $subject.  First change variable's type to rtx_insn * where possible.
>>>>> Then change the functions and fixup callers where it is still necessary to
>>>>> cast.
>>>>
>>>> #2, #4 and #8 look good and can be applied if they work independently of the
>>>> others.
>>>
>>> at most #2 should depend on #1 so it should be fine and I can check on
>>> the others.
>>>
>>>> Less certain about some of the others which introduce additional casts.
>>>
>>> yeah, its somewhat unfortunate, though one way or another we will need
>>> to add casts I think the question is just how many we will accept and
>>> where.
>> In my mind the casts represent the "bounds" of how far we've taken this
>> work.  They occur at the boundaries where we haven't converted something
>> from an "rtx" to an "rtx_insn *" and allow us to do the work piecemeal
>> rather than all-at-once.
>>
>> What I don't have a sense of is when we'll be able to push rtx_insn * far
>> enough that we're able to start removing casts.
>>
>> And that might be a good way to prioritize the next batch of work.  Pick a
>> cast, remove it and deal with the fallout.  :-)
>>
>>
>>>
>>>> Maybe LABEL_REF_LABEL needs converting first, and reorg.c has a few
>>>> variables that might have to be made rtx_insn * in patch #7 to avoid casts.
>>>
>>> LABEL_REF_LABEL might be doable, its a good idea I'll look into.  The
>>> reorg.c stuff around target_label is rather complicated unfortunately.
>>> In the end I of course agree the variables should be rtx_insn *.
>>> However currently things are assigned to that variable that are not
>>> insns.  So we need to break the variable up, but its involved in a lot
>>> of code I don't think I know well enough to really refactor.  For
>>> example it looks like target_label can hold a value between iterations
>>> of the loop, I suspect that would be a bug, but I'm not really sure.
>> I can probably help with reorg.  Hell, you might even be referring to my
>> code!
>
> ok, going through all the casts added here I see the following reasons
> for them.
>
> - in md files operands is a array of rtx, we should probably have a
>   different way to pass insns to the C code here.  That seems worth
>   investigating incrementally.
Agreed.  Presumably the exception is passing around something like a 
CODE_LABEL?   Or maybe some hackery with passing around a jump table?



>
> - JUMP_LABEL can be a return which is not an insn.  That also seems
>   like something that should be improved at some point.
The JUMP_LABEL field within a JUMP_INSN?  That sounds like a bug.


>
> - tablejump_p returns a label through a rtx * out argument.  I expect
>   that isn't hard to fix at this point.
Right.  Seems like a reasonable follow-up.

>
> - sh.c uses XEXP (SET_SRC (PATTERN (jump)) 0) with a comment JUMP_LABEL
>   might be undefined when not optimizing.  Its not clear to me if that
>   is still true.
The code in question is nearly 19 years old.  What I think Joern was 
referring to here was that in the old days jump.c was responsible for 
setting JUMP_LABEL and that would only happen when optimizing.

JUMP_LABEL was a convenient way to find the jump target of an insn.  It 
didn't matter where the label appeared as an operand.  It was also the 
case that we could derive a label, even though the insn might have been 
an indirect jump.

Anyway...

So instead of relying on JUMP_LABEL he extracts the destination out of 
the pattern (JUMP_LABEL is field outside the insn's pattern).  That 
destination should be a LABEL_REF.  Operand 0 of a LABEL_REF is its 
associated CODE_LABEL.  In the sh.c case, he happens to know precisely 
where the LABEL_REF will be inside the insn's pattern.

This points out one of the little hairballs we're going to want to sort 
out.  Essentially XEXP (X, 0) where X is a LABEL_REF needs to be an 
rtx_insn *.  Right now it's an rtx.

>
> - var_loc_node::loc is either an expr list or a note, off hand I'm not
>   sure what to do with this.
Depending on how it's set we may just want to break this into two fields.

>
> - in reorg.c variables refer to both a insn and other things I think
>   this is more results of JUMP_LABEL some times being a return.
I'd probably want to sit down with a debugger or with a more detailed 
description of when this happens.  I wouldn't be surprised if this was 
just a convenient way of marking jumps which we were later going to turn 
into simple returns.

>
> I've locally converted LABEL_REF_LABEL, but that only avoids 2 casts.
> However it does seem like an improvement so I'll send that shortly.
Sounds good.

jeff

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

* Re: [PATCH 5/8] make prev_real_insn take rtx_insn *
  2016-09-14 19:33 ` [PATCH 5/8] make prev_real_insn " tbsaunde+gcc
@ 2016-09-19 21:08   ` Jeff Law
  0 siblings, 0 replies; 38+ messages in thread
From: Jeff Law @ 2016-09-19 21:08 UTC (permalink / raw)
  To: tbsaunde+gcc, gcc-patches

On 09/14/2016 01:21 PM, tbsaunde+gcc@tbsaunde.org wrote:
> From: Trevor Saunders <tbsaunde+gcc@tbsaunde.org>
>
> gcc/ChangeLog:
>
> 2016-09-13  Trevor Saunders  <tbsaunde+gcc@tbsaunde.org>
>
> 	* emit-rtl.c (prev_real_insn): Change argument type to rtx_insn *.
> 	* rtl.h: Adjust prototype.
> 	* config/sh/sh.md: Adjust.
> 	* dwarf2out.c (add_var_loc_to_decl): Likewise.
> ---
>  gcc/config/sh/sh.md | 3 ++-
>  gcc/dwarf2out.c     | 2 +-
>  gcc/emit-rtl.c      | 4 +---
>  gcc/rtl.h           | 2 +-
>  4 files changed, 5 insertions(+), 6 deletions(-)
>
> diff --git a/gcc/config/sh/sh.md b/gcc/config/sh/sh.md
> index edc4d15..25e03ef 100644
> --- a/gcc/config/sh/sh.md
> +++ b/gcc/config/sh/sh.md
> @@ -7178,7 +7178,8 @@
>  		      (label_ref (match_operand 1 "" ""))))
>     (use (label_ref (match_operand 2 "" "")))]
>    "TARGET_SH2
> -   && (! INSN_UID (operands[1]) || prev_real_insn (operands[1]) == insn)"
> +   && (! INSN_UID (operands[1])
> +       || prev_real_insn (as_a<rtx_insn *> (operands[1])) == insn)"
OK, this is the "we passed an INSN in the operands field" fallout. 
Note how operand 1 is enclosed inside a (label_ref rtx)...

One way to deal with this would be to match the label_ref itself rather 
than the enclosed code_label.  That'd require changing the casesi 
expander, but hopefully not much else.

But that can (IMHO), be a follow-up.

>    "braf	%0%#"
>    [(set_attr "needs_delay_slot" "yes")
>     (set_attr "type" "jump_ind")])
> diff --git a/gcc/dwarf2out.c b/gcc/dwarf2out.c
> index 45eb684..fb8ec7d 100644
> --- a/gcc/dwarf2out.c
> +++ b/gcc/dwarf2out.c
> @@ -5408,7 +5408,7 @@ add_var_loc_to_decl (tree decl, rtx loc_note, const char *label)
>        && NOTE_VAR_LOCATION_LOC (temp->first->loc)
>        && GET_CODE (NOTE_VAR_LOCATION_LOC (temp->first->loc))
>  	 == GET_CODE (DECL_INCOMING_RTL (decl))
> -      && prev_real_insn (temp->first->loc) == NULL_RTX
> +      && prev_real_insn (as_a<rtx_insn *> (temp->first->loc)) == NULL_RTX
Hmm...  Hmmm.  I would think ->loc should always be a 
NOTE_INSN_VAR_LOCATION here.  Ahhh, and indeed look up a couple lines in 
the context, we only get here if NOTE_VAR_LOCATION_LOC 
(temp->first->loc) :-)


So this is OK once the prereqs have gone in.

jeff

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

* Re: [PATCH 6/8] make next/prev nonnote_nondebug_insn take rtx_insn *
  2016-09-14 19:19 ` [PATCH 6/8] make next/prev nonnote_nondebug_insn take rtx_insn * tbsaunde+gcc
@ 2016-09-19 21:09   ` Jeff Law
  0 siblings, 0 replies; 38+ messages in thread
From: Jeff Law @ 2016-09-19 21:09 UTC (permalink / raw)
  To: tbsaunde+gcc, gcc-patches

On 09/14/2016 01:21 PM, tbsaunde+gcc@tbsaunde.org wrote:
> From: Trevor Saunders <tbsaunde+gcc@tbsaunde.org>
>
> gcc/ChangeLog:
>
> 2016-09-14  Trevor Saunders  <tbsaunde+gcc@tbsaunde.org>
>
> 	* config/cris/cris.c (cris_asm_output_case_end): Change argument
> type to rtx_insn *.
> 	* emit-rtl.c (next_nonnote_nondebug_insn): Likewise.
> 	(prev_nonnote_nondebug_insn): Likewise.
> 	* config/cris/cris-protos.h: Adjust prototype.
> 	* rtl.h: Likewise.
> 	* jump.c (rtx_renumbered_equal_p): Adjust.
This is fine once prereqs are approved.  Particularly since you 
indicated LABEL_REF_LABEL may be fixable without major surgery as a 
follow-up.  That presumably would allow removal of the as_a casts in 
rtx_renumbered_equal_p.

jeff

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

* Re: [PATCH 7/8] make next/prev active_insn and active_insn_p take rtx_insn *
  2016-09-14 19:14 ` [PATCH 7/8] make next/prev active_insn and active_insn_p take rtx_insn * tbsaunde+gcc
@ 2016-09-19 21:26   ` Jeff Law
  2016-09-19 23:33     ` Trevor Saunders
  0 siblings, 1 reply; 38+ messages in thread
From: Jeff Law @ 2016-09-19 21:26 UTC (permalink / raw)
  To: tbsaunde+gcc, gcc-patches

On 09/14/2016 01:21 PM, tbsaunde+gcc@tbsaunde.org wrote:
> From: Trevor Saunders <tbsaunde+gcc@tbsaunde.org>
>
> gcc/ChangeLog:
>
> 2016-09-14  Trevor Saunders  <tbsaunde+gcc@tbsaunde.org>
>
> 	* emit-rtl.c (next_active_insn): Change argument type to
> 	rtx_insn *.
> 	(prev_active_insn): Likewise.
> 	(active_insn_p): Likewise.
> 	* rtl.h: Adjust prototypes.
> 	* cfgcleanup.c (merge_blocks_move_successor_nojumps): Adjust.
> 	* config/arc/arc.md: Likewise.
> 	* config/pa/pa.c (branch_to_delay_slot_p): Likewise.
> 	(branch_needs_nop_p): Likewise.
> 	(use_skip_p): Likewise.
> 	* config/sh/sh.c (gen_block_redirect): Likewise.
> 	(split_branches): Likewise.
> 	* reorg.c (optimize_skip): Likewise.
> 	(fill_simple_delay_slots): Likewise.
> 	(fill_slots_from_thread): Likewise.
> 	(relax_delay_slots): Likewise.
> 	* resource.c (mark_target_live_regs): Likewise.
> ---
>  gcc/cfgcleanup.c      |  2 +-
>  gcc/config/arc/arc.md | 33 +++++++++++++++++++++++----------
>  gcc/config/pa/pa.c    |  6 +++---
>  gcc/config/sh/sh.c    |  8 ++++----
>  gcc/emit-rtl.c        | 10 +++-------
>  gcc/reorg.c           | 29 +++++++++++++++++------------
>  gcc/resource.c        |  2 +-
>  gcc/rtl.h             |  6 +++---
>  8 files changed, 55 insertions(+), 41 deletions(-)
>
> diff --git a/gcc/cfgcleanup.c b/gcc/cfgcleanup.c
> index 023b9d2..2e2a635 100644
> --- a/gcc/cfgcleanup.c
> +++ b/gcc/cfgcleanup.c
> @@ -708,7 +708,7 @@ merge_blocks_move_successor_nojumps (basic_block a, basic_block b)
>    /* If there is a jump table following block B temporarily add the jump table
>       to block B so that it will also be moved to the correct location.  */
>    if (tablejump_p (BB_END (b), &label, &table)
> -      && prev_active_insn (label) == BB_END (b))
> +      && prev_active_insn (as_a<rtx_insn *> (label)) == BB_END (b))
Looking at tablejump_p, there seems to be a belief that JUMP_LABEL could 
be a RETURN (as you mentioned in a latter message).  That seems like 
something we should tackle, but I think that can happen independently.

Presumably if we get a RETURN or SIMPLE_RETURN for LABEL the as_a will 
fail because it's not something that can be converted into an rtx_insn *.


  diff --git a/gcc/config/sh/sh.c b/gcc/config/sh/sh.c
> index b3e949e..a9b5a14 100644
> --- a/gcc/config/sh/sh.c
> +++ b/gcc/config/sh/sh.c
> @@ -5503,7 +5503,8 @@ gen_block_redirect (rtx_insn *jump, int addr, int need_block)
>
>    else if (optimize && need_block >= 0)
>      {
> -      rtx_insn *next = next_active_insn (next_active_insn (dest));
> +      rtx_insn *next = next_active_insn (as_a<rtx_insn *> (dest));
It may be better to fix how we initialize "dest" to ensure it's an 
rtx_insn *.  Essentially this:

   rtx dest = XEXP (SET_SRC (PATTERN (jump)), 0);

turns into:

   rtx temp = XEXP (SET_SRC (PATTERN (jump)), 0);
   rtx_insn *dest = as_a<rtx_insn *> (temp));

But presumably we can't do this until INSN_UID (and perhaps other stuff) 
is changed to accept an rtx_insn *.

So that's future work.


> @@ -2575,7 +2575,8 @@ fill_slots_from_thread (rtx_jump_insn *insn, rtx condition,
>  			     to call update_block and delete_insn.  */
>  			  fix_reg_dead_note (prior_insn, insn);
>  			  update_reg_unused_notes (prior_insn, new_thread);
> -			  new_thread = next_active_insn (new_thread);
> +			  new_thread
> +			    = next_active_insn (as_a<rtx_insn *> (new_thread));
Probably can't avoid the cast for now without other surgery.  Nothing 
that should be terrible, but I don't think you need to pull on the that 
thread (pun intended) for this patch.

In fact, I'm generally OK with adding the casts in reorg.c until fairly 
late in this process :-)

diff --git a/gcc/resource.c b/gcc/resource.c
> index ae2f5d8..1d7ce95 100644
> --- a/gcc/resource.c
> +++ b/gcc/resource.c
> @@ -1122,7 +1122,7 @@ mark_target_live_regs (rtx_insn *insns, rtx target_maybe_return, struct resource
>        rtx_insn *stop_insn = next_active_insn (jump_insn);
>
>        if (!ANY_RETURN_P (jump_target))
> -	jump_target = next_active_insn (jump_target);
> +	jump_target = next_active_insn (as_a<rtx_insn *> (jump_target));
Right.  jump_target here can only be an rtx_insn * because of 
controlling condition.  But clearly this is something we'll want to 
clean up later.

I think this is fine once prereqs are approved.

jeff

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

* Re: [PATCH 3/8] make next/prev _nonnote_insn take rtx_insn *
  2016-09-14 19:14 ` [PATCH 3/8] make next/prev _nonnote_insn " tbsaunde+gcc
@ 2016-09-19 21:27   ` Jeff Law
  2016-09-19 23:29     ` Trevor Saunders
  0 siblings, 1 reply; 38+ messages in thread
From: Jeff Law @ 2016-09-19 21:27 UTC (permalink / raw)
  To: tbsaunde+gcc, gcc-patches

On 09/14/2016 01:21 PM, tbsaunde+gcc@tbsaunde.org wrote:
> From: Trevor Saunders <tbsaunde+gcc@tbsaunde.org>
>
> gcc/ChangeLog:
>
> 2016-09-13  Trevor Saunders  <tbsaunde+gcc@tbsaunde.org>
>
> 	* emit-rtl.c (next_nonnote_insn): Change argument type to
> 	rtx_insn *.
> 	(prev_nonnote_insn): Likewise.
> 	* jump.c (reversed_comparison_code_parts): Likewise.
> 	(reversed_comparison): Likewise.
> 	* rtl.h: Adjust prototypes.
> 	* config/arc/arc.md: Adjust.
> 	* cse.c (find_comparison_args): Likewise.
> 	* reorg.c (redundant_insn): Change return type to rtx_insn *.
> 	(fix_reg_dead_note): Change argument type to rtx_insn *.
> 	(delete_prior_computation): Likewise.
> 	(delete_computation): Likewise.
> 	(fill_slots_from_thread): Adjust.
> 	(relax_delay_slots): Likewise.
> 	* simplify-rtx.c (simplify_unary_operation_1): Likewise.
> 	(simplify_relational_operation_1): Likewise.
> 	(simplify_ternary_operation): Likewise.
The arc bits are similar in nature to the sh bits in how they match on 
the CODE_LABEL rather than on the enclosing LABEL_REF.  I think cleaning 
up those warts can be deferred.


OK once all prereqs are approved.

I don't see any more patches in this series in my inbox.  Is that 
consistent with your tracking?

jeff

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

* Re: [PATCH 0/8] make next_*_insn take rtx_insn * arguments
  2016-09-19 20:55         ` Jeff Law
@ 2016-09-19 22:02           ` Trevor Saunders
  2016-09-19 23:21             ` Segher Boessenkool
  0 siblings, 1 reply; 38+ messages in thread
From: Trevor Saunders @ 2016-09-19 22:02 UTC (permalink / raw)
  To: Jeff Law; +Cc: Bernd Schmidt, tbsaunde+gcc, gcc-patches

On Mon, Sep 19, 2016 at 02:49:49PM -0600, Jeff Law wrote:
> On 09/16/2016 04:10 AM, Trevor Saunders wrote:
> > On Thu, Sep 15, 2016 at 10:27:56AM -0600, Jeff Law wrote:
> > > On 09/15/2016 10:10 AM, Trevor Saunders wrote:
> > > > On Thu, Sep 15, 2016 at 12:52:59PM +0200, Bernd Schmidt wrote:
> > > > > On 09/14/2016 09:21 PM, tbsaunde+gcc@tbsaunde.org wrote:
> > > > > 
> > > > > > Basically $subject.  First change variable's type to rtx_insn * where possible.
> > > > > > Then change the functions and fixup callers where it is still necessary to
> > > > > > cast.
> > > > > 
> > > > > #2, #4 and #8 look good and can be applied if they work independently of the
> > > > > others.
> > > > 
> > > > at most #2 should depend on #1 so it should be fine and I can check on
> > > > the others.
> > > > 
> > > > > Less certain about some of the others which introduce additional casts.
> > > > 
> > > > yeah, its somewhat unfortunate, though one way or another we will need
> > > > to add casts I think the question is just how many we will accept and
> > > > where.
> > > In my mind the casts represent the "bounds" of how far we've taken this
> > > work.  They occur at the boundaries where we haven't converted something
> > > from an "rtx" to an "rtx_insn *" and allow us to do the work piecemeal
> > > rather than all-at-once.
> > > 
> > > What I don't have a sense of is when we'll be able to push rtx_insn * far
> > > enough that we're able to start removing casts.
> > > 
> > > And that might be a good way to prioritize the next batch of work.  Pick a
> > > cast, remove it and deal with the fallout.  :-)
> > > 
> > > 
> > > > 
> > > > > Maybe LABEL_REF_LABEL needs converting first, and reorg.c has a few
> > > > > variables that might have to be made rtx_insn * in patch #7 to avoid casts.
> > > > 
> > > > LABEL_REF_LABEL might be doable, its a good idea I'll look into.  The
> > > > reorg.c stuff around target_label is rather complicated unfortunately.
> > > > In the end I of course agree the variables should be rtx_insn *.
> > > > However currently things are assigned to that variable that are not
> > > > insns.  So we need to break the variable up, but its involved in a lot
> > > > of code I don't think I know well enough to really refactor.  For
> > > > example it looks like target_label can hold a value between iterations
> > > > of the loop, I suspect that would be a bug, but I'm not really sure.
> > > I can probably help with reorg.  Hell, you might even be referring to my
> > > code!
> > 
> > ok, going through all the casts added here I see the following reasons
> > for them.
> > 
> > - in md files operands is a array of rtx, we should probably have a
> >   different way to pass insns to the C code here.  That seems worth
> >   investigating incrementally.
> Agreed.  Presumably the exception is passing around something like a
> CODE_LABEL?   Or maybe some hackery with passing around a jump table?

 I took a quick sample of the casts in .md files.  It looks like in most
 if not all cases we have something like label_ref (match_operand 3 ""
 "") then we use operands[3] somewhere that expects a rtx_insn *.  So I
 guess it would make sense to add a match_label and add some magic to
 genrecog to populate a label_operands with the matched labels.

> > 
> > - JUMP_LABEL can be a return which is not an insn.  That also seems
> >   like something that should be improved at some point.
> The JUMP_LABEL field within a JUMP_INSN?  That sounds like a bug.

yes, see the comment before the declaration of rtx_jump_insn::jump_label ()
it can be either a possibly deleted label, or a RETURN or SIMPLE_RETURN
expression.  Memory usage concerns aside its a tempting design to
change, but at the moment itts definitely intended to work this way,
there's plenty of code that checks ANY_RETURN_P (JUMP_LABEL (x)).

> > - tablejump_p returns a label through a rtx * out argument.  I expect
> >   that isn't hard to fix at this point.
> Right.  Seems like a reasonable follow-up.

yeah, I did it locally, so I just need to split out the parts of this
queue we can all agree should go in.

> > 
> > - sh.c uses XEXP (SET_SRC (PATTERN (jump)) 0) with a comment JUMP_LABEL
> >   might be undefined when not optimizing.  Its not clear to me if that
> >   is still true.
> The code in question is nearly 19 years old.  What I think Joern was
> referring to here was that in the old days jump.c was responsible for
> setting JUMP_LABEL and that would only happen when optimizing.
> 
> JUMP_LABEL was a convenient way to find the jump target of an insn.  It
> didn't matter where the label appeared as an operand.  It was also the case
> that we could derive a label, even though the insn might have been an
> indirect jump.
> 
> Anyway...
> 
> So instead of relying on JUMP_LABEL he extracts the destination out of the
> pattern (JUMP_LABEL is field outside the insn's pattern).  That destination
> should be a LABEL_REF.  Operand 0 of a LABEL_REF is its associated
> CODE_LABEL.  In the sh.c case, he happens to know precisely where the
> LABEL_REF will be inside the insn's pattern.
> 
> This points out one of the little hairballs we're going to want to sort out.
> Essentially XEXP (X, 0) where X is a LABEL_REF needs to be an rtx_insn *.
> Right now it's an rtx.

yeah, I suspect the answer should be that the union has a field for
insns as well as for rtx, and we use XINSN instead of XEXP here.
Ultimmitly in any data structure that stores multiple things in one
place you need to cast somewhere.

> > 
> > - var_loc_node::loc is either an expr list or a note, off hand I'm not
> >   sure what to do with this.
> Depending on how it's set we may just want to break this into two fields.

yeah, I haven't looked in depth yet.

> > 
> > - in reorg.c variables refer to both a insn and other things I think
> >   this is more results of JUMP_LABEL some times being a return.
> I'd probably want to sit down with a debugger or with a more detailed
> description of when this happens.  I wouldn't be surprised if this was just
> a convenient way of marking jumps which we were later going to turn into
> simple returns.

sure, unfortunately I'm not sure I have a simple example of when it
happens on hand.  However I've definitely seen it when forgeting aout
checking ANY_RETURN_P on the result of JUMP_LABEL, I may even have seen
it on x86 somewhere other than reorg.c.

Trev

> > 
> > I've locally converted LABEL_REF_LABEL, but that only avoids 2 casts.
> > However it does seem like an improvement so I'll send that shortly.
> Sounds good.
> 
> jeff

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

* Re: [PATCH 0/8] make next_*_insn take rtx_insn * arguments
  2016-09-19 22:02           ` Trevor Saunders
@ 2016-09-19 23:21             ` Segher Boessenkool
  2016-09-20 11:31               ` Bernd Schmidt
  0 siblings, 1 reply; 38+ messages in thread
From: Segher Boessenkool @ 2016-09-19 23:21 UTC (permalink / raw)
  To: Trevor Saunders; +Cc: Jeff Law, Bernd Schmidt, tbsaunde+gcc, gcc-patches

On Mon, Sep 19, 2016 at 06:07:46PM -0400, Trevor Saunders wrote:
> > > - JUMP_LABEL can be a return which is not an insn.  That also seems
> > >   like something that should be improved at some point.
> > The JUMP_LABEL field within a JUMP_INSN?  That sounds like a bug.
> 
> yes, see the comment before the declaration of rtx_jump_insn::jump_label ()
> it can be either a possibly deleted label, or a RETURN or SIMPLE_RETURN
> expression.  Memory usage concerns aside its a tempting design to
> change, but at the moment itts definitely intended to work this way,
> there's plenty of code that checks ANY_RETURN_P (JUMP_LABEL (x)).

Yes.  But rtl.texi still says:

  Return insns count as jumps, but since they do not refer to any
  labels, their @code{JUMP_LABEL} is @code{NULL_RTX}.


Segher

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

* Re: [PATCH 3/8] make next/prev _nonnote_insn take rtx_insn *
  2016-09-19 21:27   ` Jeff Law
@ 2016-09-19 23:29     ` Trevor Saunders
  0 siblings, 0 replies; 38+ messages in thread
From: Trevor Saunders @ 2016-09-19 23:29 UTC (permalink / raw)
  To: Jeff Law; +Cc: tbsaunde+gcc, gcc-patches

On Mon, Sep 19, 2016 at 03:23:58PM -0600, Jeff Law wrote:
> On 09/14/2016 01:21 PM, tbsaunde+gcc@tbsaunde.org wrote:
> > From: Trevor Saunders <tbsaunde+gcc@tbsaunde.org>
> > 
> > gcc/ChangeLog:
> > 
> > 2016-09-13  Trevor Saunders  <tbsaunde+gcc@tbsaunde.org>
> > 
> > 	* emit-rtl.c (next_nonnote_insn): Change argument type to
> > 	rtx_insn *.
> > 	(prev_nonnote_insn): Likewise.
> > 	* jump.c (reversed_comparison_code_parts): Likewise.
> > 	(reversed_comparison): Likewise.
> > 	* rtl.h: Adjust prototypes.
> > 	* config/arc/arc.md: Adjust.
> > 	* cse.c (find_comparison_args): Likewise.
> > 	* reorg.c (redundant_insn): Change return type to rtx_insn *.
> > 	(fix_reg_dead_note): Change argument type to rtx_insn *.
> > 	(delete_prior_computation): Likewise.
> > 	(delete_computation): Likewise.
> > 	(fill_slots_from_thread): Adjust.
> > 	(relax_delay_slots): Likewise.
> > 	* simplify-rtx.c (simplify_unary_operation_1): Likewise.
> > 	(simplify_relational_operation_1): Likewise.
> > 	(simplify_ternary_operation): Likewise.
> The arc bits are similar in nature to the sh bits in how they match on the
> CODE_LABEL rather than on the enclosing LABEL_REF.  I think cleaning up
> those warts can be deferred.
> 
> 
> OK once all prereqs are approved.
> 
> I don't see any more patches in this series in my inbox.  Is that consistent
> with your tracking?

I think the only thing not approved at this point is 1/8, which needs a
fixup that was discussed a little while ago, and some retesting for
that.

Thanks for the reviews!

Trev

> 
> jeff

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

* Re: [PATCH 7/8] make next/prev active_insn and active_insn_p take rtx_insn *
  2016-09-19 21:26   ` Jeff Law
@ 2016-09-19 23:33     ` Trevor Saunders
  0 siblings, 0 replies; 38+ messages in thread
From: Trevor Saunders @ 2016-09-19 23:33 UTC (permalink / raw)
  To: Jeff Law; +Cc: tbsaunde+gcc, gcc-patches

On Mon, Sep 19, 2016 at 03:17:33PM -0600, Jeff Law wrote:
> On 09/14/2016 01:21 PM, tbsaunde+gcc@tbsaunde.org wrote:
> > From: Trevor Saunders <tbsaunde+gcc@tbsaunde.org>
> > 
> > gcc/ChangeLog:
> > 
> > 2016-09-14  Trevor Saunders  <tbsaunde+gcc@tbsaunde.org>
> > 
> > 	* emit-rtl.c (next_active_insn): Change argument type to
> > 	rtx_insn *.
> > 	(prev_active_insn): Likewise.
> > 	(active_insn_p): Likewise.
> > 	* rtl.h: Adjust prototypes.
> > 	* cfgcleanup.c (merge_blocks_move_successor_nojumps): Adjust.
> > 	* config/arc/arc.md: Likewise.
> > 	* config/pa/pa.c (branch_to_delay_slot_p): Likewise.
> > 	(branch_needs_nop_p): Likewise.
> > 	(use_skip_p): Likewise.
> > 	* config/sh/sh.c (gen_block_redirect): Likewise.
> > 	(split_branches): Likewise.
> > 	* reorg.c (optimize_skip): Likewise.
> > 	(fill_simple_delay_slots): Likewise.
> > 	(fill_slots_from_thread): Likewise.
> > 	(relax_delay_slots): Likewise.
> > 	* resource.c (mark_target_live_regs): Likewise.
> > ---
> >  gcc/cfgcleanup.c      |  2 +-
> >  gcc/config/arc/arc.md | 33 +++++++++++++++++++++++----------
> >  gcc/config/pa/pa.c    |  6 +++---
> >  gcc/config/sh/sh.c    |  8 ++++----
> >  gcc/emit-rtl.c        | 10 +++-------
> >  gcc/reorg.c           | 29 +++++++++++++++++------------
> >  gcc/resource.c        |  2 +-
> >  gcc/rtl.h             |  6 +++---
> >  8 files changed, 55 insertions(+), 41 deletions(-)
> > 
> > diff --git a/gcc/cfgcleanup.c b/gcc/cfgcleanup.c
> > index 023b9d2..2e2a635 100644
> > --- a/gcc/cfgcleanup.c
> > +++ b/gcc/cfgcleanup.c
> > @@ -708,7 +708,7 @@ merge_blocks_move_successor_nojumps (basic_block a, basic_block b)
> >    /* If there is a jump table following block B temporarily add the jump table
> >       to block B so that it will also be moved to the correct location.  */
> >    if (tablejump_p (BB_END (b), &label, &table)
> > -      && prev_active_insn (label) == BB_END (b))
> > +      && prev_active_insn (as_a<rtx_insn *> (label)) == BB_END (b))
> Looking at tablejump_p, there seems to be a belief that JUMP_LABEL could be
> a RETURN (as you mentioned in a latter message).  That seems like something
> we should tackle, but I think that can happen independently.
> 
> Presumably if we get a RETURN or SIMPLE_RETURN for LABEL the as_a will fail
> because it's not something that can be converted into an rtx_insn *.

tablejump_p checks for that and only returns an actually label.  Which
is good because as_a would assert now in the caller, but previously it
would inside prev_active_insn.

> 
> 
>  diff --git a/gcc/config/sh/sh.c b/gcc/config/sh/sh.c
> > index b3e949e..a9b5a14 100644
> > --- a/gcc/config/sh/sh.c
> > +++ b/gcc/config/sh/sh.c
> > @@ -5503,7 +5503,8 @@ gen_block_redirect (rtx_insn *jump, int addr, int need_block)
> > 
> >    else if (optimize && need_block >= 0)
> >      {
> > -      rtx_insn *next = next_active_insn (next_active_insn (dest));
> > +      rtx_insn *next = next_active_insn (as_a<rtx_insn *> (dest));
> It may be better to fix how we initialize "dest" to ensure it's an rtx_insn
> *.  Essentially this:
> 
>   rtx dest = XEXP (SET_SRC (PATTERN (jump)), 0);
> 
> turns into:
> 
>   rtx temp = XEXP (SET_SRC (PATTERN (jump)), 0);
>   rtx_insn *dest = as_a<rtx_insn *> (temp));
> 
> But presumably we can't do this until INSN_UID (and perhaps other stuff) is
> changed to accept an rtx_insn *.

actually that part is fine, rtx_insn * can implicitly convert to rtx,
the problem is a few lines down we assign JUMP_LABEL (next) to
dest, which  could certainly be changed, but it seemed better to do the
simple thing for now.

> So that's future work.
> 
> 
> > @@ -2575,7 +2575,8 @@ fill_slots_from_thread (rtx_jump_insn *insn, rtx condition,
> >  			     to call update_block and delete_insn.  */
> >  			  fix_reg_dead_note (prior_insn, insn);
> >  			  update_reg_unused_notes (prior_insn, new_thread);
> > -			  new_thread = next_active_insn (new_thread);
> > +			  new_thread
> > +			    = next_active_insn (as_a<rtx_insn *> (new_thread));
> Probably can't avoid the cast for now without other surgery.  Nothing that
> should be terrible, but I don't think you need to pull on the that thread
> (pun intended) for this patch.

heh, that's my impression too.

> 
> In fact, I'm generally OK with adding the casts in reorg.c until fairly late
> in this process :-)
> 
> diff --git a/gcc/resource.c b/gcc/resource.c
> > index ae2f5d8..1d7ce95 100644
> > --- a/gcc/resource.c
> > +++ b/gcc/resource.c
> > @@ -1122,7 +1122,7 @@ mark_target_live_regs (rtx_insn *insns, rtx target_maybe_return, struct resource
> >        rtx_insn *stop_insn = next_active_insn (jump_insn);
> > 
> >        if (!ANY_RETURN_P (jump_target))
> > -	jump_target = next_active_insn (jump_target);
> > +	jump_target = next_active_insn (as_a<rtx_insn *> (jump_target));
> Right.  jump_target here can only be an rtx_insn * because of controlling
> condition.  But clearly this is something we'll want to clean up later.

presumably once we figure out what we want to do about JUMP_LABEL not
always being a label, and instead being a RETURN / SIMPLE_RETURN and do
that this will get cleaned up in the process.

Trev

> 
> I think this is fine once prereqs are approved.
> 
> jeff

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

* Re: [PATCH 0/8] make next_*_insn take rtx_insn * arguments
  2016-09-19 23:21             ` Segher Boessenkool
@ 2016-09-20 11:31               ` Bernd Schmidt
  2016-09-20 14:54                 ` Jeff Law
  0 siblings, 1 reply; 38+ messages in thread
From: Bernd Schmidt @ 2016-09-20 11:31 UTC (permalink / raw)
  To: Segher Boessenkool, Trevor Saunders; +Cc: Jeff Law, tbsaunde+gcc, gcc-patches



On 09/20/2016 01:16 AM, Segher Boessenkool wrote:
> On Mon, Sep 19, 2016 at 06:07:46PM -0400, Trevor Saunders wrote:
>>>> - JUMP_LABEL can be a return which is not an insn.  That also seems
>>>>   like something that should be improved at some point.
>>> The JUMP_LABEL field within a JUMP_INSN?  That sounds like a bug.
>>
>> yes, see the comment before the declaration of rtx_jump_insn::jump_label ()
>> it can be either a possibly deleted label, or a RETURN or SIMPLE_RETURN
>> expression.  Memory usage concerns aside its a tempting design to
>> change, but at the moment itts definitely intended to work this way,
>> there's plenty of code that checks ANY_RETURN_P (JUMP_LABEL (x)).
>
> Yes.  But rtl.texi still says:
>
>   Return insns count as jumps, but since they do not refer to any
>   labels, their @code{JUMP_LABEL} is @code{NULL_RTX}.

And the comment at the top of jump.c:

    Each CODE_LABEL has a count of the times it is used
    stored in the LABEL_NUSES internal field, and each JUMP_INSN
    has one label that it refers to stored in the
    JUMP_LABEL internal field.  With this we can detect labels that
    become unused because of the deletion of all the jumps that
    formerly used them.  The JUMP_LABEL info is sometimes looked
    at by later passes.  For return insns, it contains either a
    RETURN or a SIMPLE_RETURN rtx.

It's intentional, and really there's nothing wrong with it, or with 
operands[] containing CODE_LABELs. The problem is trying to force a 
static type system onto a dynamically typed data structure.


Bernd

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

* Re: [PATCH 0/8] make next_*_insn take rtx_insn * arguments
  2016-09-20 11:31               ` Bernd Schmidt
@ 2016-09-20 14:54                 ` Jeff Law
  2016-09-20 17:13                   ` Segher Boessenkool
  0 siblings, 1 reply; 38+ messages in thread
From: Jeff Law @ 2016-09-20 14:54 UTC (permalink / raw)
  To: Bernd Schmidt, Segher Boessenkool, Trevor Saunders
  Cc: tbsaunde+gcc, gcc-patches

On 09/20/2016 04:38 AM, Bernd Schmidt wrote:
>
>
> On 09/20/2016 01:16 AM, Segher Boessenkool wrote:
>> On Mon, Sep 19, 2016 at 06:07:46PM -0400, Trevor Saunders wrote:
>>>>> - JUMP_LABEL can be a return which is not an insn.  That also seems
>>>>>   like something that should be improved at some point.
>>>> The JUMP_LABEL field within a JUMP_INSN?  That sounds like a bug.
>>>
>>> yes, see the comment before the declaration of
>>> rtx_jump_insn::jump_label ()
>>> it can be either a possibly deleted label, or a RETURN or SIMPLE_RETURN
>>> expression.  Memory usage concerns aside its a tempting design to
>>> change, but at the moment itts definitely intended to work this way,
>>> there's plenty of code that checks ANY_RETURN_P (JUMP_LABEL (x)).
>>
>> Yes.  But rtl.texi still says:
>>
>>   Return insns count as jumps, but since they do not refer to any
>>   labels, their @code{JUMP_LABEL} is @code{NULL_RTX}.
>
> And the comment at the top of jump.c:
>
>    Each CODE_LABEL has a count of the times it is used
>    stored in the LABEL_NUSES internal field, and each JUMP_INSN
>    has one label that it refers to stored in the
>    JUMP_LABEL internal field.  With this we can detect labels that
>    become unused because of the deletion of all the jumps that
>    formerly used them.  The JUMP_LABEL info is sometimes looked
>    at by later passes.  For return insns, it contains either a
>    RETURN or a SIMPLE_RETURN rtx.
>
> It's intentional, and really there's nothing wrong with it, or with
> operands[] containing CODE_LABELs. The problem is trying to force a
> static type system onto a dynamically typed data structure.
But what this doesn't answer is why we stuff RETURN/SIMPLE_RETURN into 
the JUMP_LABEL field of a return.  I simply can't remember any rationale 
behind that.

I suspect that if we dig further, we'll find that we can accomplish 
whatever the goal was behind that decision in a way that easily works in 
a world where we have separated rtx objects from objects that are part 
of the insn chain.

Just to be clear, I don't see us going to a world where everything we 
have as an rtx today is a statically typed object. But there are things 
that I think make sense to carve out, the most obvious being objects 
which are part of the insn chain.

jeff

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

* Re: [PATCH 0/8] make next_*_insn take rtx_insn * arguments
  2016-09-20 14:54                 ` Jeff Law
@ 2016-09-20 17:13                   ` Segher Boessenkool
  2016-09-20 17:56                     ` Jeff Law
  2016-09-20 19:27                     ` [PATCH] Fix JUMP_LABEL documentation Segher Boessenkool
  0 siblings, 2 replies; 38+ messages in thread
From: Segher Boessenkool @ 2016-09-20 17:13 UTC (permalink / raw)
  To: Jeff Law; +Cc: Bernd Schmidt, Trevor Saunders, tbsaunde+gcc, gcc-patches

On Tue, Sep 20, 2016 at 08:52:46AM -0600, Jeff Law wrote:
> >>>>>- JUMP_LABEL can be a return which is not an insn.  That also seems
> >>>>>  like something that should be improved at some point.
> >>>>The JUMP_LABEL field within a JUMP_INSN?  That sounds like a bug.
> >>>
> >>>yes, see the comment before the declaration of
> >>>rtx_jump_insn::jump_label ()
> >>>it can be either a possibly deleted label, or a RETURN or SIMPLE_RETURN
> >>>expression.  Memory usage concerns aside its a tempting design to
> >>>change, but at the moment itts definitely intended to work this way,
> >>>there's plenty of code that checks ANY_RETURN_P (JUMP_LABEL (x)).
> >>
> >>Yes.  But rtl.texi still says:
> >>
> >>  Return insns count as jumps, but since they do not refer to any
> >>  labels, their @code{JUMP_LABEL} is @code{NULL_RTX}.
> >
> >And the comment at the top of jump.c:
> >
> >   Each CODE_LABEL has a count of the times it is used
> >   stored in the LABEL_NUSES internal field, and each JUMP_INSN
> >   has one label that it refers to stored in the
> >   JUMP_LABEL internal field.  With this we can detect labels that
> >   become unused because of the deletion of all the jumps that
> >   formerly used them.  The JUMP_LABEL info is sometimes looked
> >   at by later passes.  For return insns, it contains either a
> >   RETURN or a SIMPLE_RETURN rtx.
> >
> >It's intentional, and really there's nothing wrong with it, or with
> >operands[] containing CODE_LABELs. The problem is trying to force a

I'm just pointing out the documentation is out-of-date here.  I'll do
a patch.

> >static type system onto a dynamically typed data structure.
> But what this doesn't answer is why we stuff RETURN/SIMPLE_RETURN into 
> the JUMP_LABEL field of a return.  I simply can't remember any rationale 
> behind that.

It is very inconvenient to parse the whole rtx to see if this is a
RETURN vs. a SIMPLE_RETURN (it can be inside a conditional or inside
a PARALLEL, etc.)

Ideally we would not have RETURN at all anymore, just SIMPLE_RETURN,
but that isn't going to happen any time soon.

> I suspect that if we dig further, we'll find that we can accomplish 
> whatever the goal was behind that decision in a way that easily works in 
> a world where we have separated rtx objects from objects that are part 
> of the insn chain.

We cannot easily extract the JUMP_LABEL of a normal jump from its rtx
pattern either (if at all).  So it seems the extra field of a JUMP_INSN
is here to stay.  We'll have to figure out how to nicely do INSN vs.
JUMP_INSN in an rtx_insn world.

> Just to be clear, I don't see us going to a world where everything we 
> have as an rtx today is a statically typed object. But there are things 
> that I think make sense to carve out, the most obvious being objects 
> which are part of the insn chain.

Agreed on both.  I have nightmares about the first, so please don't even
talk about going there :-)


Segher

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

* Re: [PATCH 0/8] make next_*_insn take rtx_insn * arguments
  2016-09-20 17:13                   ` Segher Boessenkool
@ 2016-09-20 17:56                     ` Jeff Law
  2016-09-20 19:27                     ` [PATCH] Fix JUMP_LABEL documentation Segher Boessenkool
  1 sibling, 0 replies; 38+ messages in thread
From: Jeff Law @ 2016-09-20 17:56 UTC (permalink / raw)
  To: Segher Boessenkool
  Cc: Bernd Schmidt, Trevor Saunders, tbsaunde+gcc, gcc-patches

On 09/20/2016 11:11 AM, Segher Boessenkool wrote:
> On Tue, Sep 20, 2016 at 08:52:46AM -0600, Jeff Law wrote:
>>>>>>> - JUMP_LABEL can be a return which is not an insn.  That also seems
>>>>>>>  like something that should be improved at some point.
>>>>>> The JUMP_LABEL field within a JUMP_INSN?  That sounds like a bug.
>>>>>
>>>>> yes, see the comment before the declaration of
>>>>> rtx_jump_insn::jump_label ()
>>>>> it can be either a possibly deleted label, or a RETURN or SIMPLE_RETURN
>>>>> expression.  Memory usage concerns aside its a tempting design to
>>>>> change, but at the moment itts definitely intended to work this way,
>>>>> there's plenty of code that checks ANY_RETURN_P (JUMP_LABEL (x)).
>>>>
>>>> Yes.  But rtl.texi still says:
>>>>
>>>>  Return insns count as jumps, but since they do not refer to any
>>>>  labels, their @code{JUMP_LABEL} is @code{NULL_RTX}.
>>>
>>> And the comment at the top of jump.c:
>>>
>>>   Each CODE_LABEL has a count of the times it is used
>>>   stored in the LABEL_NUSES internal field, and each JUMP_INSN
>>>   has one label that it refers to stored in the
>>>   JUMP_LABEL internal field.  With this we can detect labels that
>>>   become unused because of the deletion of all the jumps that
>>>   formerly used them.  The JUMP_LABEL info is sometimes looked
>>>   at by later passes.  For return insns, it contains either a
>>>   RETURN or a SIMPLE_RETURN rtx.
>>>
>>> It's intentional, and really there's nothing wrong with it, or with
>>> operands[] containing CODE_LABELs. The problem is trying to force a
>
> I'm just pointing out the documentation is out-of-date here.  I'll do
> a patch.
>
>>> static type system onto a dynamically typed data structure.
>> But what this doesn't answer is why we stuff RETURN/SIMPLE_RETURN into
>> the JUMP_LABEL field of a return.  I simply can't remember any rationale
>> behind that.
>
> It is very inconvenient to parse the whole rtx to see if this is a
> RETURN vs. a SIMPLE_RETURN (it can be inside a conditional or inside
> a PARALLEL, etc.)
So it's similar to how we use JUMP_LABEL to find the jump target without 
having to dig through the whole rtx.


>
> Ideally we would not have RETURN at all anymore, just SIMPLE_RETURN,
> but that isn't going to happen any time soon.
And it wouldn't totally resolve this anyway.

>
>> I suspect that if we dig further, we'll find that we can accomplish
>> whatever the goal was behind that decision in a way that easily works in
>> a world where we have separated rtx objects from objects that are part
>> of the insn chain.
>
> We cannot easily extract the JUMP_LABEL of a normal jump from its rtx
> pattern either (if at all).  So it seems the extra field of a JUMP_INSN
> is here to stay.  We'll have to figure out how to nicely do INSN vs.
> JUMP_INSN in an rtx_insn world.
Both INSN and JUMP_INSNs are rtx_insn *s.

The problem is we're stuffing a (return) or (simple_return) rtx into a 
field that most of the time has an rtx_insn * (CODE_LABEL).

A hack-ish way to address this would be to create special CODE_LABELs 
that represent (return) and (simple_return) and stuff that into the 
JUMP_LABEL field.  At which point the JUMP_LABEL field should turn into 
an rtx_insn * and the as_a casts related to this wart go away.

There may be cleaner ways, but there's certainly ways to move forward here.

>
>> Just to be clear, I don't see us going to a world where everything we
>> have as an rtx today is a statically typed object. But there are things
>> that I think make sense to carve out, the most obvious being objects
>> which are part of the insn chain.
>
> Agreed on both.  I have nightmares about the first, so please don't even
> talk about going there :-)
While I could see some value in static typing enough to allow static 
checking things like we don't try to access out-of-range operands (say 
XEXP (x, 1)) where X is a unary code.  I don't see the level of pain to 
get there being worth it.

It's much like what we see in the tree world.  There's value in 
separating _DECL, _TYPE and _EXPR nodes.  But going deeper into in the 
_EXPR nodes seems like a huge level of pain.

jeff


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

* [PATCH] Fix JUMP_LABEL documentation
  2016-09-20 17:13                   ` Segher Boessenkool
  2016-09-20 17:56                     ` Jeff Law
@ 2016-09-20 19:27                     ` Segher Boessenkool
  2016-09-21 13:04                       ` Bernd Schmidt
  1 sibling, 1 reply; 38+ messages in thread
From: Segher Boessenkool @ 2016-09-20 19:27 UTC (permalink / raw)
  To: Jeff Law; +Cc: Bernd Schmidt, Trevor Saunders, tbsaunde+gcc, gcc-patches

On Tue, Sep 20, 2016 at 12:11:45PM -0500, Segher Boessenkool wrote:
> I'm just pointing out the documentation is out-of-date here.  I'll do
> a patch.

Like this.  Okay for trunk, 6, 5?

Thanks,


Segher


2016-09-20  Segher Boessenkool  <segher@kernel.crashing.org>

	* doc/rtl.texi (JUMP_LABEL): Document RETURN and SIMPLE_RETURN values.

---
 gcc/doc/rtl.texi | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/gcc/doc/rtl.texi b/gcc/doc/rtl.texi
index 1b3f47e..692d9b5 100644
--- a/gcc/doc/rtl.texi
+++ b/gcc/doc/rtl.texi
@@ -3525,8 +3525,8 @@ and @code{addr_diff_vec}, where @code{JUMP_LABEL} is @code{NULL_RTX}
 and the only way to find the labels is to scan the entire body of the
 insn.
 
-Return insns count as jumps, but since they do not refer to any
-labels, their @code{JUMP_LABEL} is @code{NULL_RTX}.
+Return insns count as jumps, but their @code{JUMP_LABEL} is @code{RETURN}
+or @code{SIMPLE_RETURN}.
 
 @findex call_insn
 @item call_insn
-- 
1.9.3

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

* Re: [PATCH] Fix JUMP_LABEL documentation
  2016-09-20 19:27                     ` [PATCH] Fix JUMP_LABEL documentation Segher Boessenkool
@ 2016-09-21 13:04                       ` Bernd Schmidt
  0 siblings, 0 replies; 38+ messages in thread
From: Bernd Schmidt @ 2016-09-21 13:04 UTC (permalink / raw)
  To: Segher Boessenkool, Jeff Law; +Cc: Trevor Saunders, tbsaunde+gcc, gcc-patches

On 09/20/2016 08:51 PM, Segher Boessenkool wrote:
>
> 	* doc/rtl.texi (JUMP_LABEL): Document RETURN and SIMPLE_RETURN values.

Ok, thanks.


Bernd

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

end of thread, other threads:[~2016-09-21 12:11 UTC | newest]

Thread overview: 38+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2016-09-14 19:13 [PATCH 0/8] make next_*_insn take rtx_insn * arguments tbsaunde+gcc
2016-09-14 19:13 ` [PATCH 1/8] change a few rtx_insn * to rtx_jump_insn * tbsaunde+gcc
2016-09-15  4:33   ` Kaz Kojima
2016-09-15  5:56     ` Trevor Saunders
2016-09-15  7:08       ` Kaz Kojima
2016-09-15  8:51       ` Richard Biener
2016-09-15  9:26         ` Trevor Saunders
2016-09-14 19:14 ` [PATCH 7/8] make next/prev active_insn and active_insn_p take rtx_insn * tbsaunde+gcc
2016-09-19 21:26   ` Jeff Law
2016-09-19 23:33     ` Trevor Saunders
2016-09-14 19:14 ` [PATCH 4/8] make next/prev nondebug_insn " tbsaunde+gcc
2016-09-14 19:14 ` [PATCH 8/8] make next_cc0_user " tbsaunde+gcc
2016-09-14 19:14 ` [PATCH 3/8] make next/prev _nonnote_insn " tbsaunde+gcc
2016-09-19 21:27   ` Jeff Law
2016-09-19 23:29     ` Trevor Saunders
2016-09-14 19:14 ` [PATCH 2/8] use rtx_insn * more tbsaunde+gcc
2016-09-16 15:25   ` Alexander Monakov
2016-09-14 19:19 ` [PATCH 6/8] make next/prev nonnote_nondebug_insn take rtx_insn * tbsaunde+gcc
2016-09-19 21:09   ` Jeff Law
2016-09-14 19:33 ` [PATCH 5/8] make prev_real_insn " tbsaunde+gcc
2016-09-19 21:08   ` Jeff Law
2016-09-15 11:02 ` [PATCH 0/8] make next_*_insn take rtx_insn * arguments Bernd Schmidt
2016-09-15 16:23   ` Trevor Saunders
2016-09-15 16:42     ` Jeff Law
2016-09-15 17:42       ` Trevor Saunders
2016-09-16 10:07       ` Trevor Saunders
2016-09-16 10:21         ` Bernd Schmidt
2016-09-16 15:13           ` Trevor Saunders
2016-09-16 15:29           ` Jeff Law
2016-09-19 20:55         ` Jeff Law
2016-09-19 22:02           ` Trevor Saunders
2016-09-19 23:21             ` Segher Boessenkool
2016-09-20 11:31               ` Bernd Schmidt
2016-09-20 14:54                 ` Jeff Law
2016-09-20 17:13                   ` Segher Boessenkool
2016-09-20 17:56                     ` Jeff Law
2016-09-20 19:27                     ` [PATCH] Fix JUMP_LABEL documentation Segher Boessenkool
2016-09-21 13:04                       ` Bernd Schmidt

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).