Hi! Thanks for investigating.
On Fri, Apr 08, 2022 at 03:25:51PM +0800, Kewen.Lin wrote: > on 2022/4/8 3:29 AM, Segher Boessenkool wrote: > > On Thu, Apr 07, 2022 at 09:19:51AM -0500, will schmidt wrote: > >> On Mon, 2022-02-28 at 13:37 +0800, Kewen.Lin via Gcc-patches wrote: > >>> As PR103196 shows, p9-vec-length-full-7.c needs to be adjusted as the > >>> complete unrolling can happen on some of its loops. This patch is to > >>> use pragma "GCC unroll 0" to disable all possible loop unrollings. > >>> Hope it can help the case not that fragile. > >> > >> ok > >> > >> Is the lack of effectiveness of "-fno-unroll-loops" otherwise > >> understood, or is there further issue behind that option? > > > > There is -fpeel-loops as well, and cunroll is independent of all of > > these as well? > > Yes, kind of. As below code, unroll-loops and peel-loops only affect > whether it's acceptable to grow size. > > /* Allow cunroll to grow size accordingly. */ > if (!opts_set->x_flag_cunroll_grow_size) > opts->x_flag_cunroll_grow_size > = (opts->x_flag_unroll_loops > || opts->x_flag_peel_loops > || opts->x_optimize >= 3); > > This flag_cunroll_grow_size doesn't gate the pass, it's only for > "may_increase_size". Yuck. > unsigned int val = tree_unroll_loops_completely (flag_cunroll_grow_size, > true); > > The cunroll still can take effect if its transformation doesn't grow > size even if we specify -fno-unroll-loops and -fno-peel-loops. Yeah. And the pragma disables it, so we really should have a command line option that does the same (but globally), too. > >> I would > >> expect the effect of the option, versus the pragma, two to roughly > >> equivalent. Obviously it is not. :-) > > > > Yes, me too. And I do not see what makes the difference, if it isn't > > the peel thing :-( > > > > Ke Wen, can you try with -fno-peel-loops please? > > I had a try and it didn't help as the cunroll pass doesn't grow size > for this case. By the way, -fdisable-tree-cunroll does work. But I > thought using pragma looks better as it's not an internal thing like > the disabling option and I also expect other future unrolling related > changes would respect it. > > Do you also prefer pragma, or want that disabling option? That flag please. And we should have a -fno-unroll that disables all separate kinds of unrolling, imo -- but that is a separate problem, not something to hold up your patch for. Okay for trunk without the pragma. Thanks! (The pragma is a good solution if you want to disable unrolling for a very specific piece of code only, which is not the case here: the problem currently shows up in one place, but it could happen elsewhere!) Segher
