On Thu, Oct 11, 2012 at 8:50 AM, Bin Cheng wrote:

> +  /* x+y won't be hoisted without defaultly enabled "-fira-hoist-pressure",

defaulty comment.

> +     kinds of code motion(including code hoisting) in a unified way.

needs space between "motion" and "(".

> +   flow graph, given if it can reach BB unimpared.  Stop the search if the

s/given if/if/


> +should_hoist_expr_to_dom (basic_block expr_bb, struct expr *expr,
> +                       basic_block bb, sbitmap visited, int distance,
> +                       int *bb_size, enum reg_class pressure_class,
> +                       int *nregs, bitmap hoisted_bbs)

At least BB_SIZE and DISTANCE are not documented (also not before your
patch). While there, can you document these please?


> +      becasue hoisting constant expr aggresively results in worse code
> +      as observed.  */

s/becasue/because/
s/aggresively/aggressively/

Not sure what you mean with "as observed". Observed where/how?

> -      visited = XCNEWVEC (char, last_basic_block);
> +      visited = sbitmap_alloc (last_basic_block);
> +      sbitmap_zero (visited);

Send a separate patch for this change please. Really. Reviewing
patches is much easier if you post things one change at a time.


> +   The code hoisting pass hoists multiple computations of expression
> +   to dominator basic block, like from b2/b3 to b1 as depicted below:

"The code hoisting pass can hoist multiple computations of the same
expression along dominated paths to a dominating basic block, like
from ..."


> +   Unfortunately code hoisting generally extend live range of output

s/extend/extends the/
s/of output/of an output/


> +   from rtx cost of corresponding expression and it's used to control

s/of corresponding/of the corresponding/
Similarly elsewhere in comments.


> +     of shrinked live range of input pseudo register when hoisting an

s/range/ranges/
s/register/registers/
After all this is only possible, AFAICT, if there's more than one
input register.

I'll leave all the algorithmic stuff to Jeff...

Ciao!
Steven

Reply via email to