Rather than using global variables and then copying them into a bb
structure, would it be possible to write directly into the bb structure?
The answer's probably "no", just asking. :-)
Bernd Schmidt <ber...@codesourcery.com> writes:
> * regrename.c (struct du_head): Make nregs signed.
> (scan_rtx_reg, scan_rtx_address, dump_def_use_chain): Remove
> declarations.
This bit was split out.
> (mark_conflict, create_new_chain): Move upwards in the file.
Same here. Should mention the change to create_new_chain's interface
though.
> - 2. For each chain, the set of possible renaming registers is computed.
> + 2. We try combine the local chains across basic block boundaries by
> + comparing chains that were open at the start or end of a block to
> + those in successor/predecessor blocks.
"try to combine"
> +/* Dump all def/use chains, starting at id FROM. */
>
> static void
> -dump_def_use_chain (struct du_head *head)
> +dump_def_use_chain (int from)
> {
> - while (head)
> + du_head_p head;
> + int i;
> + FOR_EACH_VEC_ELT (du_head_p, id_to_chain, i, head)
> {
> struct du_chain *this_du = head->first;
> + if (i < from)
> + continue;
> fprintf (dump_file, "Register %s (%d):",
> reg_names[head->regno], head->nregs);
> while (this_du)
I know it's only a dumping function, but maybe this'd be a good excuse
to add:
#define FOR_EACH_VEC_ELT_FROM(T, V, I, P, FROM) \
for (I = (FROM); VEC_iterate (T, (V), (I), (P)); ++(I))
> +/* A structure recording information about each basic block. It is saved
> + and restored around basic block boundaries. */
> +struct bb_rename_info
Probably worth saying here or elsewhere that the bb->aux field points
to this information and that (more importantly) the bb->aux is null
for blocks that could not be optimised, including the exit block.
> +/* Record in RI that the block corresponding to it has an incoming
> + live value, described by CHAIN. */
> +static void
> +set_incoming_from_chain (struct bb_rename_info *ri, du_head_p chain)
> +{
> + int min_reg, max_reg, i;
> + int incoming_nregs = ri->incoming[chain->regno].nregs;
> + int nregs;
> +
> + /* If we've recorded the same information before, everything is fine. */
> + if (incoming_nregs == chain->nregs)
> + {
> + if (dump_file)
> + fprintf (dump_file, "reg %d/%d already recorded\n",
> + chain->regno, chain->nregs);
> + return;
> + }
> +
> + /* If we have no information for any of the involved registers, update
> + the incoming array. */
> + nregs = chain->nregs;
> + while (nregs-- > 0)
> + if (ri->incoming[chain->regno + nregs].nregs != 0
> + || ri->incoming[chain->regno + nregs].unusable)
> + break;
> + if (nregs < 0)
> + {
> + nregs = chain->nregs;
> + ri->incoming[chain->regno].nregs = nregs;
> + while (nregs-- > 1)
> + ri->incoming[chain->regno + nregs].nregs = -nregs;
> + if (dump_file)
> + fprintf (dump_file, "recorded reg %d/%d\n",
> + chain->regno, chain->nregs);
> + return;
> + }
> +
> + /* There must be some kind of conflict. Set the unusable for all
> + overlapping registers. */
> + min_reg = chain->regno;
> + if (incoming_nregs < 0)
> + min_reg += incoming_nregs;
> + max_reg = chain->regno + chain->nregs;
> + for (i = min_reg; i < max_reg; i++)
> + ri->incoming[i].unusable = true;
In the incoming_nregs < 0 case, we only need to set
ri->incoming[chain->regno + incoming_nregs] itself, right,
not the other registers between that and ri->incoming[chain->regno]?
If so, I think it'd be clearer to have:
/* There must be some kind of conflict. Prevent both the old and
new ranges from being used. */
if (incoming_nregs < 0)
ri->incoming[chain->regno + incoming_nregs].unusable = true;
for (i = 0; i < chain->nregs; i++)
ri->incoming[chain->regno + i].unusable = true;
When I first looked at the code, I was wondering why we changed every
register in (chain->regno + incoming_nregs, chain_regno), but none in
[chain->regno + chain->nregs, OLD_END). Seems like we should do neither
(as in the above suggestion) or both.
> + /* Process all basic blocks by using a worklist, adding unvisited successor
> + blocks whenever we reach the end of one basic blocks. This ensures that
> + whenever possible, we only process a block after at least one of its
> + predecessors, which provides a "seeding" effect to make the logic in
> + set_incoming_from_chain and init_rename_info useful. */
Wouldn't a reverse post-order (inverted_post_order_compute) allow even
more pre-opening (as well as being less code)?
Looked good to me otherwise.
Richard
