Hi!
On Thu, Jan 16, 2020 at 05:39:40PM +0800, Kewen.Lin wrote:
> --- a/gcc/cfgloop.h
> +++ b/gcc/cfgloop.h
> @@ -232,6 +232,9 @@ public:
> Other values means unroll with the given unrolling factor. */
> unsigned short unroll;
>
> + /* Like unroll field above, but it's estimated in middle-end. */
> + unsigned short estimated_uf;
Please use full words? "estimated_unroll" perhaps? (Similar for other
new names).
> +/* Implement targetm.loop_unroll_adjust_tree, strictly refers to
> + targetm.loop_unroll_adjust. */
> +
> +static unsigned
> +rs6000_loop_unroll_adjust_tree (unsigned nunroll, struct loop *loop)
> +{
> + /* For now loop_unroll_adjust is simple, just invoke directly. */
> + return rs6000_loop_unroll_adjust (nunroll, loop);
> +}
Since the two hooks have the same arguments as well, it should really
just be one hook, and an implementation can check whether
current_pass->type == RTL_PASS
if it needs to do something special for RTL, etc.? Or it can use some
more appropriate condition -- the point is you need no extra hook.
> + /* Check number of iterations is constant. */
> + if ((niter_desc->may_be_zero && !integer_zerop (niter_desc->may_be_zero))
> + || !tree_fits_uhwi_p (niter_desc->niter))
> + return false;
Check, and do what? It's easier to read if you say.
> + /* Check for an explicit unrolling factor. */
> + if (loop->unroll > 0 && loop->unroll < USHRT_MAX)
> + {
> + /* It should have been peeled instead. */
> + if (const_niter == 0 || (unsigned) loop->unroll > const_niter - 1)
> + loop->estimated_uf = 1;
> + else
> + loop->estimated_uf = loop->unroll;
> + return true;
> + }
"If loop->unroll is set, use that as loop->estimated_unroll"?
Segher
