This is the mail archive of the
gcc-patches@gcc.gnu.org
mailing list for the GCC project.
regmove bugfix
- To: egcs-patches at cygnus dot com
- Subject: regmove bugfix
- From: Jeffrey A Law <law at cygnus dot com>
- Date: Sun, 17 Jan 1999 19:27:27 -0700
- Reply-To: law at cygnus dot com
Sorry for the length of the message. Based on the history of this code, I
think it's important to walk through it in some detail to make sure we've got
it right (and to get the analysis in the archives instead of my brain :-)
The short and simple is optimize_reg_copy_1 tries to update the various
REG_N_FOO counts. In the case I'm looking at, it incorrectly sets the
calls_crossed count to zero, when the register is still live across calls.
This causes caller-save to abort when it finds a hard reg that needs a
caller-save, but no pseudos allocated to that hard reg are marked as crossing
calls.
The underlying problem is the code to update the REG_N_FOO counts to totally
bogus (specifically calls_crossed and live_length). I introduced this bogus
code in an attempt to fix a similar problem in August of last year.
--
Neither of those variables should be updated if we do not change the lifetime
of the pseudo register.
When we start optimize_reg_copy_1, INSN is a copy from SREGNO to DREGNO
where SREGNO _does not die_. DREGNO becomes live at the copy insn.
For SREGNO this implies that it is live before INSN. It's death is marked
by a REG_DEAD note at P.
The only way we change the lifetime SREGNO is when we move the REG_DEAD
note from P to INSN for SREGNO. If we do not move the note, then we do not
change the lifetime to SREGNO. Think about it, remembering that SREGNO is
live _BEFORE_ INSN.
For DREGNO, it's life begins at the copy insn and extends to its REG_DEAD
note, or to an unknown location if no death note is found before P.
Similarly to SREGNO, the only time we change the lifetime of DREGNO is when
we move the REG_DEAD note for DREGNO. No matter how many changes we make,
if we do not move the REG_DEAD note, then the lifetime of DREGNO does not
change.
Therefore, the code which updates REG_N_LIVE_LENGTH and REG_N_CALLS_CROSSED
is bogus since they are updated every time to substitute DREGNO for SREGNO.
In addition to causing caller-save to abort, these updates are probably having
undesirable effects on register allocation.
For SREGNO, we need to keep track of the total number of calls crossed and
the total live length adjustment and only update the REG_N_BLAH counters when
we move the note for SREGNO.
For DREGNO, we need to keep track of the total number of calls crossed and
the total live length adjustment *after* we pass the insn with the REG_DEAD
note for DREGNO. When we move the death note for DREGNO, then and only then
do we update the REG_N_BLAH counters.
Phew. Now to double check the case I was trying to fix back in August. We
past the REG_DEAD for DEST, made a substitution, then had a substitution fail.
The original code didn't update REG_N_CALLS_CROSSED when any substitution
failed. So if between the original location of the death note for DREGNO and
the successful substitution we crossed a call, we would fail to account for
that in REG_N_CALLS_CROSSED. My August patch updated the counters when
we performed a successful substitution, which was wrong and led to today's bug.
With the attached code, we always update REG_N_CALLS_CROSSED for the
destination once we pass its death note, so I believe this patch will also
do the right thing for the bug I was trying to fix in August.
--
To reiterate, making a substitution does not change the range of insns over
which SREGNO or DREGNO is live. The only operation that changes liveness
in this code is moving a death note. Therefore, it is wrong to update
REG_N_CALLS_CROSSED or REG_LIVE_LENGTH when a substitution is made.
Anyway, this patch fixes optimize_reg_copy_1 to handle those counters
correctly.
* regmove.c (optimize_reg_copy_1): Undo Aug 18 change. Update
REG_N_CALLS_CROSSED and REG_LIVE_LENGH if and only if we change
where a register is live.
Index: regmove.c
===================================================================
RCS file: /egcs/carton/cvsfiles/egcs/gcc/regmove.c,v
retrieving revision 1.28.2.4
diff -c -3 -p -r1.28.2.4 regmove.c
*** regmove.c 1998/08/22 23:49:36 1.28.2.4
--- regmove.c 1999/01/18 02:21:38
***************
*** 1,5 ****
/* Move registers around to reduce number of move instructions needed.
! Copyright (C) 1987, 88, 89, 92-97, 1998 Free Software Foundation, Inc.
This file is part of GNU CC.
--- 1,5 ----
/* Move registers around to reduce number of move instructions needed.
! Copyright (C) 1987, 88, 89, 92-98, 1999 Free Software Foundation, Inc.
This file is part of GNU CC.
*************** optimize_reg_copy_1 (insn, dest, src)
*** 254,263 ****
&& GET_MODE (XEXP (note, 0)) == GET_MODE (src))
{
int failed = 0;
- int length = 0;
int d_length = 0;
! int n_calls = 0;
int d_n_calls = 0;
/* We can do the optimization. Scan forward from INSN again,
replacing regs as we go. Set FAILED if a replacement can't
--- 254,263 ----
&& GET_MODE (XEXP (note, 0)) == GET_MODE (src))
{
int failed = 0;
int d_length = 0;
! int s_length = 0;
int d_n_calls = 0;
+ int s_n_calls = 0;
/* We can do the optimization. Scan forward from INSN again,
replacing regs as we go. Set FAILED if a replacement can't
*************** optimize_reg_copy_1 (insn, dest, src)
*** 291,332 ****
insn in the REG_N_REFS updates below. If this is not
correct, no great harm is done.
!
! We do not undo this substitution if something later
! fails. Therefore, we must update the other REG_N_*
! counters now to keep them accurate. */
if (sregno >= FIRST_PSEUDO_REGISTER)
! {
! REG_N_REFS (sregno) -= loop_depth;
!
! if (REG_LIVE_LENGTH (sregno) >= 0)
! {
! REG_LIVE_LENGTH (sregno) -= length;
! /* REG_LIVE_LENGTH is only an approximation after
! combine if sched is not run, so make sure that
! we still have a reasonable value. */
! if (REG_LIVE_LENGTH (sregno) < 2)
! REG_LIVE_LENGTH (sregno) = 2;
! }
!
! REG_N_CALLS_CROSSED (sregno) -= n_calls;
! }
if (dregno >= FIRST_PSEUDO_REGISTER)
! {
! REG_N_REFS (dregno) += loop_depth;
!
! if (REG_LIVE_LENGTH (dregno) >= 0)
! REG_LIVE_LENGTH (dregno) += d_length;
!
! REG_N_CALLS_CROSSED (dregno) += d_n_calls;
! }
!
! /* We've done a substitution, clear the counters. */
! length = 0;
! d_length = 0;
! n_calls = 0;
! d_n_calls = 0;
}
else
{
--- 291,304 ----
insn in the REG_N_REFS updates below. If this is not
correct, no great harm is done.
! Since we do not know if we will change the lifetime of
! SREGNO or DREGNO, we must not update REG_LIVE_LENGTH
! or REG_N_CALLS_CROSSED at this time. */
if (sregno >= FIRST_PSEUDO_REGISTER)
! REG_N_REFS (sregno) -= loop_depth;
if (dregno >= FIRST_PSEUDO_REGISTER)
! REG_N_REFS (dregno) += loop_depth;
}
else
{
*************** optimize_reg_copy_1 (insn, dest, src)
*** 335,343 ****
}
}
! /* Count the insns and CALL_INSNs passed. If we passed the
! death note of DEST, show increased live length. */
! length++;
if (dest_death)
d_length++;
--- 307,316 ----
}
}
! /* For SREGNO, count the total number of insns scanned.
! For DREGNO, count the total number of insns scanned after
! passing the death note for DREGNO. */
! s_length++;
if (dest_death)
d_length++;
*************** optimize_reg_copy_1 (insn, dest, src)
*** 345,351 ****
as a call that has been crossed. Otherwise, count it. */
if (q != p && GET_CODE (q) == CALL_INSN)
{
! n_calls++;
if (dest_death)
d_n_calls++;
}
--- 318,326 ----
as a call that has been crossed. Otherwise, count it. */
if (q != p && GET_CODE (q) == CALL_INSN)
{
! /* Similarly, total calls for SREGNO, total calls beyond
! the death note for DREGNO. */
! s_n_calls++;
if (dest_death)
d_n_calls++;
}
*************** optimize_reg_copy_1 (insn, dest, src)
*** 365,375 ****
if (! failed)
{
if (sregno >= FIRST_PSEUDO_REGISTER)
{
if (REG_LIVE_LENGTH (sregno) >= 0)
{
! REG_LIVE_LENGTH (sregno) -= length;
/* REG_LIVE_LENGTH is only an approximation after
combine if sched is not run, so make sure that we
still have a reasonable value. */
--- 340,352 ----
if (! failed)
{
+ /* These counters need to be updated if and only if we are
+ going to move the REG_DEAD note. */
if (sregno >= FIRST_PSEUDO_REGISTER)
{
if (REG_LIVE_LENGTH (sregno) >= 0)
{
! REG_LIVE_LENGTH (sregno) -= s_length;
/* REG_LIVE_LENGTH is only an approximation after
combine if sched is not run, so make sure that we
still have a reasonable value. */
*************** optimize_reg_copy_1 (insn, dest, src)
*** 377,393 ****
REG_LIVE_LENGTH (sregno) = 2;
}
! REG_N_CALLS_CROSSED (sregno) -= n_calls;
}
- if (dregno >= FIRST_PSEUDO_REGISTER)
- {
- if (REG_LIVE_LENGTH (dregno) >= 0)
- REG_LIVE_LENGTH (dregno) += d_length;
-
- REG_N_CALLS_CROSSED (dregno) += d_n_calls;
- }
-
/* Move death note of SRC from P to INSN. */
remove_note (p, note);
XEXP (note, 1) = REG_NOTES (insn);
--- 354,362 ----
REG_LIVE_LENGTH (sregno) = 2;
}
! REG_N_CALLS_CROSSED (sregno) -= s_n_calls;
}
/* Move death note of SRC from P to INSN. */
remove_note (p, note);
XEXP (note, 1) = REG_NOTES (insn);
*************** optimize_reg_copy_1 (insn, dest, src)
*** 399,404 ****
--- 368,382 ----
{
XEXP (dest_death, 1) = REG_NOTES (p);
REG_NOTES (p) = dest_death;
+
+ if (dregno >= FIRST_PSEUDO_REGISTER)
+ {
+ /* If and only if we are moving the death note for DREGNO,
+ then we need to update its counters. */
+ if (REG_LIVE_LENGTH (dregno) >= 0)
+ REG_LIVE_LENGTH (dregno) += d_length;
+ REG_N_CALLS_CROSSED (dregno) += d_n_calls;
+ }
}
return ! failed;