On 09/10/2015 05:14 PM, Segher Boessenkool wrote:
This patch rewrites the shrink-wrapping algorithm, allowing non-linear pieces of CFG to be duplicated for use without prologue instead of just linear pieces.
An example would be good for this kind of patch, also in the comments.
- add_to_hard_reg_set (&hardregs, GET_MODE (dreg), - REGNO (dreg)); + add_to_hard_reg_set (&hardregs, GET_MODE (dreg), REGNO (dreg)); }
In general please try to avoid mixing formatting changes with functional ones. No need to remove these ones from future versions of the patch though.
+ /* If there is more than one predecessor of PRO not dominated by PRO, + fail. We don't have to do this (can split the block), but do this + for now (the original code disallowed this, too).
Comments shouldn't reference previous versions. Also, a comment describing the why rather than just what is being done would be more helpful.
I'm wondering how your new algorithm prevents the prologue from being placed inside a loop. Can you have a situation where this picks a predecessor that is reachable but not dominated by PRO?
+ int num = (*entry_edge)->probability; + int den = REG_BR_PROB_BASE; + + + if (*entry_edge == orig_entry_edge) + goto out;
There are a couple of places where there are multiple blank lines. I think we avoid that.
+ FOR_EACH_EDGE (e, ei, src->succs) + { + if (e->dest == EXIT_BLOCK_PTR_FOR_FN (cfun)) { - all_set = false; - break; }
Minor, but I'd prefer a continue rather than an empty { } block. Other than that it looks pretty good. Bernd