This is the mail archive of the
gcc-patches@gcc.gnu.org
mailing list for the GCC project.
Re: [PATCH] Fix simd attribute handling on aarch64
- From: Steve Ellcey <sellcey at marvell dot com>
- To: "richard dot sandiford at arm dot com" <richard dot sandiford at arm dot com>
- Cc: "gcc-patches at gcc dot gnu dot org" <gcc-patches at gcc dot gnu dot org>, "Richard dot Earnshaw at arm dot com" <Richard dot Earnshaw at arm dot com>, "Szabolcs dot Nagy at arm dot com" <Szabolcs dot Nagy at arm dot com>
- Date: Thu, 18 Jul 2019 15:48:26 +0000
- Subject: Re: [PATCH] Fix simd attribute handling on aarch64
- Arc-authentication-results: i=1; mx.microsoft.com 1;spf=pass smtp.mailfrom=marvell.com;dmarc=pass action=none header.from=marvell.com;dkim=pass header.d=marvell.com;arc=none
- Arc-message-signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=microsoft.com; s=arcselector9901; h=From:Date:Subject:Message-ID:Content-Type:MIME-Version:X-MS-Exchange-SenderADCheck; bh=U6MUvVZzjVz/YsesoBXsaDBBct3J6tps0OidkQ0u5lY=; b=fq7D80FpLaf+DNeUlKzihZ90emLMWmux92o6XxT/Bpso0BH8+DppisbugiktX2vwebpvD7wgOvAX5kQlQ6OggdupX5Ubd0JqwSwoRWzwVTWil2G6PodT4eAuPzLKjVDICl792VhflygCFaEa57+WqtAdWJGhD0Hs1CibqQbyN9LBcMPrjUa1Egj69DuMF843Ljc8JzIpD+DVLgwhHDgdURIDKkKd6giyIf0xF0sTK+VmJmDG97w5+4yoyVm9OUriegkQqvoO0kmR9ReoO9R/TTwQYvkjMtA9T4QFCqU6EEKuxm3Lytk4nYgkGX4PCPjHDVIUMQaIwUjbbcngkPI0RA==
- Arc-seal: i=1; a=rsa-sha256; s=arcselector9901; d=microsoft.com; cv=none; b=Hc9gthY2KZLk0EuguBfihcCclcb4fZhr8TEDzQ/ht8ErO4zCdKnpf/qzHq5T49jMFWA7hF70UFEHqAhKvGiIlYCTtGbos94KVAITTSRe9ZIp5PNLw1ltZjpMJb1RWexgf9YNivUbIrURlsBEmGuqwJmt4XnXjnfO/aV6qz/gfwtXcIIvxQ8x90TU3+yEdFvSt1kxxuYW/BAmkdHgXB+OBYzWsG3VB8xuJUMmczxXMvGQ8IHQC1gYG5AneI7Pfrkmw/0O9RNl6eml84a3I2+D+n1xlQ4BIco26/mjGPmZMG5fd3ZIpNW5ZQMiKG/ECIbfUfot4jDudUDrfvFJr1pSVg==
- References: <97b75bf8798a32e72827a3d021c11fb42d984c8e.camel@marvell.com> <mpttvbj4wum.fsf@arm.com>
On Thu, 2019-07-18 at 08:37 +0100, Richard Sandiford wrote:
>
> > 2019-07-17 Steve Ellcey <sellcey@marvell.com>
> >
> > * omp-simd-clone.c (simd_clone_adjust): Call targetm.simd_clone.adjust
> > after calling simd_clone_adjust_return_type.
> > (expand_simd_clones): Ditto.
>
> It should be pretty easy to add a test for this, now that we use
> .variant_pcs to mark symbols with the attribute.
OK, I will add some tests that makes sure this mark is not on the
scalar version of a simd function.
> > diff --git a/gcc/omp-simd-clone.c b/gcc/omp-simd-clone.c
> > index caa8da3cba5..6a6b439d146 100644
> > --- a/gcc/omp-simd-clone.c
> > +++ b/gcc/omp-simd-clone.c
> > @@ -1164,9 +1164,8 @@ simd_clone_adjust (struct cgraph_node *node)
> > {
> > push_cfun (DECL_STRUCT_FUNCTION (node->decl));
> >
> > - targetm.simd_clone.adjust (node);
> > -
> > tree retval = simd_clone_adjust_return_type (node);
> > + targetm.simd_clone.adjust (node);
> > ipa_parm_adjustment_vec adjustments
> > = simd_clone_adjust_argument_types (node);
> >
> > @@ -1737,8 +1736,8 @@ expand_simd_clones (struct cgraph_node *node)
> > simd_clone_adjust (n);
> > else
> > {
> > - targetm.simd_clone.adjust (n);
> > simd_clone_adjust_return_type (n);
> > + targetm.simd_clone.adjust (n);
> > simd_clone_adjust_argument_types (n);
> > }
> > }
>
> I don't think this is enough, since simd_clone_adjust_return_type
> does nothing for functions that return void (e.g. sincos).
> I think instead aarch64_simd_clone_adjust should do something like:
>
> TREE_TYPE (node->decl) = build_distinct_type_copy (TREE_TYPE (node-
> >decl));
>
> But maybe that has consequences that I've not thought about...
I think that would work, but it would build two distinct types for non-
void functions, one of which would be unused/uneeded. I.e.
aarch64_simd_clone_adjust would create a distinct type and then
simd_clone_adjust_return_type would create another distinct type
and the previous one would no longer be used anywhere.
What do you think about moving the call to build_distinct_type_copy
out of simd_clone_adjust_return_type and doing it even for null
types. Below is what I am thinking about (untested). I suppose
we could also leave the call to build_distinct_type_copy in
simd_clone_adjust_return_type but just move it above the check
for a NULL type so that a distinct type is always created there.
That would still require that we change the order of the
targetm.simd_clone.adjust and simd_clone_adjust_return_type
calls as my original patch does.
diff --git a/gcc/omp-simd-clone.c b/gcc/omp-simd-clone.c
index caa8da3cba5..427d6f6f514 100644
--- a/gcc/omp-simd-clone.c
+++ b/gcc/omp-simd-clone.c
@@ -498,7 +498,6 @@ simd_clone_adjust_return_type (struct cgraph_node
*node)
/* Adjust the function return type. */
if (orig_rettype == void_type_node)
return NULL_TREE;
- TREE_TYPE (fndecl) = build_distinct_type_copy (TREE_TYPE (fndecl));
t = TREE_TYPE (TREE_TYPE (fndecl));
if (INTEGRAL_TYPE_P (t) || POINTER_TYPE_P (t))
veclen = node->simdclone->vecsize_int;
@@ -1164,6 +1163,7 @@ simd_clone_adjust (struct cgraph_node *node)
{
push_cfun (DECL_STRUCT_FUNCTION (node->decl));
+ TREE_TYPE (node->decl) = build_distinct_type_copy (TREE_TYPE (node-
>decl));
targetm.simd_clone.adjust (node);
tree retval = simd_clone_adjust_return_type (node);
@@ -1737,6 +1737,8 @@ expand_simd_clones (struct cgraph_node *node)
simd_clone_adjust (n);
else
{
+ TREE_TYPE (n->decl)
+ = build_distinct_type_copy (TREE_TYPE (n->decl));
targetm.simd_clone.adjust (n);
simd_clone_adjust_return_type (n);
simd_clone_adjust_argument_types (n);
Steve Ellcey
sellcey@marvell.com