On 6/14/19 4:53 PM, Segher Boessenkool wrote:
> 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.
How about "due to loop nesting?"
Bill
>
>> +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
>
