diff mbox

Fix -fcompare-debug issue in reload1 (PR debug/46252)

Message ID 201011022031.55320.ebotcazou@adacore.com
State New
Headers show

Commit Message

Eric Botcazou Nov. 2, 2010, 7:31 p.m. UTC 
> For prev_real_insn I think you're right, the remaining two uses also do
> want to skip debug insns. 

It's really only one use, insert_insn_end_bb_new is unused, removed by the 
attached patch.

> For next_real_insn I'm not that sure, it is used e.g. in the scheduler where
> we want to see debug insns. 
> Having prev_real_insn skip debug insns and next_real_insn not skip them
> would be inconsistent though.

Should we replace all the calls to prev_real_insn by prev_active_insn then and 
delete the former?  The *_active_insn family doesn't return DEBUG_INSNS...


	* basic-block.h (insert_insn_end_bb_new): Delete.
	* cfgrtl.c (insert_insn_end_bb_new): Likewise.

Comments

Jakub Jelinek Nov. 2, 2010, 8:03 p.m. UTC | #1 
On Tue, Nov 02, 2010 at 08:31:55PM +0100, Eric Botcazou wrote:
> > For prev_real_insn I think you're right, the remaining two uses also do
> > want to skip debug insns. 
> 
> It's really only one use, insert_insn_end_bb_new is unused, removed by the 
> attached patch.
> 
> > For next_real_insn I'm not that sure, it is used e.g. in the scheduler where
> > we want to see debug insns. 
> > Having prev_real_insn skip debug insns and next_real_insn not skip them
> > would be inconsistent though.
> 
> Should we replace all the calls to prev_real_insn by prev_active_insn then and 
> delete the former?  The *_active_insn family doesn't return DEBUG_INSNS...

Yes, but after reload it also skips USE/CLOBBERs.  I'm not sure we want to
do that in cfgcleanup, in reload1.c it should be fine, as reload_completed
there is still 0, and in gcse.c it is fine too.

	Jakub
Eric Botcazou Nov. 2, 2010, 8:46 p.m. UTC | #2 
> Yes, but after reload it also skips USE/CLOBBERs.  I'm not sure we want to
> do that in cfgcleanup, in reload1.c it should be fine, as reload_completed
> there is still 0, and in gcse.c it is fine too.

There is another flavor of *_active_insn in ifcvt.c. :-)  Would it work to:
 - export last_active_insn from ifcvt.c,
 - call it in cfgcleanup.c,
 - replace the other uses of prev_real_insn by prev_active_insn,
 - delete prev_real_insn,
 - document that next_real_insn can return DEBUG_INSNs (and that its uses may 
need to be audited wrt DEBUG_INSNs in a ??? note)?
diff mbox

Patch

Index: basic-block.h
===================================================================
--- basic-block.h	(revision 166172)
+++ basic-block.h	(working copy)
@@ -881,9 +881,6 @@  extern basic_block get_bb_copy (basic_bl
 void set_loop_copy (struct loop *, struct loop *);
 struct loop *get_loop_copy (struct loop *);
 
-
-extern rtx insert_insn_end_bb_new (rtx, basic_block);
-
 #include "cfghooks.h"
 
 /* Return true when one of the predecessor edges of BB is marked with EDGE_EH.  */
Index: cfgrtl.c
===================================================================
--- cfgrtl.c	(revision 166172)
+++ cfgrtl.c	(working copy)
@@ -3090,94 +3090,6 @@  init_rtl_bb_info (basic_block bb)
   bb->il.rtl = ggc_alloc_cleared_rtl_bb_info ();
 }
 
-
-/* Add EXPR to the end of basic block BB.  */
-
-rtx
-insert_insn_end_bb_new (rtx pat, basic_block bb)
-{
-  rtx insn = BB_END (bb);
-  rtx new_insn;
-  rtx pat_end = pat;
-
-  while (NEXT_INSN (pat_end) != NULL_RTX)
-    pat_end = NEXT_INSN (pat_end);
-
-  /* If the last insn is a jump, insert EXPR in front [taking care to
-     handle cc0, etc. properly].  Similarly we need to care trapping
-     instructions in presence of non-call exceptions.  */
-
-  if (JUMP_P (insn)
-      || (NONJUMP_INSN_P (insn)
-          && (!single_succ_p (bb)
-              || single_succ_edge (bb)->flags & EDGE_ABNORMAL)))
-    {
-#ifdef HAVE_cc0
-      rtx note;
-#endif
-      /* If this is a jump table, then we can't insert stuff here.  Since
-         we know the previous real insn must be the tablejump, we insert
-         the new instruction just before the tablejump.  */
-      if (GET_CODE (PATTERN (insn)) == ADDR_VEC
-          || GET_CODE (PATTERN (insn)) == ADDR_DIFF_VEC)
-        insn = prev_real_insn (insn);
-
-#ifdef HAVE_cc0
-      /* FIXME: 'twould be nice to call prev_cc0_setter here but it aborts
-         if cc0 isn't set.  */
-      note = find_reg_note (insn, REG_CC_SETTER, NULL_RTX);
-      if (note)
-        insn = XEXP (note, 0);
-      else
-        {
-          rtx maybe_cc0_setter = prev_nonnote_insn (insn);
-          if (maybe_cc0_setter
-              && INSN_P (maybe_cc0_setter)
-              && sets_cc0_p (PATTERN (maybe_cc0_setter)))
-            insn = maybe_cc0_setter;
-        }
-#endif
-      /* FIXME: What if something in cc0/jump uses value set in new
-         insn?  */
-      new_insn = emit_insn_before_noloc (pat, insn, bb);
-    }
-
-  /* Likewise if the last insn is a call, as will happen in the presence
-     of exception handling.  */
-  else if (CALL_P (insn)
-           && (!single_succ_p (bb)
-               || single_succ_edge (bb)->flags & EDGE_ABNORMAL))
-    {
-      /* Keeping in mind targets with small register classes and parameters
-         in registers, we search backward and place the instructions before
-	 the first parameter is loaded.  Do this for everyone for consistency
-	 and a presumption that we'll get better code elsewhere as well.  */
-
-      /* Since different machines initialize their parameter registers
-         in different orders, assume nothing.  Collect the set of all
-         parameter registers.  */
-      insn = find_first_parameter_load (insn, BB_HEAD (bb));
-
-      /* If we found all the parameter loads, then we want to insert
-         before the first parameter load.
-
-         If we did not find all the parameter loads, then we might have
-         stopped on the head of the block, which could be a CODE_LABEL.
-         If we inserted before the CODE_LABEL, then we would be putting
-         the insn in the wrong basic block.  In that case, put the insn
-         after the CODE_LABEL.  Also, respect NOTE_INSN_BASIC_BLOCK.  */
-      while (LABEL_P (insn)
-             || NOTE_INSN_BASIC_BLOCK_P (insn))
-        insn = NEXT_INSN (insn);
-
-      new_insn = emit_insn_before_noloc (pat, insn, bb);
-    }
-  else
-    new_insn = emit_insn_after_noloc (pat, insn, bb);
-
-  return new_insn;
-}
-
 /* Returns true if it is possible to remove edge E by redirecting
    it to the destination of the other edge from E->src.  */