> -----Original Message-----
> From: Steven Bosscher [mailto:[email protected]]
> Sent: Friday, October 12, 2012 7:04 AM
> To: Bin Cheng
> Cc: Jeff Law; [email protected]
> Subject: Re: [PATCH RFA] Implement register pressure directed hoist pass
> 
> 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...

Very sorry I didn't realize there are so many language errors. Since English
is not my native language, I will pay more attention to comments in the
future.

Thanks very much for your patient.



Reply via email to