Re: [EXT] Re: [Patch 2/4][Aarch64] v2: Implement Aarch64 SIMD ABI
On Wed, 2019-01-23 at 12:50 +, Tamar Christina wrote: > Hi Steve, > > > > > Hi Steve, > > > > No we are using aarch64_be-*-* but this is the only one that popped > > up as a > > failure which is why I didn't change the others. > > But now I'm wondering why... I'll check the log file manually > > tomorrow to be > > sure. > > I've checked and the reason this didn't show up is because we don't > run AArch64 big-endian cross linux tests by default and the baremetal > builds don't support gomp. > > I'm happy to update them for you if you want though. I already updated them. Thanks for checking on the failures. Steve Ellcey sell...@cavium.com
RE: [EXT] Re: [Patch 2/4][Aarch64] v2: Implement Aarch64 SIMD ABI
Hi Steve, > > Hi Steve, > > No we are using aarch64_be-*-* but this is the only one that popped up as a > failure which is why I didn't change the others. > But now I'm wondering why... I'll check the log file manually tomorrow to be > sure. I've checked and the reason this didn't show up is because we don't run AArch64 big-endian cross linux tests by default and the baremetal builds don't support gomp. I'm happy to update them for you if you want though. Cheers, Tamar > > Cheers, > Tamar > > From: Steve Ellcey > Sent: Tuesday, January 22, 2019 10:34 PM > To: Tamar Christina; Richard Sandiford > Cc: gcc-patches@gcc.gnu.org; nd; christophe.l...@linaro.org > Subject: Re: [EXT] Re: [Patch 2/4][Aarch64] v2: Implement Aarch64 SIMD ABI > > On Mon, 2019-01-21 at 18:00 +, Tamar Christina wrote: > > > > > That would need to be aarch64*-*-* to include big-endian. Fixing > > > that here and in the other tests is OK under the obvious rule. > > > > Ah, true, I didn't look at the testcase. I tested the ILP32 case and > > committed the fix for both. > > > > Thanks, > > Tamar > > > > > > > > Adding && lp64 (as per Steve's patch below) is OK too if it works. > > > > > > Thanks, > > > Richard > > Thanks for fixing this test Tamar. I am going to change the others to use > 'aarch64*-*-*' instead of 'aarch64-*-*' since that seems to be the standard > method (though I see a few other tests that are not using > aarch64 instead of aarch64*). > > I guess that while you are testing big-endian the target triplet you are > using is > still aarch64-*-* and not aarch64be-*-*? Otherwise I would have expected > you to have more failures. > > Steve Ellcey > sell...@marvell.com
Re: [EXT] Re: [Patch 2/4][Aarch64] v2: Implement Aarch64 SIMD ABI
Hi Steve, No we are using aarch64_be-*-* but this is the only one that popped up as a failure which is why I didn't change the others. But now I'm wondering why... I'll check the log file manually tomorrow to be sure. Cheers, Tamar From: Steve Ellcey Sent: Tuesday, January 22, 2019 10:34 PM To: Tamar Christina; Richard Sandiford Cc: gcc-patches@gcc.gnu.org; nd; christophe.l...@linaro.org Subject: Re: [EXT] Re: [Patch 2/4][Aarch64] v2: Implement Aarch64 SIMD ABI On Mon, 2019-01-21 at 18:00 +, Tamar Christina wrote: > > > That would need to be aarch64*-*-* to include big-endian. Fixing that here > > and in the other tests is OK under the obvious rule. > > Ah, true, I didn't look at the testcase. I tested the ILP32 case and > committed the fix for both. > > Thanks, > Tamar > > > > > Adding && lp64 (as per Steve's patch below) is OK too if it works. > > > > Thanks, > > Richard Thanks for fixing this test Tamar. I am going to change the others to use 'aarch64*-*-*' instead of 'aarch64-*-*' since that seems to be the standard method (though I see a few other tests that are not using aarch64 instead of aarch64*). I guess that while you are testing big-endian the target triplet you are using is still aarch64-*-* and not aarch64be-*-*? Otherwise I would have expected you to have more failures. Steve Ellcey sell...@marvell.com
Re: [EXT] Re: [Patch 2/4][Aarch64] v2: Implement Aarch64 SIMD ABI
On Mon, 2019-01-21 at 18:00 +, Tamar Christina wrote: > > > That would need to be aarch64*-*-* to include big-endian. Fixing that here > > and in the other tests is OK under the obvious rule. > > Ah, true, I didn't look at the testcase. I tested the ILP32 case and > committed the fix for both. > > Thanks, > Tamar > > > > > Adding && lp64 (as per Steve's patch below) is OK too if it works. > > > > Thanks, > > Richard Thanks for fixing this test Tamar. I am going to change the others to use 'aarch64*-*-*' instead of 'aarch64-*-*' since that seems to be the standard method (though I see a few other tests that are not using aarch64 instead of aarch64*). I guess that while you are testing big-endian the target triplet you are using is still aarch64-*-* and not aarch64be-*-*? Otherwise I would have expected you to have more failures. Steve Ellcey sell...@marvell.com
RE: [EXT] Re: [Patch 2/4][Aarch64] v2: Implement Aarch64 SIMD ABI
Hi Richard,
> -Original Message-
> From: Richard Sandiford
> Sent: Monday, January 21, 2019 16:42
> To: Tamar Christina
> Cc: Steve Ellcey ; christophe.l...@linaro.org; gcc-
> patc...@gcc.gnu.org; nd
> Subject: Re: [EXT] Re: [Patch 2/4][Aarch64] v2: Implement Aarch64 SIMD ABI
>
> Tamar Christina writes:
> > Hi All,
> >
> > The simd-clone-7.cc tests seem to fail on big-endian with
> >
> > testsuite/g++.dg/vect/simd-clone-7.cc:7:1: warning: GCC does not
> > currently support mixed size types for 'simd' functions
> >
> > The test probably miss an effective target check?
>
> The current condition is:
>
> // { dg-warning "GCC does not currently support mixed size types for 'simd'
> functions" "" { target aarch64-*-* } .-4 }
>
> That would need to be aarch64*-*-* to include big-endian. Fixing that here
> and in the other tests is OK under the obvious rule.
Ah, true, I didn't look at the testcase. I tested the ILP32 case and committed
the fix for both.
Thanks,
Tamar
>
> Adding && lp64 (as per Steve's patch below) is OK too if it works.
>
> Thanks,
> Richard
>
> >> -Original Message-
> >> From: gcc-patches-ow...@gcc.gnu.org
> >> On Behalf Of Steve Ellcey
> >> Sent: Friday, January 18, 2019 17:58
> >> To: christophe.l...@linaro.org
> >> Cc: gcc-patches@gcc.gnu.org; Richard Sandiford
> >>
> >> Subject: Re: [EXT] Re: [Patch 2/4][Aarch64] v2: Implement Aarch64
> >> SIMD ABI
> >>
> >> On Fri, 2019-01-18 at 15:35 +0100, Christophe Lyon wrote:
> >> >
> >> > Hi Steve,
> >> >
> >> > I've noticed that
> >> > FAIL: g++.dg/vect/simd-clone-7.cc -std=c++14 (test for warnings,
> >> > line 7) (and for c++17 and c++98) when forcing -mabi=ilp32.
> >> >
> >> > I suspect you want to skip the test in this case?
> >> >
> >> > Christophe
> >>
> >> Actually, I think we can compile that test, it just would not
> >> generate a warning in ILP32 mode because int, floats and pointers
> >> would now all be the same size. So I think the fix is:
> >>
> >>
> >> % git diff simd-clone-7.cc
> >> diff --git a/gcc/testsuite/g++.dg/vect/simd-clone-7.cc
> >> b/gcc/testsuite/g++.dg/vect/simd-clone-7.cc
> >> index c2a63cd5f8e..3617f0ab6a7 100644
> >> --- a/gcc/testsuite/g++.dg/vect/simd-clone-7.cc
> >> +++ b/gcc/testsuite/g++.dg/vect/simd-clone-7.cc
> >> @@ -8,4 +8,4 @@ bar (float x, float *y, int) {
> >>return y[0] + y[1] * x;
> >> }
> >> -// { dg-warning "GCC does not currently support mixed size types for
> 'simd'
> >> functions" "" { target aarch64-*-* } .-4 }
> >> +// { dg-warning "GCC does not currently support mixed size types for
> >> +'simd' functions" "" { target { { aarch64-*-* } && lp64 } } .-4 }
> >>
> >>
> >> I haven't tested this, I don't have an ILP32 build sitting around right
> >> now.
> >> Does it work for you? I can build a toolchain, test it, and submit a
> >> patch if you want.
> >>
> >>
> >> Steve Ellcey
> >> sell...@marvell.com
Re: [EXT] Re: [Patch 2/4][Aarch64] v2: Implement Aarch64 SIMD ABI
Tamar Christina writes:
> Hi All,
>
> The simd-clone-7.cc tests seem to fail on big-endian with
>
> testsuite/g++.dg/vect/simd-clone-7.cc:7:1: warning: GCC does not currently
> support mixed size types for 'simd' functions
>
> The test probably miss an effective target check?
The current condition is:
// { dg-warning "GCC does not currently support mixed size types for 'simd'
functions" "" { target aarch64-*-* } .-4 }
That would need to be aarch64*-*-* to include big-endian. Fixing that
here and in the other tests is OK under the obvious rule.
Adding && lp64 (as per Steve's patch below) is OK too if it works.
Thanks,
Richard
>> -Original Message-
>> From: gcc-patches-ow...@gcc.gnu.org
>> On Behalf Of Steve Ellcey
>> Sent: Friday, January 18, 2019 17:58
>> To: christophe.l...@linaro.org
>> Cc: gcc-patches@gcc.gnu.org; Richard Sandiford
>>
>> Subject: Re: [EXT] Re: [Patch 2/4][Aarch64] v2: Implement Aarch64 SIMD ABI
>>
>> On Fri, 2019-01-18 at 15:35 +0100, Christophe Lyon wrote:
>> >
>> > Hi Steve,
>> >
>> > I've noticed that
>> > FAIL: g++.dg/vect/simd-clone-7.cc -std=c++14 (test for warnings,
>> > line 7) (and for c++17 and c++98) when forcing -mabi=ilp32.
>> >
>> > I suspect you want to skip the test in this case?
>> >
>> > Christophe
>>
>> Actually, I think we can compile that test, it just would not generate a
>> warning in ILP32 mode because int, floats and pointers would now all be the
>> same size. So I think the fix is:
>>
>>
>> % git diff simd-clone-7.cc
>> diff --git a/gcc/testsuite/g++.dg/vect/simd-clone-7.cc
>> b/gcc/testsuite/g++.dg/vect/simd-clone-7.cc
>> index c2a63cd5f8e..3617f0ab6a7 100644
>> --- a/gcc/testsuite/g++.dg/vect/simd-clone-7.cc
>> +++ b/gcc/testsuite/g++.dg/vect/simd-clone-7.cc
>> @@ -8,4 +8,4 @@ bar (float x, float *y, int) {
>>return y[0] + y[1] * x;
>> }
>> -// { dg-warning "GCC does not currently support mixed size types for 'simd'
>> functions" "" { target aarch64-*-* } .-4 }
>> +// { dg-warning "GCC does not currently support mixed size types for
>> +'simd' functions" "" { target { { aarch64-*-* } && lp64 } } .-4 }
>>
>>
>> I haven't tested this, I don't have an ILP32 build sitting around right now.
>> Does it work for you? I can build a toolchain, test it, and submit a patch
>> if you
>> want.
>>
>>
>> Steve Ellcey
>> sell...@marvell.com
RE: [EXT] Re: [Patch 2/4][Aarch64] v2: Implement Aarch64 SIMD ABI
Hi All,
The simd-clone-7.cc tests seem to fail on big-endian with
testsuite/g++.dg/vect/simd-clone-7.cc:7:1: warning: GCC does not currently
support mixed size types for 'simd' functions
The test probably miss an effective target check?
Cheers,
Tamar
> -Original Message-
> From: gcc-patches-ow...@gcc.gnu.org
> On Behalf Of Steve Ellcey
> Sent: Friday, January 18, 2019 17:58
> To: christophe.l...@linaro.org
> Cc: gcc-patches@gcc.gnu.org; Richard Sandiford
>
> Subject: Re: [EXT] Re: [Patch 2/4][Aarch64] v2: Implement Aarch64 SIMD ABI
>
> On Fri, 2019-01-18 at 15:35 +0100, Christophe Lyon wrote:
> >
> > Hi Steve,
> >
> > I've noticed that
> > FAIL: g++.dg/vect/simd-clone-7.cc -std=c++14 (test for warnings,
> > line 7) (and for c++17 and c++98) when forcing -mabi=ilp32.
> >
> > I suspect you want to skip the test in this case?
> >
> > Christophe
>
> Actually, I think we can compile that test, it just would not generate a
> warning in ILP32 mode because int, floats and pointers would now all be the
> same size. So I think the fix is:
>
>
> % git diff simd-clone-7.cc
> diff --git a/gcc/testsuite/g++.dg/vect/simd-clone-7.cc
> b/gcc/testsuite/g++.dg/vect/simd-clone-7.cc
> index c2a63cd5f8e..3617f0ab6a7 100644
> --- a/gcc/testsuite/g++.dg/vect/simd-clone-7.cc
> +++ b/gcc/testsuite/g++.dg/vect/simd-clone-7.cc
> @@ -8,4 +8,4 @@ bar (float x, float *y, int) {
>return y[0] + y[1] * x;
> }
> -// { dg-warning "GCC does not currently support mixed size types for 'simd'
> functions" "" { target aarch64-*-* } .-4 }
> +// { dg-warning "GCC does not currently support mixed size types for
> +'simd' functions" "" { target { { aarch64-*-* } && lp64 } } .-4 }
>
>
> I haven't tested this, I don't have an ILP32 build sitting around right now.
> Does it work for you? I can build a toolchain, test it, and submit a patch
> if you
> want.
>
>
> Steve Ellcey
> sell...@marvell.com
Re: [EXT] Re: [Patch 2/4][Aarch64] v2: Implement Aarch64 SIMD ABI
On Fri, 2019-01-18 at 15:35 +0100, Christophe Lyon wrote:
>
> Hi Steve,
>
> I've noticed that
> FAIL: g++.dg/vect/simd-clone-7.cc -std=c++14 (test for warnings,
> line 7)
> (and for c++17 and c++98)
> when forcing -mabi=ilp32.
>
> I suspect you want to skip the test in this case?
>
> Christophe
Actually, I think we can compile that test, it just would not generate
a warning in ILP32 mode because int, floats and pointers would now all
be the same size. So I think the fix is:
% git diff simd-clone-7.cc
diff --git a/gcc/testsuite/g++.dg/vect/simd-clone-7.cc
b/gcc/testsuite/g++.dg/vect/simd-clone-7.cc
index c2a63cd5f8e..3617f0ab6a7 100644
--- a/gcc/testsuite/g++.dg/vect/simd-clone-7.cc
+++ b/gcc/testsuite/g++.dg/vect/simd-clone-7.cc
@@ -8,4 +8,4 @@ bar (float x, float *y, int)
{
return y[0] + y[1] * x;
}
-// { dg-warning "GCC does not currently support mixed size types for 'simd'
functions" "" { target aarch64-*-* } .-4 }
+// { dg-warning "GCC does not currently support mixed size types for 'simd'
functions" "" { target { { aarch64-*-* } && lp64 } } .-4 }
I haven't tested this, I don't have an ILP32 build sitting around right
now. Does it work for you? I can build a toolchain, test it, and
submit a patch if you want.
Steve Ellcey
sell...@marvell.com
Re: [EXT] Re: [Patch 2/4][Aarch64] v2: Implement Aarch64 SIMD ABI
On Thu, 17 Jan 2019 at 20:11, Steve Ellcey wrote: > > On Thu, 2019-01-17 at 09:10 +, Richard Sandiford wrote: > > > > > +static bool supported_simd_type (tree t) > > > > Missing line break after "static bool". > > Fixed. > > > > +static bool currently_supported_simd_type (tree t, tree b) > > > > Same here. > > Fixed. > > > > + return 0; > > > > The return should use tab indentation. > > Fixed. > > > > OK otherwise, thanks. > > > > Richard > > Thanks for the reviews Richard, I made those changes and checked in the > patch. That is the last of the Aarch64 SIMD / Vector ABI patches I > have so everything should be checked in and working now. > Hi Steve, I've noticed that FAIL: g++.dg/vect/simd-clone-7.cc -std=c++14 (test for warnings, line 7) (and for c++17 and c++98) when forcing -mabi=ilp32. I suspect you want to skip the test in this case? Christophe > Steve Ellcey > sell...@marvell.com
Re: [EXT] Re: [Patch 2/4][Aarch64] v2: Implement Aarch64 SIMD ABI
On Thu, 2019-01-17 at 09:10 +, Richard Sandiford wrote: > > > +static bool supported_simd_type (tree t) > > Missing line break after "static bool". Fixed. > > +static bool currently_supported_simd_type (tree t, tree b) > > Same here. Fixed. > > + return 0; > > The return should use tab indentation. Fixed. > > OK otherwise, thanks. > > Richard Thanks for the reviews Richard, I made those changes and checked in the patch. That is the last of the Aarch64 SIMD / Vector ABI patches I have so everything should be checked in and working now. Steve Ellcey sell...@marvell.com
Re: [EXT] Re: [Patch 2/4][Aarch64] v2: Implement Aarch64 SIMD ABI
Steve Ellcey writes:
> Here is an updated version of the GCC patch to enable SIMD functions on
> Aarch64. There are a number of changes from the last patch.
>
> I reduced the shared code changes, there is still one change in shared code
> (omp-simd-clone.c) to call targetm.simd_clone.adjust from expand_simd_clones
> but it now uses the same argument as the existing call. This new call allows
> Aarch64 to add the aarch64_vector_pcs attribute to SIMD clone definitions
> which in turn ensures they use the correct ABI. Previously this target
> function was only called on declarations, not definitions. This change
> affects
> the x86 target so I modified ix86_simd_clone_adjust to return and do nothing
> when called with a definition. This means there is no change in behaviour
> on x86. I did a build and GCC testsuite run on x86 to verify this.
>
> Most of the changes from the previous patch are in the
> aarch64_simd_clone_compute_vecsize_and_simdlen function.
>
> The previous version was heavily based on the x86 function, this one has
> changes to address the issues that were raised in the earlier patch
> and so it no longer looks like the x86 version. I use types instead of modes
> to check for what we can/cannot vectorize and I (try to) differentiate
> between vectors that we are not currently handling (but could later) and
> those that won't ever be handled.
>
> I have also added a testsuite patch to fix regressions in the gcc.dg/gomp
> and g++.dg/gomp tests. There are no regressions with this patch applied.
>
> Steve Ellcey
> sell...@marvell.com
>
> 2018-12-19 Steve Ellcey
>
> * config/aarch64/aarch64.c (cgraph.h): New include.
> (supported_simd_type): New function.
> (currently_supported_simd_type): Ditto.
> (aarch64_simd_clone_compute_vecsize_and_simdlen): Ditto.
> (aarch64_simd_clone_adjust): Ditto.
> (aarch64_simd_clone_usable): Ditto.
> (TARGET_SIMD_CLONE_COMPUTE_VECSIZE_AND_SIMDLEN): New macro.
> (TARGET_SIMD_CLONE_ADJUST): Ditto.
> (TARGET_SIMD_CLONE_USABLE): Ditto.
> * config/i386/i386.c (ix86_simd_clone_adjust): Add definition check.
> * omp-simd-clone.c (expand_simd_clones): Add targetm.simd_clone.adjust
> call.
>
> 2018-12-19 Steve Ellcey
>
> * g++.dg/gomp/declare-simd-1.C: Add aarch64 specific
> warning checks and assembler scans.
> * g++.dg/gomp/declare-simd-3.C: Ditto.
> * g++.dg/gomp/declare-simd-4.C: Ditto.
> * g++.dg/gomp/declare-simd-7.C: Ditto.
> * gcc.dg/gomp/declare-simd-1.c: Ditto.
> * gcc.dg/gomp/declare-simd-3.c: Ditto.
Sorry, hadn't realised this was still unreviewed.
> @@ -18064,6 +18066,138 @@ aarch64_estimated_poly_value (poly_int64 val)
>return val.coeffs[0] + val.coeffs[1] * over_128 / 128;
> }
>
> +
> +/* Return true for types that could be supported as SIMD return or
> + argument types. */
> +
> +static bool supported_simd_type (tree t)
> +{
> + return (FLOAT_TYPE_P (t) || INTEGRAL_TYPE_P (t));
We should also check that the size is 1, 2, 4 or 8 bytes.
> +}
> +
> +/* Return true for types that currently are supported as SIMD return
> + or argument types. */
> +
> +static bool currently_supported_simd_type (tree t)
> +{
> + if (COMPLEX_FLOAT_TYPE_P (t))
> +return false;
> +
> + return supported_simd_type (t);
> +}
> +
> +/* Implement TARGET_SIMD_CLONE_COMPUTE_VECSIZE_AND_SIMDLEN. */
> +
> +static int
> +aarch64_simd_clone_compute_vecsize_and_simdlen (struct cgraph_node *node,
> + struct cgraph_simd_clone *clonei,
> + tree base_type,
> + int num ATTRIBUTE_UNUSED)
> +{
> + const char *wmsg;
> + int vsize;
> + tree t, ret_type, arg_type;
> +
> + if (!TARGET_SIMD)
> +return 0;
> +
> + if (clonei->simdlen
> + && (clonei->simdlen < 2
> + || clonei->simdlen > 1024
> + || (clonei->simdlen & (clonei->simdlen - 1)) != 0))
> +{
> + warning_at (DECL_SOURCE_LOCATION (node->decl), 0,
> + "unsupported simdlen %d", clonei->simdlen);
> + return 0;
> +}
> +
> + ret_type = TREE_TYPE (TREE_TYPE (node->decl));
> + if (TREE_CODE (ret_type) != VOID_TYPE
> + && !currently_supported_simd_type (ret_type))
> +{
> + if (supported_simd_type (ret_type))
> + wmsg = G_("GCC does not currently support return type %qT for simd");
> + else
> + wmsg = G_("unsupported return type return type %qT for simd");
Typo: doubled "return type".
Maybe s/for simd/for % functions/ in both messages.
Since that will blow the line limit...
> + warning_at (DECL_SOURCE_LOCATION (node->decl), 0, wmsg, ret_type);
> + return 0;
> +}
...it's probably worth just calling warning_at in each arm of the "if".
We'll then get proper format checking in bootstraps.
> + for (t = DECL_ARGUMENTS (node->decl); t; t = DECL_CHAIN (t))
> +{
> + arg_type = TREE_TYPE (t);
> +
Re: [EXT] Re: [Patch 2/4][Aarch64] v2: Implement Aarch64 SIMD ABI
On Fri, Dec 21, 2018 at 06:01:56PM +, Steve Ellcey wrote: > Here is an update to the test part of this patch. I did not change the > actual source code part of this, just the tests, so that is all I am > including here. I removed the x86 changes that had gotten in there by > accident and used relative line numbers in the warning checks instead > of absolute line numbers. I also moved the warning checks to be closer > to the lines where the warnings are generated. > > Retested on x86 and aarch64 with no regressions. > > Steve Ellcey > sell...@cavium.com > > > 2018-12-21 Steve Ellcey > > * g++.dg/gomp/declare-simd-1.C: Add aarch64 specific > warning checks and assembler scans. > * g++.dg/gomp/declare-simd-3.C: Ditto. > * g++.dg/gomp/declare-simd-4.C: Ditto. > * g++.dg/gomp/declare-simd-7.C: Ditto. > * gcc.dg/gomp/declare-simd-1.c: Ditto. > * gcc.dg/gomp/declare-simd-3.c: Ditto. LGTM. Jakub
Re: [EXT] Re: [Patch 2/4][Aarch64] v2: Implement Aarch64 SIMD ABI
Here is an update to the test part of this patch. I did not change the
actual source code part of this, just the tests, so that is all I am
including here. I removed the x86 changes that had gotten in there by
accident and used relative line numbers in the warning checks instead
of absolute line numbers. I also moved the warning checks to be closer
to the lines where the warnings are generated.
Retested on x86 and aarch64 with no regressions.
Steve Ellcey
sell...@cavium.com
2018-12-21 Steve Ellcey
* g++.dg/gomp/declare-simd-1.C: Add aarch64 specific
warning checks and assembler scans.
* g++.dg/gomp/declare-simd-3.C: Ditto.
* g++.dg/gomp/declare-simd-4.C: Ditto.
* g++.dg/gomp/declare-simd-7.C: Ditto.
* gcc.dg/gomp/declare-simd-1.c: Ditto.
* gcc.dg/gomp/declare-simd-3.c: Ditto.
diff --git a/gcc/testsuite/g++.dg/gomp/declare-simd-1.C b/gcc/testsuite/g++.dg/gomp/declare-simd-1.C
index d2659e1..f44efd5 100644
--- a/gcc/testsuite/g++.dg/gomp/declare-simd-1.C
+++ b/gcc/testsuite/g++.dg/gomp/declare-simd-1.C
@@ -14,6 +14,7 @@ int f2 (int a, int *b, int c)
return a + *b + c;
}
+// { dg-warning "GCC does not currently support simdlen 8 for type 'int'" "" { target aarch64-*-* } .-5 }
// { dg-final { scan-assembler-times "_ZGVbM8uva32l4__Z2f2iPii:" 1 { target { i?86-*-* x86_64-*-* } } } }
// { dg-final { scan-assembler-times "_ZGVbN8uva32l4__Z2f2iPii:" 1 { target { i?86-*-* x86_64-*-* } } } }
// { dg-final { scan-assembler-times "_ZGVcM8uva32l4__Z2f2iPii:" 1 { target { i?86-*-* x86_64-*-* } } } }
@@ -89,6 +90,7 @@ namespace N1
// { dg-final { scan-assembler-times "_ZGVdN2va16__ZN2N12N23f10EPx:" 1 { target { i?86-*-* x86_64-*-* } } } }
// { dg-final { scan-assembler-times "_ZGVeM2va16__ZN2N12N23f10EPx:" 1 { target { i?86-*-* x86_64-*-* } } } }
// { dg-final { scan-assembler-times "_ZGVeN2va16__ZN2N12N23f10EPx:" 1 { target { i?86-*-* x86_64-*-* } } } }
+// { dg-final { scan-assembler-times "_ZN2N12N23f10EPx:" 1 { target { aarch64-*-* } } } }
struct A
{
@@ -191,6 +193,7 @@ int B::f25<7> (int a, int *b, int c)
return a + *b + c;
}
+// { dg-warning "unsupported argument type 'B' for simd" "" { target aarch64-*-* } .-5 }
// { dg-final { scan-assembler-times "_ZGVbM8vuva32u__ZN1BIiE3f25ILi7EEEiiPii:" 1 { target { i?86-*-* x86_64-*-* } } } }
// { dg-final { scan-assembler-times "_ZGVbN8vuva32u__ZN1BIiE3f25ILi7EEEiiPii:" 1 { target { i?86-*-* x86_64-*-* } } } }
// { dg-final { scan-assembler-times "_ZGVcM8vuva32u__ZN1BIiE3f25ILi7EEEiiPii:" 1 { target { i?86-*-* x86_64-*-* } } } }
@@ -216,6 +219,7 @@ int B::f26<-1> (int a, int *b, int c)
// { dg-final { scan-assembler-times "_ZGVdN4vl2va32__ZN1BIiE3f26ILin1EEEiiPii:" 1 { target { i?86-*-* x86_64-*-* } } } }
// { dg-final { scan-assembler-times "_ZGVeM4vl2va32__ZN1BIiE3f26ILin1EEEiiPii:" 1 { target { i?86-*-* x86_64-*-* } } } }
// { dg-final { scan-assembler-times "_ZGVeN4vl2va32__ZN1BIiE3f26ILin1EEEiiPii:" 1 { target { i?86-*-* x86_64-*-* } } } }
+// { dg-final { scan-assembler-times "_ZN1BIiE3f26ILin1EEEiiPii:" 1 { target { aarch64-*-* } } } }
int
f27 (int x)
@@ -239,6 +243,7 @@ f30 (int x)
return x;
}
+// { dg-warning "GCC does not currently support simdlen 16 for type 'int'" "" { target aarch64-*-* } .-7 }
// { dg-final { scan-assembler-times "_ZGVbM16v__Z3f30i:" 1 { target { i?86-*-* x86_64-*-* } } } }
// { dg-final { scan-assembler-times "_ZGVbN16v__Z3f30i:" 1 { target { i?86-*-* x86_64-*-* } } } }
// { dg-final { scan-assembler-times "_ZGVcM16v__Z3f30i:" 1 { target { i?86-*-* x86_64-*-* } } } }
@@ -281,6 +286,7 @@ struct D
int f37 (int a);
int e;
};
+// { dg-warning "GCC does not currently support simdlen 16 for type 'int'" "" { target aarch64-*-* } .-3 }
void
f38 (D &d)
diff --git a/gcc/testsuite/g++.dg/gomp/declare-simd-3.C b/gcc/testsuite/g++.dg/gomp/declare-simd-3.C
index 32cdc58..3d668ff 100644
--- a/gcc/testsuite/g++.dg/gomp/declare-simd-3.C
+++ b/gcc/testsuite/g++.dg/gomp/declare-simd-3.C
@@ -21,6 +21,8 @@ int f1 (int a, int b, int c, int &d, int &e, int &f)
// { dg-final { scan-assembler-times "_ZGVdN8vulLUR4__Z2f1iiiRiS_S_:" 1 { target { i?86-*-* x86_64-*-* } } } }
// { dg-final { scan-assembler-times "_ZGVeM16vulLUR4__Z2f1iiiRiS_S_:" 1 { target { i?86-*-* x86_64-*-* } } } }
// { dg-final { scan-assembler-times "_ZGVeN16vulLUR4__Z2f1iiiRiS_S_:" 1 { target { i?86-*-* x86_64-*-* } } } }
+// { dg-final { scan-assembler-times "_ZGVnN4vulLUR4__Z2f1iiiRiS_S_:" 1 { target { aarch64-*-* } } } }
+
#pragma omp declare simd uniform(b) linear(c, d) linear(uval(e)) linear(ref(f))
int f2 (int a, int b, int c, int &d, int &e, int &f)
@@ -48,6 +50,7 @@ int f2 (int a, int b, int c, int &d, int &e, int &f)
// { dg-final { scan-assembler-times "_ZGVdN8vulLUR4__Z2f2iiiRiS_S_:" 1 { target { i?86-*-* x86_64-*-* } } } }
// { dg-final { scan-assembler-times "_ZGVeM16vulLUR4__Z2f2iiiRiS_S_:" 1 { target { i?86-*-* x86_64-*-*
Re: [EXT] Re: [Patch 2/4][Aarch64] v2: Implement Aarch64 SIMD ABI
On Wed, 2018-12-19 at 23:57 +0100, Jakub Jelinek wrote:
> On Wed, Dec 19, 2018 at 10:10:19PM +, Steve Ellcey wrote:
> > @@ -199,6 +201,7 @@ int B::f25<7> (int a, int *b, int c)
> > // { dg-final { scan-assembler-times
> > "_ZGVdN8vuva32u__ZN1BIiE3f25ILi7EEEiiPii:" 1 { target { i?86-*-*
> > x86_64-*-* } } } }
> > // { dg-final { scan-assembler-times
> > "_ZGVeM8vuva32u__ZN1BIiE3f25ILi7EEEiiPii:" 1 { target { i?86-*-*
> > x86_64-*-* } } } }
> > // { dg-final { scan-assembler-times
> > "_ZGVeN8vuva32u__ZN1BIiE3f25ILi7EEEiiPii:" 1 { target { i?86-*-*
> > x86_64-*-* } } } }
> > +// { dg-warning "unsupported argument type 'B' for simd" "" {
> > target aarch64-*-* } 191 }
>
> Can you use relative line number instead, like .-10 or so?
That sounds like a good idea.
>
> > @@ -62,7 +65,7 @@ int f3 (const int a, const int b, const int c,
> > const int &d, const int &e, const
> > // { dg-final { scan-assembler-times
> > "_ZGVdM8vulLUR4__Z2f3iiiRKiS0_S0_:" 1 { target { i?86-*-* x86_64-*-
> > * } } } }
> > // { dg-final { scan-assembler-times
> > "_ZGVdN8vulLUR4__Z2f3iiiRKiS0_S0_:" 1 { target { i?86-*-* x86_64-*-
> > * } } } }
> > // { dg-final { scan-assembler-times
> > "_ZGVeM16vulLUR4__Z2f3iiiRKiS0_S0_:" 1 { target { i?86-*-* x86_64-
> > *-* } } } }
> > -// { dg-final { scan-assembler-times
> > "_ZGVeN16vulLUR4__Z2f3iiiRKiS0_S0_:" 1 { target { i?86-*-* x86_64-
> > *-* } } } }
> > +// { dg-final { scan-assembler-times
> > "_ZGVeN4vulLUR4__Z2f3iiiRKiS0_S0_:" 1 { target { i?86-*-* x86_64-*-
> > * } } } }
>
> Can you explain this change? Are you changing the x86 ABI?
No, that is a mistake that snuck in. None of the x86 lines should
change. Same for the other x86 changes. I was changing the aarch64
manglings and obviously messed up some of the x86 ones. Unfortunately
I did those changes after I did my x86 testing to verify the x86
code change I made so I didn't notice them. I will fix those so that
no x86 lines are different.
Steve Ellcey
sell...@marvell.com
Re: [EXT] Re: [Patch 2/4][Aarch64] v2: Implement Aarch64 SIMD ABI
On Wed, Dec 19, 2018 at 10:10:19PM +, Steve Ellcey wrote:
> @@ -199,6 +201,7 @@ int B::f25<7> (int a, int *b, int c)
> // { dg-final { scan-assembler-times
> "_ZGVdN8vuva32u__ZN1BIiE3f25ILi7EEEiiPii:" 1 { target { i?86-*-* x86_64-*-* }
> } } }
> // { dg-final { scan-assembler-times
> "_ZGVeM8vuva32u__ZN1BIiE3f25ILi7EEEiiPii:" 1 { target { i?86-*-* x86_64-*-* }
> } } }
> // { dg-final { scan-assembler-times
> "_ZGVeN8vuva32u__ZN1BIiE3f25ILi7EEEiiPii:" 1 { target { i?86-*-* x86_64-*-* }
> } } }
> +// { dg-warning "unsupported argument type 'B' for simd" "" { target
> aarch64-*-* } 191 }
Can you use relative line number instead, like .-10 or so?
> @@ -62,7 +65,7 @@ int f3 (const int a, const int b, const int c, const int
> &d, const int &e, const
> // { dg-final { scan-assembler-times "_ZGVdM8vulLUR4__Z2f3iiiRKiS0_S0_:" 1 {
> target { i?86-*-* x86_64-*-* } } } }
> // { dg-final { scan-assembler-times "_ZGVdN8vulLUR4__Z2f3iiiRKiS0_S0_:" 1 {
> target { i?86-*-* x86_64-*-* } } } }
> // { dg-final { scan-assembler-times "_ZGVeM16vulLUR4__Z2f3iiiRKiS0_S0_:" 1
> { target { i?86-*-* x86_64-*-* } } } }
> -// { dg-final { scan-assembler-times "_ZGVeN16vulLUR4__Z2f3iiiRKiS0_S0_:" 1
> { target { i?86-*-* x86_64-*-* } } } }
> +// { dg-final { scan-assembler-times "_ZGVeN4vulLUR4__Z2f3iiiRKiS0_S0_:" 1 {
> target { i?86-*-* x86_64-*-* } } } }
Can you explain this change? Are you changing the x86 ABI?
> #pragma omp declare simd uniform(b) linear(c, d) linear(uval(e))
> linear(ref(f))
> int f4 (const int a, const int b, const int c, const int &d, const int &e,
> const int &f)
> @@ -83,4 +86,4 @@ int f4 (const int a, const int b, const int c, const int
> &d, const int &e, const
> // { dg-final { scan-assembler-times "_ZGVdM8vulLUR4__Z2f4iiiRKiS0_S0_:" 1 {
> target { i?86-*-* x86_64-*-* } } } }
> // { dg-final { scan-assembler-times "_ZGVdN8vulLUR4__Z2f4iiiRKiS0_S0_:" 1 {
> target { i?86-*-* x86_64-*-* } } } }
> // { dg-final { scan-assembler-times "_ZGVeM16vulLUR4__Z2f4iiiRKiS0_S0_:" 1
> { target { i?86-*-* x86_64-*-* } } } }
> -// { dg-final { scan-assembler-times "_ZGVeN16vulLUR4__Z2f4iiiRKiS0_S0_:" 1
> { target { i?86-*-* x86_64-*-* } } } }
> +// { dg-final { scan-assembler-times "_ZGVeN4vulLUR4__Z2f4iiiRKiS0_S0_:" 1 {
> target { i?86-*-* x86_64-*-* } } } }
Likewise.
> --- a/gcc/testsuite/g++.dg/gomp/declare-simd-4.C
> +++ b/gcc/testsuite/g++.dg/gomp/declare-simd-4.C
> @@ -13,6 +13,8 @@ f1 (int *p, int *q, short *s)
> // { dg-final { scan-assembler-times "_ZGVdN8l4ln4ln6__Z2f1PiS_Ps:" 1 {
> target { i?86-*-* x86_64-*-* } } } }
> // { dg-final { scan-assembler-times "_ZGVeM16l4ln4ln6__Z2f1PiS_Ps:" 1 {
> target { i?86-*-* x86_64-*-* } } } }
> // { dg-final { scan-assembler-times "_ZGVeN16l4ln4ln6__Z2f1PiS_Ps:" 1 {
> target { i?86-*-* x86_64-*-* } } } }
> +// { dg-final { scan-assembler-times "_ZGVnM4l4ln4ln6__Z2f1PiS_Ps:" 1 {
> target { i?86-*-* x86_64-*-* } } } }
> +// { dg-final { scan-assembler-times "_ZGVnN4l4ln4ln6__Z2f1PiS_Ps:" 1 {
> target { i?86-*-* x86_64-*-* } } } }
This will also surely fail on x86.
> @@ -21,6 +21,7 @@ int f2 (int a, int *b, int c)
> /* { dg-final { scan-assembler-times "_ZGVdN8uva32l4_f2:" 1 { target {
> i?86-*-* x86_64-*-* } } } } */
> /* { dg-final { scan-assembler-times "_ZGVeM8uva32l4_f2:" 1 { target {
> i?86-*-* x86_64-*-* } } } } */
> /* { dg-final { scan-assembler-times "_ZGVeN8uva32l4_f2:" 1 { target {
> i?86-*-* x86_64-*-* } } } } */
> +/* { dg-warning "GCC does not currently support simdlen 8 for type 'int'" ""
> { target aarch64-*-* } 11 } */
.-x here too.
Jakub
Re: [EXT] Re: [Patch 2/4][Aarch64] v2: Implement Aarch64 SIMD ABI
Here is an updated version of the GCC patch to enable SIMD functions on
Aarch64. There are a number of changes from the last patch.
I reduced the shared code changes, there is still one change in shared code
(omp-simd-clone.c) to call targetm.simd_clone.adjust from expand_simd_clones
but it now uses the same argument as the existing call. This new call allows
Aarch64 to add the aarch64_vector_pcs attribute to SIMD clone definitions
which in turn ensures they use the correct ABI. Previously this target
function was only called on declarations, not definitions. This change affects
the x86 target so I modified ix86_simd_clone_adjust to return and do nothing
when called with a definition. This means there is no change in behaviour
on x86. I did a build and GCC testsuite run on x86 to verify this.
Most of the changes from the previous patch are in the
aarch64_simd_clone_compute_vecsize_and_simdlen function.
The previous version was heavily based on the x86 function, this one has
changes to address the issues that were raised in the earlier patch
and so it no longer looks like the x86 version. I use types instead of modes
to check for what we can/cannot vectorize and I (try to) differentiate
between vectors that we are not currently handling (but could later) and
those that won't ever be handled.
I have also added a testsuite patch to fix regressions in the gcc.dg/gomp
and g++.dg/gomp tests. There are no regressions with this patch applied.
Steve Ellcey
sell...@marvell.com
2018-12-19 Steve Ellcey
* config/aarch64/aarch64.c (cgraph.h): New include.
(supported_simd_type): New function.
(currently_supported_simd_type): Ditto.
(aarch64_simd_clone_compute_vecsize_and_simdlen): Ditto.
(aarch64_simd_clone_adjust): Ditto.
(aarch64_simd_clone_usable): Ditto.
(TARGET_SIMD_CLONE_COMPUTE_VECSIZE_AND_SIMDLEN): New macro.
(TARGET_SIMD_CLONE_ADJUST): Ditto.
(TARGET_SIMD_CLONE_USABLE): Ditto.
* config/i386/i386.c (ix86_simd_clone_adjust): Add definition check.
* omp-simd-clone.c (expand_simd_clones): Add targetm.simd_clone.adjust
call.
2018-12-19 Steve Ellcey
* g++.dg/gomp/declare-simd-1.C: Add aarch64 specific
warning checks and assembler scans.
* g++.dg/gomp/declare-simd-3.C: Ditto.
* g++.dg/gomp/declare-simd-4.C: Ditto.
* g++.dg/gomp/declare-simd-7.C: Ditto.
* gcc.dg/gomp/declare-simd-1.c: Ditto.
* gcc.dg/gomp/declare-simd-3.c: Ditto.
diff --git a/gcc/config/aarch64/aarch64.c b/gcc/config/aarch64/aarch64.c
index 6038494..e61f6e1 100644
--- a/gcc/config/aarch64/aarch64.c
+++ b/gcc/config/aarch64/aarch64.c
@@ -40,6 +40,7 @@
#include "regs.h"
#include "emit-rtl.h"
#include "recog.h"
+#include "cgraph.h"
#include "diagnostic.h"
#include "insn-attr.h"
#include "alias.h"
@@ -71,6 +72,7 @@
#include "selftest.h"
#include "selftest-rtl.h"
#include "rtx-vector-builder.h"
+#include "intl.h"
/* This file should be included last. */
#include "target-def.h"
@@ -18064,6 +18066,138 @@ aarch64_estimated_poly_value (poly_int64 val)
return val.coeffs[0] + val.coeffs[1] * over_128 / 128;
}
+
+/* Return true for types that could be supported as SIMD return or
+ argument types. */
+
+static bool supported_simd_type (tree t)
+{
+ return (FLOAT_TYPE_P (t) || INTEGRAL_TYPE_P (t));
+}
+
+/* Return true for types that currently are supported as SIMD return
+ or argument types. */
+
+static bool currently_supported_simd_type (tree t)
+{
+ if (COMPLEX_FLOAT_TYPE_P (t))
+return false;
+
+ return supported_simd_type (t);
+}
+
+/* Implement TARGET_SIMD_CLONE_COMPUTE_VECSIZE_AND_SIMDLEN. */
+
+static int
+aarch64_simd_clone_compute_vecsize_and_simdlen (struct cgraph_node *node,
+ struct cgraph_simd_clone *clonei,
+ tree base_type,
+ int num ATTRIBUTE_UNUSED)
+{
+ const char *wmsg;
+ int vsize;
+ tree t, ret_type, arg_type;
+
+ if (!TARGET_SIMD)
+return 0;
+
+ if (clonei->simdlen
+ && (clonei->simdlen < 2
+ || clonei->simdlen > 1024
+ || (clonei->simdlen & (clonei->simdlen - 1)) != 0))
+{
+ warning_at (DECL_SOURCE_LOCATION (node->decl), 0,
+ "unsupported simdlen %d", clonei->simdlen);
+ return 0;
+}
+
+ ret_type = TREE_TYPE (TREE_TYPE (node->decl));
+ if (TREE_CODE (ret_type) != VOID_TYPE
+ && !currently_supported_simd_type (ret_type))
+{
+ if (supported_simd_type (ret_type))
+ wmsg = G_("GCC does not currently support return type %qT for simd");
+ else
+ wmsg = G_("unsupported return type return type %qT for simd");
+ warning_at (DECL_SOURCE_LOCATION (node->decl), 0, wmsg, ret_type);
+ return 0;
+}
+
+ for (t = DECL_ARGUMENTS (node->decl); t; t = DECL_CHAIN (t))
+{
+ arg_type = TREE_TYPE (t);
+ if (POINTER_TYPE_P (arg_type))
+ arg_type = TREE_TYPE (arg_type);
+ if (!currently_supported_simd_type (arg_type))
+ {
+ if (supported_simd_type (arg
Re: [EXT] Re: [Patch 2/4][Aarch64] v2: Implement Aarch64 SIMD ABI
On Wed, 2018-12-12 at 13:41 +0100, Jakub Jelinek wrote: > External Email > > --- > --- > On Wed, Dec 12, 2018 at 12:34:46PM +, Richard Sandiford wrote: > > > I considered comparing node->decl and cfun->decl to differentiate > > > between definitions and declarations instead of using a new > > > argument > > > but having an argument seemed cleaner and clearer. > > > > Yeah, agreed. > > I actually disagree, there is no point in passing another argument. > You should be able to just check node->definition whether it is a > definition > or declaration. > > Jakub You are right, I can just use node->definition and not add the new argument. I will make that change. Steve Ellcey sell...@cavium.com
