diff mbox

[testsuite] Change xfail conditions for bb-slp-34.c

Message ID yddfuvzamw1.fsf@lokon.CeBiTec.Uni-Bielefeld.DE
State New
Headers show

Commit Message

Rainer Orth March 9, 2016, 11:53 a.m. UTC 
Richard Biener <rguenther@suse.de> writes:

> On Thu, 3 Mar 2016, James Greenhalgh wrote:
>
>> 
>> Hi,
>> 
>> ARM and AArch64 will still vectorize bb-slp-34.c - we're not operating
>> with a cost model so we vectorize to a 64-bit vector of two ints, and the
>> permutes are just element swaps.
>> 
>> So, don't mark this test xfail for arm/aarch64.
>> 
>> Checked on x86_64-none-linux-gnu, arm-none-eabi and aarch64-none-elf with
>> no issues.
>> 
>> OK?
>
> Ok.  Indeed with using V2SI vectors the vectorization is valid. 

Unfortunately, the patch broke the test on sparc:

FAIL: gcc.dg/vect/bb-slp-34.c -flto -ffat-lto-objects  scan-tree-dump slp2 "basic block vectorized"
FAIL: gcc.dg/vect/bb-slp-34.c scan-tree-dump slp2 "basic block vectorized"

While before the scan-tree-dump wasn't run due to missing vect_perm
support, it now fails.

I believe you meant to just not xfail the test on aarch64* and arm*,
which is what the following patch does.  James, could you please test
it?

Thanks.
        Rainer


2016-03-09  Rainer Orth  <ro@CeBiTec.Uni-Bielefeld.DE>

	* gcc.dg/vect/bb-slp-34.c: Really don't xfail on aarch64-*-*,
	arm-*-*.

Comments

James Greenhalgh March 9, 2016, 12:12 p.m. UTC | #1 
On Wed, Mar 09, 2016 at 12:53:02PM +0100, Rainer Orth wrote:
> Richard Biener <rguenther@suse.de> writes:
> 
> > On Thu, 3 Mar 2016, James Greenhalgh wrote:
> >
> >> 
> >> Hi,
> >> 
> >> ARM and AArch64 will still vectorize bb-slp-34.c - we're not operating
> >> with a cost model so we vectorize to a 64-bit vector of two ints, and the
> >> permutes are just element swaps.
> >> 
> >> So, don't mark this test xfail for arm/aarch64.
> >> 
> >> Checked on x86_64-none-linux-gnu, arm-none-eabi and aarch64-none-elf with
> >> no issues.
> >> 
> >> OK?
> >
> > Ok.  Indeed with using V2SI vectors the vectorization is valid. 
> 
> Unfortunately, the patch broke the test on sparc:
> 
> FAIL: gcc.dg/vect/bb-slp-34.c -flto -ffat-lto-objects  scan-tree-dump slp2 "basic block vectorized"
> FAIL: gcc.dg/vect/bb-slp-34.c scan-tree-dump slp2 "basic block vectorized"
> 
> While before the scan-tree-dump wasn't run due to missing vect_perm
> support, it now fails.
> 
> I believe you meant to just not xfail the test on aarch64* and arm*,

Indeed.

> which is what the following patch does.  James, could you please test
> it?

Yes, that still does the right thing for me on aarch64-none-elf, and I think
I now understand why.

Before I touched it that said:

  /* { dg-final { scan-tree-dump "basic block vectorized" "slp2" { target vect_perm xfail *-*-* } } } */

Which means "only run this statement for vect_perm targets, but expect it
to fail on all targets".

Then I changed it to say :

  /* { dg-final { scan-tree-dump "basic block vectorized" "slp2" { xfail { { vect_perm } && { ! { aarch64*-*-* arm*-*-* } } } } } } */

Which means "always run this, but expect it to fail on vect_perm targets that
are not arm/aarch64".

Now you've changed it to say:

  /* { dg-final { scan-tree-dump "basic block vectorized" "slp2" { target vect_perm xfail { ! { aarch64*-*-* arm*-*-* } } } } } */

Which means "only run this for vect perm targets, and expect it to fail
if the target is not arm/aarch64.

Thanks for the patch, it looks right to me and is what I was originally
trying to write. Sorry for the sparc (and presumably other
vect_int && !vect_perm targets) break.

Thanks,
James
Rainer Orth March 9, 2016, 1:11 p.m. UTC | #2 
Hi James,

>> Unfortunately, the patch broke the test on sparc:
>> 
>> FAIL: gcc.dg/vect/bb-slp-34.c -flto -ffat-lto-objects scan-tree-dump slp2
>> "basic block vectorized"
>> FAIL: gcc.dg/vect/bb-slp-34.c scan-tree-dump slp2 "basic block vectorized"
>> 
>> While before the scan-tree-dump wasn't run due to missing vect_perm
>> support, it now fails.
>> 
>> I believe you meant to just not xfail the test on aarch64* and arm*,
>
> Indeed.
>
>> which is what the following patch does.  James, could you please test
>> it?
>
> Yes, that still does the right thing for me on aarch64-none-elf, and I think
> I now understand why.
>
> Before I touched it that said:
>
>   /* { dg-final { scan-tree-dump "basic block vectorized" "slp2" { target
> vect_perm xfail *-*-* } } } */
>
> Which means "only run this statement for vect_perm targets, but expect it
> to fail on all targets".
>
> Then I changed it to say :
>
>   /* { dg-final { scan-tree-dump "basic block vectorized" "slp2" { xfail {
> { vect_perm } && { ! { aarch64*-*-* arm*-*-* } } } } } } */
>
> Which means "always run this, but expect it to fail on vect_perm targets that
> are not arm/aarch64".
>
> Now you've changed it to say:
>
>   /* { dg-final { scan-tree-dump "basic block vectorized" "slp2" { target
> vect_perm xfail { ! { aarch64*-*-* arm*-*-* } } } } } */
>
> Which means "only run this for vect perm targets, and expect it to fail
> if the target is not arm/aarch64.
>
> Thanks for the patch, it looks right to me and is what I was originally
> trying to write. Sorry for the sparc (and presumably other
> vect_int && !vect_perm targets) break.

no worries, and thanks for the confirmation.  I know this
effective-target keyword business can be hard to get right ;-(

I've now installed the patch.

	Rainer
diff mbox

Patch

# HG changeset patch
# Parent  2b37d3d00ad8084dfecd2fc2165d20fffc47e5bf
Really don't xfail gcc.dg/vect/bb-slp-34.c on aarch64, arm

diff --git a/gcc/testsuite/gcc.dg/vect/bb-slp-34.c b/gcc/testsuite/gcc.dg/vect/bb-slp-34.c
--- a/gcc/testsuite/gcc.dg/vect/bb-slp-34.c
+++ b/gcc/testsuite/gcc.dg/vect/bb-slp-34.c
@@ -33,4 +33,4 @@  int main()
 }
 
 /* ??? XFAILed because we access "excess" elements with the permutation.  */
-/* { dg-final { scan-tree-dump "basic block vectorized" "slp2" { xfail { { vect_perm } && { ! { aarch64*-*-* arm*-*-* } } } } } } */
+/* { dg-final { scan-tree-dump "basic block vectorized" "slp2" { target vect_perm xfail { ! { aarch64*-*-* arm*-*-* } } } } } */