It is customary to ping a patch after a week (like 5 workdays) earliest.
I was already typing a reply when I saw it.
Regarding enabling the new option by default: I actually view that favorably
but would like to bring it up in the patchwork meeting tomorrow. Maybe if
we're even more conservative, e.g. unroll 2x by default, the decision won't be
contested :)
> diff --git a/gcc/config/riscv/riscv.cc b/gcc/config/riscv/riscv.cc
> index cc1b8cd16a9..110abe5cad1 100644
> --- a/gcc/config/riscv/riscv.cc
> +++ b/gcc/config/riscv/riscv.cc
> @@ -300,6 +300,8 @@ struct riscv_tune_param
> const char *jump_align;
> const char *loop_align;
> bool prefer_agnostic;
> + unsigned int unroll_small_loop_ninsns = 4;
> + unsigned int unroll_small_loop_factor = 4;
> };
I'd suggest renaming this to small_loop_unroll_ninsns, likewise for factor.
Also, this is the first instance of default params in the struct itself.
I'm unsure if we want that, so far everything has been explicit.
On the other hand, I don't think it takes away anything so I'd tend to agree
with your approach now that we can assume a C++14 compiler.
> +/* This function adjusts the unroll factor based on
> + the hardware capabilities. */
based on the current tune parameters.
> +++ b/gcc/testsuite/gcc.target/riscv/unroll-explicit.c
> @@ -0,0 +1,21 @@
> +/* { dg-do compile } */
> +/* { dg-options "-O2 -funroll-loops -fdump-rtl-loop2_unroll-details" } */
How about a test specific to your tune model still?
Maybe also a test that explicitly does #pragma GCC unroll 2 with
-munroll-only-small-loops.
--
Regards
Robin
