diff mbox series

[testuite] Fix pr80481.C after epilogue vectorization

Message ID 90a15a74-0eb4-7719-cff5-53345764ba30@arm.com
State New
Headers show
Series [testuite] Fix pr80481.C after epilogue vectorization | expand

Commit Message

Andre Vieira (lists) Oct. 31, 2019, 1:55 p.m. UTC
Hi,

I used to have this testcase in my patch when testing but forgot to 
include it in the patch I sent upstream.  This testcase checks that a 
vmovaps isn't generated when vectorizing the loop.  When I turn epilogue 
vectorization it seems to come back.

@Jakub: This test has -fopenmp but I debuged through the testcase and 
loop->simdlen always seems to be 0 for any loop we analyze, so I don't 
think this is a conflict between my epilogue vectorization and openmp 
code paths in vect_analyze_loop.

I suspect the vmovaps is introduced because of the epilogue and it is 
just a testism, but would like a second opinion.

Cheers,
Andre

gcc/testsuite/ChangeLog:
2019-10-31  Andre Vieira  <andre.simoesdiasvieira@arm.com>

         * g++.dg/pr80481.C: Disable epilogue vectorization.

Comments

Jakub Jelinek Oct. 31, 2019, 2:04 p.m. UTC | #1
On Thu, Oct 31, 2019 at 01:55:26PM +0000, Andre Vieira (lists) wrote:
> I used to have this testcase in my patch when testing but forgot to include
> it in the patch I sent upstream.  This testcase checks that a vmovaps isn't
> generated when vectorizing the loop.  When I turn epilogue vectorization it
> seems to come back.
> 
> @Jakub: This test has -fopenmp but I debuged through the testcase and
> loop->simdlen always seems to be 0 for any loop we analyze, so I don't think
> this is a conflict between my epilogue vectorization and openmp code paths
> in vect_analyze_loop.

The test certainly should have used -fopenmp-simd instead, it only uses
#pragma omp simd and no other OpenMP pragmas.
All the loops marked as omp simd are certainly marked with corresponding
simdlen during ompexp, not sure what happens afterwards.

	Jakub
Andre Vieira (lists) Oct. 31, 2019, 6:02 p.m. UTC | #2
On 31/10/2019 14:04, Jakub Jelinek wrote:
> On Thu, Oct 31, 2019 at 01:55:26PM +0000, Andre Vieira (lists) wrote:
>> I used to have this testcase in my patch when testing but forgot to include
>> it in the patch I sent upstream.  This testcase checks that a vmovaps isn't
>> generated when vectorizing the loop.  When I turn epilogue vectorization it
>> seems to come back.
>>
>> @Jakub: This test has -fopenmp but I debuged through the testcase and
>> loop->simdlen always seems to be 0 for any loop we analyze, so I don't think
>> this is a conflict between my epilogue vectorization and openmp code paths
>> in vect_analyze_loop.
> 
> The test certainly should have used -fopenmp-simd instead, it only uses
> #pragma omp simd and no other OpenMP pragmas.
> All the loops marked as omp simd are certainly marked with corresponding
> simdlen during ompexp, not sure what happens afterwards.
> 
> 	Jakub
> 

When I debug the vect_analyze_loop calls loop->simdlen is 0 everywhere, 
with or without epilogue vectorization turned on.  However, I also 
noticed that excluding -fopenmp and -fopenmp-simd will yield the 
generation of vmovaps even without epilogue vectorization. So maybe its 
worth understanding why this vmovaps is geneated. I will be honest, I 
don't even know what the instruction does right now...
Jakub Jelinek Oct. 31, 2019, 6:08 p.m. UTC | #3
On Thu, Oct 31, 2019 at 06:02:02PM +0000, Andre Vieira (lists) wrote:
> When I debug the vect_analyze_loop calls loop->simdlen is 0 everywhere, with
> or without epilogue vectorization turned on.  However, I also noticed that
> excluding -fopenmp and -fopenmp-simd will yield the generation of vmovaps
> even without epilogue vectorization. So maybe its worth understanding why
> this vmovaps is geneated. I will be honest, I don't even know what the
> instruction does right now...

vmovaps is a vector register move instruction (or load or store if used with
memory operand).
Not really sure it is worth spending much time on it, the PR for which this
testcase was created was a register allocation issue, so if the IL in GIMPLE
is changed massively, it can be expected that the RA will also change
significantly.
So I'd say just disable epilogue vectorization for it, perhaps change
-fopenmp to -fopenmp-simd and be done with this.

	Jakub
Jeff Law March 5, 2020, 11:59 p.m. UTC | #4
On Thu, 2019-10-31 at 13:55 +0000, Andre Vieira (lists) wrote:
> Hi,
> 
> I used to have this testcase in my patch when testing but forgot to 
> include it in the patch I sent upstream.  This testcase checks that a 
> vmovaps isn't generated when vectorizing the loop.  When I turn epilogue 
> vectorization it seems to come back.
> 
> @Jakub: This test has -fopenmp but I debuged through the testcase and 
> loop->simdlen always seems to be 0 for any loop we analyze, so I don't 
> think this is a conflict between my epilogue vectorization and openmp 
> code paths in vect_analyze_loop.
> 
> I suspect the vmovaps is introduced because of the epilogue and it is 
> just a testism, but would like a second opinion.
> 
> Cheers,
> Andre
> 
> gcc/testsuite/ChangeLog:
> 2019-10-31  Andre Vieira  <andre.simoesdiasvieira@arm.com>
> 
>          * g++.dg/pr80481.C: Disable epilogue vectorization.
THanks.  I've reviewed the discussion between Jakub and yourself and think this
is fine for the trunk, even at this late stage as it's only a testsuite change. 
I'll push it momentarily.

Jeff

ps.  In case anyone is wondering, I already had this in my tester and I'm trying
to flush out stuff.
diff mbox series

Patch

diff --git a/gcc/testsuite/g++.dg/pr80481.C b/gcc/testsuite/g++.dg/pr80481.C
index bd5002c89d5137012f36f16946eec3d4ea7dc97c..1f2697825cd854287a7209328ff5e6e90fbab481 100644
--- a/gcc/testsuite/g++.dg/pr80481.C
+++ b/gcc/testsuite/g++.dg/pr80481.C
@@ -1,6 +1,8 @@ 
 // { dg-do compile { target { i?86-*-* x86_64-*-* }  && { ! *-*-solaris* } } }
 // { dg-options "-Ofast -funroll-loops -fopenmp -march=knl" }
 // { dg-final { scan-assembler-not "vmovaps" } }
+// Disabling epilogues until we find a better way to deal with scans.
+// { dg-additional-options "--param vect-epilogues-nomask=0" }
 
 #include <math.h>