This is the mail archive of the
gcc-patches@gcc.gnu.org
mailing list for the GCC project.
Re: Fix postreload liveness computation
- From: Eric Botcazou <ebotcazou at libertysurf dot fr>
- To: Richard Sandiford <rsandifo at nildram dot co dot uk>
- Cc: gcc-patches at gcc dot gnu dot org
- Date: Thu, 24 May 2007 23:11:32 +0200
- Subject: Re: Fix postreload liveness computation
- References: <87abvx96e6.fsf@firetop.home>
> Unfortunately, not all live_at_start sets are valid before reload_combine:
>
> /* Set up LABEL_LIVE and EVER_LIVE_AT_START. The register lifetime
> information is a bit fuzzy immediately after reload, but it's
> still good enough to determine which registers are live at a jump
> destination. */
>
> I think what it means by "a bit fuzzy" is that the liveness is wrong
> in cases where reload might inherit across a block boundary. In other
> words, the liveness is wrong for blocks that are only reached via
> fallthru edges. So, rather than look at the live_at_end sets directly,
> reload_combine instead tries to work out the liveness from the known-valid
> live_at_start sets. The problem is that it does so only for jumps;
> it doesn't consider exceptional edges.
I'm not a specialist of reload_combine, but I think that your interpretation
is quite sound.
> This causes a miscompilation of eh_personality.cc on sh-elf with a patch I'm
> working on.
Out of curiosity, which part and how?
> The CFG itself is valid at this point, so I think the fix is simply
> to compute the union of all successor non-fallthru live_at_start sets.
Sounds OK to me, but the implementation looks a litte convoluted. :-)
> + /* Set up BB_LIVE_AT_START[B] to the set of hard registers live at
> + the beginning of block B. This information is only valid for
> + the entry and exit blocks, or for those that start with labels.
> + The live-at-start sets are no longer valid for blocks that are
> + only reached via fallthrough; reload may have inherited from the
> + previous block. */
> + bb_live_at_start = XNEWVEC (HARD_REG_SET, last_basic_block);
> + FOR_ALL_BB (bb)
> + {
> + live = &bb_live_at_start[bb->index];
> + REG_SET_TO_HARD_REG_SET (*live, bb->il.rtl->global_live_at_start);
> + compute_use_by_pseudos (live, bb->il.rtl->global_live_at_start);
> }
What about skipping the computation when it is not valid to match the comment?
if (EDGE_COUNT (bb->preds) == 1
&& (EDGE_PRED ((bb), 0)->flags & EDGE_FALLTHRU) != 0)
> @@ -778,6 +766,35 @@ reload_combine (void)
> if (! INSN_P (insn))
> continue;
>
> + /* See if INSN is the end of a basic block. */
> + new_bb = BLOCK_FOR_INSN (insn);
> + if (new_bb && new_bb != bb)
> + {
> + bb = new_bb;
Why not use BB_END?
> + all_live = NULL;
> + FOR_EACH_EDGE (e, ei, new_bb->succs)
> + if ((e->flags & EDGE_FALLTHRU) == 0)
> + {
> + live = &bb_live_at_start[e->dest->index];
> + if (!all_live)
> + all_live = live;
> + else
> + {
> + if (all_live != &all_live_tmp)
> + {
> + COPY_HARD_REG_SET (all_live_tmp, *all_live);
> + all_live = &all_live_tmp;
> + }
> + IOR_HARD_REG_SET (all_live_tmp, *live);
> + }
> + }
> +
> + if (all_live)
> + for (i = FIRST_PSEUDO_REGISTER - 1; i >= 0; --i)
> + if (TEST_HARD_REG_BIT (*all_live, i))
> + reg_state[i].use_index = -1;
> + }
Hum... so now that we have a nice HARD_REG_SET API, we don't use it?
Why not the straightforward following?
CLEAR_HARD_REG_SET (...)
FOR_EACH_EDGE (e, ei, new_bb->succs)
if ((e->flags & EDGE_FALLTHRU) == 0)
IOR_HARD_REG_SET (...)
if (!hard_reg_set_empty_p (...))
for (i = FIRST_PSEUDO_REGISTER - 1; i >= 0; --i)
if (TEST_HARD_REG_BIT (..., i))
...
--
Eric Botcazou