on 2022/4/8 10:34 PM, Segher Boessenkool wrote:
> 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!
>
Thanks Segher! I just committed the attached patch in r12-8075 which
follows your preference with -fdisable-tree-cunroll.
> (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!)
>
>
Yeah, but for this failure, all loops of the cases use the same macro
on which the proposed pragma is supposed to be used, so all loops should
be affected (I would expect it's the same as global effect). :)
BR,
Kewen
From e2ecbeeb47d20d6679993a74064bc9efdb827eef Mon Sep 17 00:00:00 2001
From: Kewen Lin <li...@linux.ibm.com>
Date: Sun, 10 Apr 2022 21:31:09 -0500
Subject: [PATCH] rs6000/test: Adjust p9-vec-length-{full,epil}-7.c [PR103196]
As PR103196 shows, complete unrolling pass still takes effect even
if we have specified the option "-fno-unroll-loops". The loops in
that case are not expected to be transformed by it, otherwise the
expected counts change. This patch is to add the disabling option
to make them not sensitive to complete unrolling.
PR testsuite/103196
gcc/testsuite/ChangeLog:
* gcc.target/powerpc/p9-vec-length-epil-7.c: Add option
-fdisable-tree-cunroll.
* gcc.target/powerpc/p9-vec-length-full-7.c: Likewise.
---
gcc/testsuite/gcc.target/powerpc/p9-vec-length-epil-7.c | 4 +++-
gcc/testsuite/gcc.target/powerpc/p9-vec-length-full-7.c | 4 +++-
2 files changed, 6 insertions(+), 2 deletions(-)
diff --git a/gcc/testsuite/gcc.target/powerpc/p9-vec-length-epil-7.c
b/gcc/testsuite/gcc.target/powerpc/p9-vec-length-epil-7.c
index a27ee347ca1..011b731f7c5 100644
--- a/gcc/testsuite/gcc.target/powerpc/p9-vec-length-epil-7.c
+++ b/gcc/testsuite/gcc.target/powerpc/p9-vec-length-epil-7.c
@@ -1,5 +1,7 @@
/* { dg-do compile { target { lp64 && powerpc_p9vector_ok } } } */
-/* { dg-options "-mdejagnu-cpu=power9 -O2 -ftree-vectorize
-fno-vect-cost-model -fno-unroll-loops -ffast-math" } */
+/* Pass cunroll isn't disabled by -fno-unroll-loops, so use explicit
+ disabling option for it. */
+/* { dg-options "-mdejagnu-cpu=power9 -O2 -ftree-vectorize
-fno-vect-cost-model -fno-unroll-loops -ffast-math -fdisable-tree-cunroll" } */
/* { dg-additional-options "--param=vect-partial-vector-usage=1" } */
diff --git a/gcc/testsuite/gcc.target/powerpc/p9-vec-length-full-7.c
b/gcc/testsuite/gcc.target/powerpc/p9-vec-length-full-7.c
index 89ff38443e7..e0e51d9a972 100644
--- a/gcc/testsuite/gcc.target/powerpc/p9-vec-length-full-7.c
+++ b/gcc/testsuite/gcc.target/powerpc/p9-vec-length-full-7.c
@@ -1,5 +1,7 @@
/* { dg-do compile { target { lp64 && powerpc_p9vector_ok } } } */
-/* { dg-options "-mdejagnu-cpu=power9 -O2 -ftree-vectorize
-fno-vect-cost-model -fno-unroll-loops -ffast-math" } */
+/* Pass cunroll isn't disabled by -fno-unroll-loops, so use explicit
+ disabling option for it. */
+/* { dg-options "-mdejagnu-cpu=power9 -O2 -ftree-vectorize
-fno-vect-cost-model -fno-unroll-loops -ffast-math -fdisable-tree-cunroll" } */
/* { dg-additional-options "--param=vect-partial-vector-usage=2" } */
--
2.27.0
