On Thu, 2019-07-18 at 08:37 +0100, Richard Sandiford wrote:
>
> > 2019-07-17 Steve Ellcey <sell...@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
sell...@marvell.com
