Re: [RFC] Tree Loop Unroller Pass

[Andrew Pinski <pinskia at gmail dot com>]

On Mon, Feb 12, 2018 at 3:55 PM, Kugan Vivekanandarajah
<kugan.vivekanandarajah@linaro.org> wrote:
> Implements tree loop unroller using the infrastructure provided.
>
> gcc/ChangeLog:
>
> 2018-02-12  Kugan Vivekanandarajah  <kuganv@linaro.org>
>
>     * Makefile.in (OBJS): Add tree-ssa-loop-unroll.o.
>     * common.opt (ftree-loop-unroll): New option.
>     * passes.def: Add pass_tree_loop_uroll
>     * timevar.def (TV_TREE_LOOP_UNROLL): Add.
>     * tree-pass.h (make_pass_tree_loop_unroll): Declare.
>     * tree-ssa-loop-unroll.c: New file.


I think it was decided to name new gimple passes as gimple-* rather
than tree-ssa-*.
The option should most likely just take over -funrol-loops, etc.

Shouldn't:
+  if (targetm.hw_max_mem_read_streams
+      && (max_load_streams = targetm.hw_max_mem_read_streams ()) > 0)
+    {
+      load_streams = count_mem_load_streams (loop, max_load_streams);
+      if (load_streams > 0)
+ {
+   signed t = 1 << (floor_log2 (max_load_streams / load_streams));
+   if (t < n_unroll)
+     n_unroll = t;
+ }
+    }

this be a target hook instead of doing inline here.  It seems too
target specific notion of what a stream is.  Even the notion of a
stream here seems micro-arch specific and not generic enough.


A few more comments about the pass.
You don't check can_duplicate_loop_p on the loop.
You use optimize_function_for_size_p on the whole function instead of
checking if the loop is cold (by checking optimize_loop_for_size_p).

In my version of gimple loop unrolling, I had to add
lpt_decision.already_unrolled and mark the loop as already unrolled so
the rtl loop unroller would not happen again.
-fopt-report does not report when the unrolling has happened unlike
the RTL version.

Thanks,
Andrew