RFA: MEP: Fix use of delete_insn.

RFA: MEP: Fix use of delete_insn.

[Nick Clifton]

Hi DJ,

  The delete_insn() function no longer returns the insn after the one
  that has been deleted, so gcc/config/mep/mep.c:mep_reorg_regmove() no
  longer compiles.  The patch below is a simple fix for the problem, but
  I was not sure whether it would be better to use
  next_nonnote_nondebug_insn() instead.  What do you think ?

Cheers
  Nick

gcc/ChangeLog
2012-06-29  Nick Clifton  <nickc@redhat.com>

	* config/mep/mep.c (mep_reorg_regmove): Get next insn before
	calling delete_insn.

Index: gcc/config/mep/mep.c
===================================================================
--- gcc/config/mep/mep.c	(revision 189064)
+++ gcc/config/mep/mep.c	(working copy)
@@ -5096,7 +5096,8 @@
 					       follow, where))
 		{
 		  count ++;
-		  next = delete_insn (insn);
+		  next = NEXT_INSN (insn);
+		  delete_insn (insn);
 		  if (dump_file)
 		    {
 		      fprintf (dump_file, "\n----- Success!  new insn:\n\n");

Re: RFA: MEP: Fix use of delete_insn.

[DJ Delorie]

We have this at the top of the loop, so I don't think it matters:

	  next = NEXT_INSN (insn);
	  if (GET_CODE (insn) != INSN)
	    continue;

However, I think an insn will be skipped if we use NEXT.  Perhaps we
want PREV?  Or the loop might need to be altered to account for this
potential skipping.

Re: RFA: MEP: Fix use of delete_insn.

[nick clifton]

Hi DJ,

> We have this at the top of the loop, so I don't think it matters:
>
> 	  next = NEXT_INSN (insn);
> 	  if (GET_CODE (insn) != INSN)
> 	    continue;

Good point.

> However, I think an insn will be skipped if we use NEXT.  Perhaps we
> want PREV?  Or the loop might need to be altered to account for this
> potential skipping.

Hmm, I think that just using the NEXT_INSN at the head of the loop 
should work.  As far as I can tell from looking at the code in 
mep_reorg_remove() we are never going to delete an insn that might 
involve removing more than one real insn, so we do not have to worry 
about side effects.  Hence I would suggest the following alternative patch.

OK to apply ?

Cheers
   Nick

Index: gcc/config/mep/mep.c
===================================================================
--- gcc/config/mep/mep.c	(revision 189108)
+++ gcc/config/mep/mep.c	(working copy)
@@ -5096,7 +5096,7 @@
  					       follow, where))
  		{
  		  count ++;
-		  next = delete_insn (insn);
+		  delete_insn (insn);
  		  if (dump_file)
  		    {
  		      fprintf (dump_file, "\n----- Success!  new insn:\n\n");

Re: RFA: MEP: Fix use of delete_insn.

[DJ Delorie]

My concern is more about calling NEXT_INSN on a deleted insn.  If
that's guaranteed to be "reliable", I'm OK with it.

Alternately, call NEXT_INSN at the top of the loop, but save the value
until the *next* iteration of the loop, so we can delete the insn and
not have to call NEXT_INSN on it after being deleted.

next_insn = get_insns ();
while (next_insn)
  {
    insn = next_insn;
    next_insn = NEXT_INSN (insn);
    . . .
  }

Of course, *that* assumes that we never delete more than just the one
"insn" we're processing.  In that case, though, we could still just
update next_insn so the next loop gets the right one.

So pick whichever solution is more future-proof and go for it :-)

Re: RFA: MEP: Fix use of delete_insn.

[nick clifton]

Hi DJ,


> My concern is more about calling NEXT_INSN on a deleted insn.  If
> that's guaranteed to be "reliable", I'm OK with it.

> So pick whichever solution is more future-proof and go for it :-)

OK, I have gone with the following.  I have replaced NEXT_INSN with 
next_nonnote_nondebug_insn, so that we are sure that we are dealing with 
a valid potential insn for removal.  We do not have to worry about 
calling this on a deleted insn because we know that delete_insn() will 
only be called on a single set insn, and that deleting it will only 
remove that insn and not any others that follow it.

Whilst doing this I noticed that follow might be set incorrectly (to a 
debug insn), so I fixed that as well.

Cheers
   Nick

gcc/ChangeLog
2012-07-03  Nick Clifton  <nickc@redhat.com>

	* config/mep/mep.c (mep_reorg_regmove): Use
	next_nonnote_non_debug_insn to advance to the next insn.  Do not
	expect delete_insn to return an rtx.

Index: gcc/config/mep/mep.c
===================================================================
--- gcc/config/mep/mep.c	(revision 189193)
+++ gcc/config/mep/mep.c	(working copy)
@@ -5022,7 +5022,7 @@
        done = 1;
        for (insn = insns; insn; insn = next)
  	{
-	  next = NEXT_INSN (insn);
+	  next = next_nonnote_nondebug_insn (insn);
  	  if (GET_CODE (insn) != INSN)
  	    continue;
  	  pat = PATTERN (insn);
@@ -5035,7 +5035,7 @@
  	      && find_regno_note (insn, REG_DEAD, REGNO (SET_SRC (pat)))
  	      && mep_compatible_reg_class (REGNO (SET_SRC (pat)), REGNO 
(SET_DEST (pat))))
  	    {
-	      follow = next_nonnote_insn (insn);
+	      follow = next_nonnote_nondebug_insn (insn);
  	      if (dump_file)
  		fprintf (dump_file, "superfluous moves: considering %d\n", INSN_UID 
(insn));

@@ -5096,7 +5096,7 @@
  					       follow, where))
  		{
  		  count ++;
-		  next = delete_insn (insn);
+		  delete_insn (insn);
  		  if (dump_file)
  		    {
  		      fprintf (dump_file, "\n----- Success!  new insn:\n\n");