* RFA: MEP: Fix use of delete_insn.
@ 2012-06-29 9:22 Nick Clifton
2012-06-29 20:15 ` DJ Delorie
0 siblings, 1 reply; 5+ messages in thread
From: Nick Clifton @ 2012-06-29 9:22 UTC (permalink / raw)
To: dj; +Cc: gcc-patches
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");
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: RFA: MEP: Fix use of delete_insn.
2012-06-29 9:22 RFA: MEP: Fix use of delete_insn Nick Clifton
@ 2012-06-29 20:15 ` DJ Delorie
2012-07-02 8:25 ` nick clifton
0 siblings, 1 reply; 5+ messages in thread
From: DJ Delorie @ 2012-06-29 20:15 UTC (permalink / raw)
To: Nick Clifton; +Cc: gcc-patches
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.
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: RFA: MEP: Fix use of delete_insn.
2012-06-29 20:15 ` DJ Delorie
@ 2012-07-02 8:25 ` nick clifton
2012-07-02 17:38 ` DJ Delorie
0 siblings, 1 reply; 5+ messages in thread
From: nick clifton @ 2012-07-02 8:25 UTC (permalink / raw)
To: DJ Delorie; +Cc: gcc-patches
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");
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: RFA: MEP: Fix use of delete_insn.
2012-07-02 8:25 ` nick clifton
@ 2012-07-02 17:38 ` DJ Delorie
2012-07-03 15:53 ` nick clifton
0 siblings, 1 reply; 5+ messages in thread
From: DJ Delorie @ 2012-07-02 17:38 UTC (permalink / raw)
To: nick clifton; +Cc: gcc-patches
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 :-)
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: RFA: MEP: Fix use of delete_insn.
2012-07-02 17:38 ` DJ Delorie
@ 2012-07-03 15:53 ` nick clifton
0 siblings, 0 replies; 5+ messages in thread
From: nick clifton @ 2012-07-03 15:53 UTC (permalink / raw)
To: DJ Delorie; +Cc: gcc-patches
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");
^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2012-07-03 15:53 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2012-06-29 9:22 RFA: MEP: Fix use of delete_insn Nick Clifton
2012-06-29 20:15 ` DJ Delorie
2012-07-02 8:25 ` nick clifton
2012-07-02 17:38 ` DJ Delorie
2012-07-03 15:53 ` nick clifton
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).