* dbr_schedule vs uninitialized registers (2)
@ 2009-04-27 10:46 Eric Botcazou
2009-04-28 7:28 ` Hans-Peter Nilsson
0 siblings, 1 reply; 2+ messages in thread
From: Eric Botcazou @ 2009-04-27 10:46 UTC (permalink / raw)
To: gcc-patches; +Cc: rdsandiford
[-- Attachment #1: Type: text/plain, Size: 1538 bytes --]
More than one year ago Richard Sandiford diagnosed a problem with the liveness
computation of the delayed branches scheduling pass and proposed a patch:
http://gcc.gnu.org/ml/gcc-patches/2007-12/msg00506.html
I suggested to use a simpler approach instead (changing the DF problem to LR
to expose uninitialized registers) and this was sufficient to fix the bug.
Ironically enough, there is a problem on the SPARC with this approach because
of the save register window instruction: it clobbers all the registers on
function entry so uninitialized registers are not detected as live if there
is no BARRIER between the entry and the instruction. That's pretty rare in
practice but I have an Ada testcase.
That's why I've finally applied Richard's patch on the mainline as well as
changed back the DF problem to LIVE. Tested on sparc-sun-solaris2.9 and
sparc64-sun-solaris2.10.
2009-04-27 Richard Sandiford <rdsandiford@googlemail.com>
Eric Botcazou <ebotcazou@adacore.com>
* resource.c (find_basic_block): Use BLOCK_FOR_INSN to look up
a label's basic block.
(mark_target_live_regs): Tidy and rework obsolete comments.
Change back DF problem to LIVE. If a label starts a basic block,
assume that all registers that used to be live then still are.
(init_resource_info): If a label starts a basic block, set its
BLOCK_FOR_INSN accordingly.
(free_resource_info): Undo the setting of BLOCK_FOR_INSN.
2009-04-27 Eric Botcazou <ebotcazou@adacore.com>
* gnat.dg/opt2.adb: New test.
--
Eric Botcazou
[-- Attachment #2: p.diff --]
[-- Type: text/x-diff, Size: 4847 bytes --]
Index: resource.c
===================================================================
--- resource.c (revision 146825)
+++ resource.c (working copy)
@@ -135,8 +135,6 @@ update_live_status (rtx dest, const_rtx
static int
find_basic_block (rtx insn, int search_limit)
{
- basic_block bb;
-
/* Scan backwards to the previous BARRIER. Then see if we can find a
label that starts a basic block. Return the basic block number. */
for (insn = prev_nonnote_insn (insn);
@@ -157,11 +155,8 @@ find_basic_block (rtx insn, int search_l
for (insn = next_nonnote_insn (insn);
insn && LABEL_P (insn);
insn = next_nonnote_insn (insn))
- {
- FOR_EACH_BB (bb)
- if (insn == BB_HEAD (bb))
- return bb->index;
- }
+ if (BLOCK_FOR_INSN (insn))
+ return BLOCK_FOR_INSN (insn)->index;
return -1;
}
@@ -848,13 +843,12 @@ return_insn_p (const_rtx insn)
(with no intervening active insns) to see if any of them start a basic
block. If we hit the start of the function first, we use block 0.
- Once we have found a basic block and a corresponding first insns, we can
- accurately compute the live status from basic_block_live_regs and
- reg_renumber. (By starting at a label following a BARRIER, we are immune
- to actions taken by reload and jump.) Then we scan all insns between
- that point and our target. For each CLOBBER (or for call-clobbered regs
- when we pass a CALL_INSN), mark the appropriate registers are dead. For
- a SET, mark them as live.
+ Once we have found a basic block and a corresponding first insn, we can
+ accurately compute the live status (by starting at a label following a
+ BARRIER, we are immune to actions taken by reload and jump.) Then we
+ scan all insns between that point and our target. For each CLOBBER (or
+ for call-clobbered regs when we pass a CALL_INSN), mark the appropriate
+ registers are dead. For a SET, mark them as live.
We have to be careful when using REG_DEAD notes because they are not
updated by such things as find_equiv_reg. So keep track of registers
@@ -954,13 +948,10 @@ mark_target_live_regs (rtx insns, rtx ta
TARGET. Otherwise, we must assume everything is live. */
if (b != -1)
{
- regset regs_live = DF_LR_IN (BASIC_BLOCK (b));
+ regset regs_live = df_get_live_in (BASIC_BLOCK (b));
rtx start_insn, stop_insn;
- /* Compute hard regs live at start of block -- this is the real hard regs
- marked live, plus live pseudo regs that have been renumbered to
- hard regs. */
-
+ /* Compute hard regs live at start of block. */
REG_SET_TO_HARD_REG_SET (current_live_regs, regs_live);
/* Get starting and ending insn, handling the case where each might
@@ -1046,10 +1037,24 @@ mark_target_live_regs (rtx insns, rtx ta
else if (LABEL_P (real_insn))
{
+ basic_block bb;
+
/* A label clobbers the pending dead registers since neither
reload nor jump will propagate a value across a label. */
AND_COMPL_HARD_REG_SET (current_live_regs, pending_dead_regs);
CLEAR_HARD_REG_SET (pending_dead_regs);
+
+ /* We must conservatively assume that all registers that used
+ to be live here still are. The fallthrough edge may have
+ left a live register uninitialized. */
+ bb = BLOCK_FOR_INSN (real_insn);
+ if (bb)
+ {
+ HARD_REG_SET extra_live;
+
+ REG_SET_TO_HARD_REG_SET (extra_live, df_get_live_in (bb));
+ IOR_HARD_REG_SET (current_live_regs, extra_live);
+ }
}
/* The beginning of the epilogue corresponds to the end of the
@@ -1121,6 +1126,7 @@ void
init_resource_info (rtx epilogue_insn)
{
int i;
+ basic_block bb;
/* Indicate what resources are required to be valid at the end of the current
function. The condition code never is and memory always is. If the
@@ -1189,6 +1195,11 @@ init_resource_info (rtx epilogue_insn)
/* Allocate and initialize the tables used by mark_target_live_regs. */
target_hash_table = XCNEWVEC (struct target_info *, TARGET_HASH_PRIME);
bb_ticks = XCNEWVEC (int, last_basic_block);
+
+ /* Set the BLOCK_FOR_INSN of each label that starts a basic block. */
+ FOR_EACH_BB (bb)
+ if (LABEL_P (BB_HEAD (bb)))
+ BLOCK_FOR_INSN (BB_HEAD (bb)) = bb;
}
\f
/* Free up the resources allocated to mark_target_live_regs (). This
@@ -1197,6 +1208,8 @@ init_resource_info (rtx epilogue_insn)
void
free_resource_info (void)
{
+ basic_block bb;
+
if (target_hash_table != NULL)
{
int i;
@@ -1222,6 +1235,10 @@ free_resource_info (void)
free (bb_ticks);
bb_ticks = NULL;
}
+
+ FOR_EACH_BB (bb)
+ if (LABEL_P (BB_HEAD (bb)))
+ BLOCK_FOR_INSN (BB_HEAD (bb)) = NULL;
}
\f
/* Clear any hashed information that we have stored for INSN. */
[-- Attachment #3: opt2.adb --]
[-- Type: text/x-adasrc, Size: 564 bytes --]
-- { dg-do run }
-- { dg-options "-O2 -fno-inline" }
procedure Opt2 is
function Get return String is
begin
return "[]";
end Get;
Message : String := Get;
F, L : Integer;
begin
for J in Message'Range loop
if Message (J) = '[' then
F := J;
elsif Message (J) = ']' then
L := J;
exit;
end if;
end loop;
declare
M : String :=
Message (Message'First .. F) & Message (L .. Message'Last);
begin
if M /= "[]" then
raise Program_Error;
end if;
end;
end;
^ permalink raw reply [flat|nested] 2+ messages in thread
* Re: dbr_schedule vs uninitialized registers (2)
2009-04-27 10:46 dbr_schedule vs uninitialized registers (2) Eric Botcazou
@ 2009-04-28 7:28 ` Hans-Peter Nilsson
0 siblings, 0 replies; 2+ messages in thread
From: Hans-Peter Nilsson @ 2009-04-28 7:28 UTC (permalink / raw)
To: Eric Botcazou; +Cc: gcc-patches, rdsandiford
On Mon, 27 Apr 2009, Eric Botcazou wrote:
> 2009-04-27 Richard Sandiford <rdsandiford@googlemail.com>
> Eric Botcazou <ebotcazou@adacore.com>
>
> * resource.c (find_basic_block): Use BLOCK_FOR_INSN to look up
> a label's basic block.
> (mark_target_live_regs): Tidy and rework obsolete comments.
> Change back DF problem to LIVE. If a label starts a basic block,
> assume that all registers that used to be live then still are.
> (init_resource_info): If a label starts a basic block, set its
> BLOCK_FOR_INSN accordingly.
> (free_resource_info): Undo the setting of BLOCK_FOR_INSN.
This caused PR39938.
brgds, H-P
^ permalink raw reply [flat|nested] 2+ messages in thread
end of thread, other threads:[~2009-04-28 6:12 UTC | newest]
Thread overview: 2+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2009-04-27 10:46 dbr_schedule vs uninitialized registers (2) Eric Botcazou
2009-04-28 7:28 ` Hans-Peter Nilsson
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).