Hi Kewen, On Thu, Jun 13, 2019 at 01:50:05PM +0800, Kewen.Lin wrote: > Comparing with the previous version, I dropped the checks > on costly niter and invalid stmt scanning. As Richard's > suggestion, keep this as simple as possible, refine it > if finding any cases which matter later.
I think we'll want the invalid statement thing back pretty soon, it just happens too often (20% of the time), and it makes ivopts make bad decisions. It does not matter (much) for the performance of the generated code, but still :-) > +/* Predict whether the given loop in gimple will be transformed in the RTL > + doloop_optimize pass. This is for rs6000 target specific. */ Everything in rs6000/ is target-specific ;-) Remove that part? > + /* On rs6000, targetm.can_use_doloop_p is actually > + can_use_doloop_if_innermost. Just ensure it's innermost. */ "the loop is innermost". > + fprintf (dump_file, "Predict doloop failure due to" > + " no innermost.\n"); "due to no innermost" isn't great, but I don't know how th phrase it better, either :-/ It's just debug output of course, so not so very important. > +This target hook is required only when the target supports low-overhead > +loops, and will help some earlier middle-end passes to make some decisions. Maybe just say it is for ivopts? Or say that targets that do support low-overhead loops *should* implement it? > +/* True if we can predict this loop is possible to be transformed to a > + low-overhead loop, otherwise returns false. "Return true if we predict the loop LOOP will be transformed to a low-overhead loop, otherwise return false"? Or mention doloop_optimize like below: > +/* Predict whether the given loop will be transformed in the RTL > + doloop_optimize pass. Attempt to duplicate some doloop_optimize checks. > + This is only for target independent checks, see targetm.predict_doloop_p > + for the target dependent ones. > + > + Note that according to some initial investigation, some checks like costly > + niter check and invalid stmt scanning don't have much gains among general > + cases, so keep this as simple as possible first. > + > + Some RTL specific checks seems unable to be checked in gimple, if any new > + checks or easy checks _are_ missing here, please add them. */ Good useful note, thanks :-) The rs6000 part is okay for trunk. Thanks! Segher
