public inbox for gcc-patches@gcc.gnu.org
 help / color / mirror / Atom feed
* [PATCH] Fix simd attribute handling on aarch64
@ 2019-07-17 21:29 Steve Ellcey
  2019-07-18  9:22 ` Richard Sandiford
  0 siblings, 1 reply; 12+ messages in thread
From: Steve Ellcey @ 2019-07-17 21:29 UTC (permalink / raw)
  To: gcc-patches; +Cc: Richard.Earnshaw, richard.sandiford, Szabolcs.Nagy

This patch fixes a bug with SIMD functions on Aarch64.  I found it
while trying to run SPEC with ToT GCC and a glibc that defines vector
math functions for aarch64.  When a function is declared with the simd
attribute GCC creates vector clones of that function with the return
and argument types changed to vector types.  On Aarch64 the vector
clones are also marked with the aarch64_vector_pcs attribute to signify
that they use an alternate calling convention.  Due to a bug in GCC the
non-vector version of the function being cloned was also being marked
with this attribute.

Because simd_clone_adjust and expand_simd_clones are calling
targetm.simd_clone.adjust (which attached the aarch64_vector_pcs
attribute to the function type) before calling
simd_clone_adjust_return_type (which created a new distinct type tree
for the cloned function) the attribute got attached to both the
'normal' scalar version of the SIMD function and any vector versions of
the function.  The attribute should only be on the vector versions.

My fix is to call simd_clone_adjust_return_type and create the new type
before calling targetm.simd_clone.adjust which adds the attribute.  The
only other platform that this patch could affect is x86 because that is
the only other platform to use targetm.simd_clone.adjust.  I did a
bootstrap and gcc test run on x86 (as well as Aarch64) and got no
regressions.

OK to checkin?

Steve Ellcey
sellcey@marvell.com


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.


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);
 	    }
 	}



^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH] Fix simd attribute handling on aarch64
  2019-07-17 21:29 [PATCH] Fix simd attribute handling on aarch64 Steve Ellcey
@ 2019-07-18  9:22 ` Richard Sandiford
  2019-07-18 16:04   ` Steve Ellcey
  2019-07-19 15:57   ` [PATCH] Fix simd attribute handling on aarch64 (version 2) Steve Ellcey
  0 siblings, 2 replies; 12+ messages in thread
From: Richard Sandiford @ 2019-07-18  9:22 UTC (permalink / raw)
  To: Steve Ellcey; +Cc: gcc-patches, Richard.Earnshaw, Szabolcs.Nagy

Steve Ellcey <sellcey@marvell.com> writes:
> This patch fixes a bug with SIMD functions on Aarch64.  I found it
> while trying to run SPEC with ToT GCC and a glibc that defines vector
> math functions for aarch64.  When a function is declared with the simd
> attribute GCC creates vector clones of that function with the return
> and argument types changed to vector types.  On Aarch64 the vector
> clones are also marked with the aarch64_vector_pcs attribute to signify
> that they use an alternate calling convention.  Due to a bug in GCC the
> non-vector version of the function being cloned was also being marked
> with this attribute.
>
> Because simd_clone_adjust and expand_simd_clones are calling
> targetm.simd_clone.adjust (which attached the aarch64_vector_pcs
> attribute to the function type) before calling
> simd_clone_adjust_return_type (which created a new distinct type tree
> for the cloned function) the attribute got attached to both the
> 'normal' scalar version of the SIMD function and any vector versions of
> the function.  The attribute should only be on the vector versions.
>
> My fix is to call simd_clone_adjust_return_type and create the new type
> before calling targetm.simd_clone.adjust which adds the attribute.  The
> only other platform that this patch could affect is x86 because that is
> the only other platform to use targetm.simd_clone.adjust.  I did a
> bootstrap and gcc test run on x86 (as well as Aarch64) and got no
> regressions.
>
> OK to checkin?
>
> Steve Ellcey
> sellcey@marvell.com
>
>
> 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.

> 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...

Thanks,
Richard

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH] Fix simd attribute handling on aarch64
  2019-07-18  9:22 ` Richard Sandiford
@ 2019-07-18 16:04   ` Steve Ellcey
  2019-07-19 15:57   ` [PATCH] Fix simd attribute handling on aarch64 (version 2) Steve Ellcey
  1 sibling, 0 replies; 12+ messages in thread
From: Steve Ellcey @ 2019-07-18 16:04 UTC (permalink / raw)
  To: richard.sandiford; +Cc: gcc-patches, Richard.Earnshaw, Szabolcs.Nagy

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

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH] Fix simd attribute handling on aarch64 (version 2)
  2019-07-18  9:22 ` Richard Sandiford
  2019-07-18 16:04   ` Steve Ellcey
@ 2019-07-19 15:57   ` Steve Ellcey
  2019-07-19 18:26     ` Richard Sandiford
  1 sibling, 1 reply; 12+ messages in thread
From: Steve Ellcey @ 2019-07-19 15:57 UTC (permalink / raw)
  To: richard.sandiford; +Cc: gcc-patches, Richard.Earnshaw, Szabolcs.Nagy

[-- Attachment #1: Type: text/plain, Size: 1480 bytes --]

Here is version two of my patch to fix simd attribute handling on
aarch64.  Unlike the first patch where I swapped the order of the
calls to targetm.simd_clone.adjust and simd_clone_adjust_return_type,
in this one I remove the (conditional) call to build_distinct_type_copy
from simd_clone_adjust_return_type and do it unconditionally before
calling either routine.  The only downside to this that I can see is
that on non-aarch64 platforms where the return type of a vector
function is VOID (and not changed), we will create a distinct type
where we did not before.

I also added some tests to ensure that, on aarch64, the vector
functions created by cloning a simd function have the .variant_pcs
directive and that the original non-vector version of the function
does not have the directive.  Without this patch the non-vector
version is putting out the directive, that is what this patch
fixes.

Retested on x86 and aarch64 with no regressions.

OK to checkin?

Steve Ellcey
sellcey@marvell.com


2019-07-19  Steve Ellcey  <sellcey@marvell.com>

	* omp-simd-clone.c (simd_clone_adjust_return_type): Remove call to
	build_distinct_type_copy.
	(simd_clone_adjust): Call build_distinct_type_copy.
	(expand_simd_clones): Ditto.


2019-07-19  Steve Ellcey  <sellcey@marvell.com>

	* gcc.target/aarch64/simd_pcs_attribute.c: New test.
	* gcc.target/aarch64/simd_pcs_attribute-2.c: Ditto.
	* gcc.target/aarch64/simd_pcs_attribute-3.c: Ditto.



[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: gcc-simd.patch --]
[-- Type: text/x-patch; name="gcc-simd.patch", Size: 1153 bytes --]

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);

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #3: gcc-simd-test.patch --]
[-- Type: text/x-patch; name="gcc-simd-test.patch", Size: 2616 bytes --]

diff --git a/gcc/testsuite/gcc.target/aarch64/simd_pcs_attribute-2.c b/gcc/testsuite/gcc.target/aarch64/simd_pcs_attribute-2.c
index e69de29bb2d..913960c607b 100644
--- a/gcc/testsuite/gcc.target/aarch64/simd_pcs_attribute-2.c
+++ b/gcc/testsuite/gcc.target/aarch64/simd_pcs_attribute-2.c
@@ -0,0 +1,17 @@
+/* { dg-do compile } */
+/* { dg-options "-Ofast" } */
+/* { dg-require-effective-target aarch64_variant_pcs } */
+
+__attribute__ ((__simd__ ("notinbranch")))
+__attribute__ ((__nothrow__ , __leaf__ , __const__))
+extern double foo (double x);
+
+void bar(double * f, int n)
+{
+	int i;
+	for (i = 0; i < n; i++)
+		f[i] = foo(f[i]);
+}
+
+/* { dg-final { scan-assembler-not {\.variant_pcs\tfoo} } } */
+/* { dg-final { scan-assembler-times {\.variant_pcs\t_ZGVnN2v_foo} 1 } } */
diff --git a/gcc/testsuite/gcc.target/aarch64/simd_pcs_attribute-3.c b/gcc/testsuite/gcc.target/aarch64/simd_pcs_attribute-3.c
index e69de29bb2d..e3debb0ab18 100644
--- a/gcc/testsuite/gcc.target/aarch64/simd_pcs_attribute-3.c
+++ b/gcc/testsuite/gcc.target/aarch64/simd_pcs_attribute-3.c
@@ -0,0 +1,25 @@
+/* { dg-do compile } */
+/* { dg-options "-Ofast" } */
+/* { dg-require-effective-target aarch64_variant_pcs } */
+
+__attribute__ ((__simd__))
+__attribute__ ((__nothrow__ , __leaf__ , __const__))
+double foo (double x);
+
+void bar(double *f, int n)
+{
+	int i;
+	for (i = 0; i < n; i++)
+		f[i] = foo(f[i]);
+}
+
+double foo(double x)
+{
+	return x * x / 3.0;
+}
+
+/* { dg-final { scan-assembler-not {\.variant_pcs\tfoo} } } */
+/* { dg-final { scan-assembler-times {\.variant_pcs\t_ZGVnM1v_foo} 1 } } */
+/* { dg-final { scan-assembler-times {\.variant_pcs\t_ZGVnM2v_foo} 1 } } */
+/* { dg-final { scan-assembler-times {\.variant_pcs\t_ZGVnN1v_foo} 1 } } */
+/* { dg-final { scan-assembler-times {\.variant_pcs\t_ZGVnN2v_foo} 1 } } */
diff --git a/gcc/testsuite/gcc.target/aarch64/simd_pcs_attribute.c b/gcc/testsuite/gcc.target/aarch64/simd_pcs_attribute.c
index e69de29bb2d..17a0a701cf4 100644
--- a/gcc/testsuite/gcc.target/aarch64/simd_pcs_attribute.c
+++ b/gcc/testsuite/gcc.target/aarch64/simd_pcs_attribute.c
@@ -0,0 +1,17 @@
+/* { dg-do compile } */
+/* { dg-options "-Ofast" } */
+/* { dg-require-effective-target aarch64_variant_pcs } */
+
+__attribute__ ((__simd__ ("notinbranch")))
+__attribute__ ((__nothrow__ , __leaf__ , __const__))
+extern double log (double __x);
+
+void foo(double *f, int n)
+{
+	int i;
+	for (i = 0; i < n; i++)
+		f[i] = log(f[i]);
+}
+
+/* { dg-final { scan-assembler-not {\.variant_pcs\tlog} } } */
+/* { dg-final { scan-assembler-times {\.variant_pcs\t_ZGVnN2v_log} 1 } } */

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH] Fix simd attribute handling on aarch64 (version 2)
  2019-07-19 15:57   ` [PATCH] Fix simd attribute handling on aarch64 (version 2) Steve Ellcey
@ 2019-07-19 18:26     ` Richard Sandiford
  2019-07-22 18:26       ` Steve Ellcey
  0 siblings, 1 reply; 12+ messages in thread
From: Richard Sandiford @ 2019-07-19 18:26 UTC (permalink / raw)
  To: Steve Ellcey; +Cc: gcc-patches, Richard.Earnshaw, Szabolcs.Nagy

Steve Ellcey <sellcey@marvell.com> writes:
> Here is version two of my patch to fix simd attribute handling on
> aarch64.  Unlike the first patch where I swapped the order of the
> calls to targetm.simd_clone.adjust and simd_clone_adjust_return_type,
> in this one I remove the (conditional) call to build_distinct_type_copy
> from simd_clone_adjust_return_type and do it unconditionally before
> calling either routine.  The only downside to this that I can see is
> that on non-aarch64 platforms where the return type of a vector
> function is VOID (and not changed), we will create a distinct type
> where we did not before.
>
> I also added some tests to ensure that, on aarch64, the vector
> functions created by cloning a simd function have the .variant_pcs
> directive and that the original non-vector version of the function
> does not have the directive.  Without this patch the non-vector
> version is putting out the directive, that is what this patch
> fixes.
>
> Retested on x86 and aarch64 with no regressions.
>
> OK to checkin?
>
> Steve Ellcey
> sellcey@marvell.com
>
>
> 2019-07-19  Steve Ellcey  <sellcey@marvell.com>
>
> 	* omp-simd-clone.c (simd_clone_adjust_return_type): Remove call to
> 	build_distinct_type_copy.
> 	(simd_clone_adjust): Call build_distinct_type_copy.
> 	(expand_simd_clones): Ditto.
>
>
> 2019-07-19  Steve Ellcey  <sellcey@marvell.com>
>
> 	* gcc.target/aarch64/simd_pcs_attribute.c: New test.
> 	* gcc.target/aarch64/simd_pcs_attribute-2.c: Ditto.
> 	* gcc.target/aarch64/simd_pcs_attribute-3.c: Ditto.
>
>
> 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);

You can probably also remove:

      tree new_type = build_distinct_type_copy (TREE_TYPE (node->decl));
      ...
      TREE_TYPE (node->decl) = new_type;

in simd_clone_adjust_argument_types.

I'm happy doing it this way or doing the copy in the AArch64 hook.
It's really Jakub's call.

I don't think the tests need:

/* { dg-require-effective-target aarch64_variant_pcs } */

since they're only dg-do compile.  Leaving the line out would get more
coverage for people using older binutils.

The tests are OK with that change, thanks.

Richard

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH] Fix simd attribute handling on aarch64 (version 2)
  2019-07-19 18:26     ` Richard Sandiford
@ 2019-07-22 18:26       ` Steve Ellcey
  2019-07-29 23:00         ` Steve Ellcey
  2019-07-30 13:35         ` Richard Sandiford
  0 siblings, 2 replies; 12+ messages in thread
From: Steve Ellcey @ 2019-07-22 18:26 UTC (permalink / raw)
  To: jakub, richard.sandiford; +Cc: gcc-patches, Richard.Earnshaw, Szabolcs.Nagy

[-- Attachment #1: Type: text/plain, Size: 1449 bytes --]

On Fri, 2019-07-19 at 19:24 +0100, Richard Sandiford wrote:
> 
> You can probably also remove:
> 
>       tree new_type = build_distinct_type_copy (TREE_TYPE (node->decl));
>       ...
>       TREE_TYPE (node->decl) = new_type;
> 
> in simd_clone_adjust_argument_types.
> 
> I'm happy doing it this way or doing the copy in the AArch64 hook.
> It's really Jakub's call.

You are right, that is no longer needed with the current patch.  I
removed it and retested with no regressions.  Jakub, do you have
any preference?  I have attached a new version of the patch to this
email.

> I don't think the tests need:
> 
> /* { dg-require-effective-target aarch64_variant_pcs } */
> 
> since they're only dg-do compile.  Leaving the line out would get more
> coverage for people using older binutils.
> 
> The tests are OK with that change, thanks.

OK, I made that change to the tests.


Latest version of the patch:

2019-07-22  Steve Ellcey  <sellcey@marvell.com>

	* omp-simd-clone.c (simd_clone_adjust_return_type): Remove call to
	build_distinct_type_copy.
	(simd_clone_adjust_argument_types): Ditto.
	(simd_clone_adjust): Call build_distinct_type_copy here.
	(expand_simd_clones): Ditto.


2019-07-22  Steve Ellcey  <sellcey@marvell.com>

	* gcc.target/aarch64/simd_pcs_attribute.c: New test.
	* gcc.target/aarch64/simd_pcs_attribute-2.c: Ditto.
	* gcc.target/aarch64/simd_pcs_attribute-3.c: Ditto.



[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: gcc-simd.patch --]
[-- Type: text/x-patch; name="gcc-simd.patch", Size: 1511 bytes --]

diff --git a/gcc/omp-simd-clone.c b/gcc/omp-simd-clone.c
index caa8da3cba5..da01d9b8e6c 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;
@@ -724,11 +723,6 @@ simd_clone_adjust_argument_types (struct cgraph_node *node)
 	  else
 	    new_reversed = void_list_node;
 	}
-
-      tree new_type = build_distinct_type_copy (TREE_TYPE (node->decl));
-      TYPE_ARG_TYPES (new_type) = new_reversed;
-      TREE_TYPE (node->decl) = new_type;
-
       adjustments.release ();
     }
   args.release ();
@@ -1164,6 +1158,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 +1732,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);

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #3: gcc-simd-test.patch --]
[-- Type: text/x-patch; name="gcc-simd-test.patch", Size: 2439 bytes --]

diff --git a/gcc/testsuite/gcc.target/aarch64/simd_pcs_attribute-2.c b/gcc/testsuite/gcc.target/aarch64/simd_pcs_attribute-2.c
index e69de29bb2d..8eec6824f37 100644
--- a/gcc/testsuite/gcc.target/aarch64/simd_pcs_attribute-2.c
+++ b/gcc/testsuite/gcc.target/aarch64/simd_pcs_attribute-2.c
@@ -0,0 +1,16 @@
+/* { dg-do compile } */
+/* { dg-options "-Ofast" } */
+
+__attribute__ ((__simd__ ("notinbranch")))
+__attribute__ ((__nothrow__ , __leaf__ , __const__))
+extern double foo (double x);
+
+void bar(double * f, int n)
+{
+	int i;
+	for (i = 0; i < n; i++)
+		f[i] = foo(f[i]);
+}
+
+/* { dg-final { scan-assembler-not {\.variant_pcs\tfoo} } } */
+/* { dg-final { scan-assembler-times {\.variant_pcs\t_ZGVnN2v_foo} 1 } } */
diff --git a/gcc/testsuite/gcc.target/aarch64/simd_pcs_attribute-3.c b/gcc/testsuite/gcc.target/aarch64/simd_pcs_attribute-3.c
index e69de29bb2d..95f6a6803e8 100644
--- a/gcc/testsuite/gcc.target/aarch64/simd_pcs_attribute-3.c
+++ b/gcc/testsuite/gcc.target/aarch64/simd_pcs_attribute-3.c
@@ -0,0 +1,24 @@
+/* { dg-do compile } */
+/* { dg-options "-Ofast" } */
+
+__attribute__ ((__simd__))
+__attribute__ ((__nothrow__ , __leaf__ , __const__))
+double foo (double x);
+
+void bar(double *f, int n)
+{
+	int i;
+	for (i = 0; i < n; i++)
+		f[i] = foo(f[i]);
+}
+
+double foo(double x)
+{
+	return x * x / 3.0;
+}
+
+/* { dg-final { scan-assembler-not {\.variant_pcs\tfoo} } } */
+/* { dg-final { scan-assembler-times {\.variant_pcs\t_ZGVnM1v_foo} 1 } } */
+/* { dg-final { scan-assembler-times {\.variant_pcs\t_ZGVnM2v_foo} 1 } } */
+/* { dg-final { scan-assembler-times {\.variant_pcs\t_ZGVnN1v_foo} 1 } } */
+/* { dg-final { scan-assembler-times {\.variant_pcs\t_ZGVnN2v_foo} 1 } } */
diff --git a/gcc/testsuite/gcc.target/aarch64/simd_pcs_attribute.c b/gcc/testsuite/gcc.target/aarch64/simd_pcs_attribute.c
index e69de29bb2d..eddcef3597c 100644
--- a/gcc/testsuite/gcc.target/aarch64/simd_pcs_attribute.c
+++ b/gcc/testsuite/gcc.target/aarch64/simd_pcs_attribute.c
@@ -0,0 +1,16 @@
+/* { dg-do compile } */
+/* { dg-options "-Ofast" } */
+
+__attribute__ ((__simd__ ("notinbranch")))
+__attribute__ ((__nothrow__ , __leaf__ , __const__))
+extern double log (double __x);
+
+void foo(double *f, int n)
+{
+	int i;
+	for (i = 0; i < n; i++)
+		f[i] = log(f[i]);
+}
+
+/* { dg-final { scan-assembler-not {\.variant_pcs\tlog} } } */
+/* { dg-final { scan-assembler-times {\.variant_pcs\t_ZGVnN2v_log} 1 } } */

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH] Fix simd attribute handling on aarch64 (version 2)
  2019-07-22 18:26       ` Steve Ellcey
@ 2019-07-29 23:00         ` Steve Ellcey
  2019-07-30 13:35         ` Richard Sandiford
  1 sibling, 0 replies; 12+ messages in thread
From: Steve Ellcey @ 2019-07-29 23:00 UTC (permalink / raw)
  To: jakub, richard.sandiford; +Cc: gcc-patches, Richard.Earnshaw, Szabolcs.Nagy

Ping.

Steve Ellcey
sellcey@marvell.com

On Mon, 2019-07-22 at 11:25 -0700, Steve Ellcey wrote:
> On Fri, 2019-07-19 at 19:24 +0100, Richard Sandiford wrote:
> > 
> > You can probably also remove:
> > 
> >       tree new_type = build_distinct_type_copy (TREE_TYPE (node-
> > >decl));
> >       ...
> >       TREE_TYPE (node->decl) = new_type;
> > 
> > in simd_clone_adjust_argument_types.
> > 
> > I'm happy doing it this way or doing the copy in the AArch64 hook.
> > It's really Jakub's call.
> 
> You are right, that is no longer needed with the current patch.  I
> removed it and retested with no regressions.  Jakub, do you have
> any preference?  I have attached a new version of the patch to this
> email.
> 
> > I don't think the tests need:
> > 
> > /* { dg-require-effective-target aarch64_variant_pcs } */
> > 
> > since they're only dg-do compile.  Leaving the line out would get
> > more
> > coverage for people using older binutils.
> > 
> > The tests are OK with that change, thanks.
> 
> OK, I made that change to the tests.
> 
> 
> Latest version of the patch:
> 
> 2019-07-22  Steve Ellcey  <sellcey@marvell.com>
> 
> 	* omp-simd-clone.c (simd_clone_adjust_return_type): Remove call
> to
> 	build_distinct_type_copy.
> 	(simd_clone_adjust_argument_types): Ditto.
> 	(simd_clone_adjust): Call build_distinct_type_copy here.
> 	(expand_simd_clones): Ditto.
> 
> 
> 2019-07-22  Steve Ellcey  <sellcey@marvell.com>
> 
> 	* gcc.target/aarch64/simd_pcs_attribute.c: New test.
> 	* gcc.target/aarch64/simd_pcs_attribute-2.c: Ditto.
> 	* gcc.target/aarch64/simd_pcs_attribute-3.c: Ditto.
> 
> 

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH] Fix simd attribute handling on aarch64 (version 2)
  2019-07-22 18:26       ` Steve Ellcey
  2019-07-29 23:00         ` Steve Ellcey
@ 2019-07-30 13:35         ` Richard Sandiford
  2019-07-30 22:14           ` Steve Ellcey
  1 sibling, 1 reply; 12+ messages in thread
From: Richard Sandiford @ 2019-07-30 13:35 UTC (permalink / raw)
  To: Steve Ellcey; +Cc: jakub, gcc-patches, Richard.Earnshaw, Szabolcs.Nagy

Steve Ellcey <sellcey@marvell.com> writes:
> On Fri, 2019-07-19 at 19:24 +0100, Richard Sandiford wrote:
>> 
>> You can probably also remove:
>> 
>>       tree new_type = build_distinct_type_copy (TREE_TYPE (node->decl));
>>       ...
>>       TREE_TYPE (node->decl) = new_type;
>> 
>> in simd_clone_adjust_argument_types.
>> 
>> I'm happy doing it this way or doing the copy in the AArch64 hook.
>> It's really Jakub's call.
>
> You are right, that is no longer needed with the current patch.  I
> removed it and retested with no regressions.  Jakub, do you have
> any preference?  I have attached a new version of the patch to this
> email.
>
>> I don't think the tests need:
>> 
>> /* { dg-require-effective-target aarch64_variant_pcs } */
>> 
>> since they're only dg-do compile.  Leaving the line out would get more
>> coverage for people using older binutils.
>> 
>> The tests are OK with that change, thanks.
>
> OK, I made that change to the tests.
>
>
> Latest version of the patch:
>
> 2019-07-22  Steve Ellcey  <sellcey@marvell.com>
>
> 	* omp-simd-clone.c (simd_clone_adjust_return_type): Remove call to
> 	build_distinct_type_copy.
> 	(simd_clone_adjust_argument_types): Ditto.
> 	(simd_clone_adjust): Call build_distinct_type_copy here.
> 	(expand_simd_clones): Ditto.
>
>
> 2019-07-22  Steve Ellcey  <sellcey@marvell.com>
>
> 	* gcc.target/aarch64/simd_pcs_attribute.c: New test.
> 	* gcc.target/aarch64/simd_pcs_attribute-2.c: Ditto.
> 	* gcc.target/aarch64/simd_pcs_attribute-3.c: Ditto.
>
>
> diff --git a/gcc/omp-simd-clone.c b/gcc/omp-simd-clone.c
> index caa8da3cba5..da01d9b8e6c 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;
> @@ -724,11 +723,6 @@ simd_clone_adjust_argument_types (struct cgraph_node *node)
>  	  else
>  	    new_reversed = void_list_node;
>  	}
> -
> -      tree new_type = build_distinct_type_copy (TREE_TYPE (node->decl));
> -      TYPE_ARG_TYPES (new_type) = new_reversed;

I think you still need this line, just applied to the existing type
rather than a new one.

> -      TREE_TYPE (node->decl) = new_type;
> -
>        adjustments.release ();
>      }
>    args.release ();
> @@ -1164,6 +1158,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 +1732,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);
>
> diff --git a/gcc/testsuite/gcc.target/aarch64/simd_pcs_attribute-2.c b/gcc/testsuite/gcc.target/aarch64/simd_pcs_attribute-2.c
> index e69de29bb2d..8eec6824f37 100644
> --- a/gcc/testsuite/gcc.target/aarch64/simd_pcs_attribute-2.c
> +++ b/gcc/testsuite/gcc.target/aarch64/simd_pcs_attribute-2.c
> @@ -0,0 +1,16 @@
> +/* { dg-do compile } */
> +/* { dg-options "-Ofast" } */
> +
> +__attribute__ ((__simd__ ("notinbranch")))
> +__attribute__ ((__nothrow__ , __leaf__ , __const__))
> +extern double foo (double x);
> +
> +void bar(double * f, int n)
> +{
> +	int i;
> +	for (i = 0; i < n; i++)
> +		f[i] = foo(f[i]);
> +}
> +
> +/* { dg-final { scan-assembler-not {\.variant_pcs\tfoo} } } */
> +/* { dg-final { scan-assembler-times {\.variant_pcs\t_ZGVnN2v_foo} 1 } } */
> diff --git a/gcc/testsuite/gcc.target/aarch64/simd_pcs_attribute-3.c b/gcc/testsuite/gcc.target/aarch64/simd_pcs_attribute-3.c
> index e69de29bb2d..95f6a6803e8 100644
> --- a/gcc/testsuite/gcc.target/aarch64/simd_pcs_attribute-3.c
> +++ b/gcc/testsuite/gcc.target/aarch64/simd_pcs_attribute-3.c
> @@ -0,0 +1,24 @@
> +/* { dg-do compile } */
> +/* { dg-options "-Ofast" } */
> +
> +__attribute__ ((__simd__))
> +__attribute__ ((__nothrow__ , __leaf__ , __const__))
> +double foo (double x);
> +
> +void bar(double *f, int n)
> +{
> +	int i;
> +	for (i = 0; i < n; i++)
> +		f[i] = foo(f[i]);
> +}
> +
> +double foo(double x)
> +{
> +	return x * x / 3.0;
> +}
> +
> +/* { dg-final { scan-assembler-not {\.variant_pcs\tfoo} } } */
> +/* { dg-final { scan-assembler-times {\.variant_pcs\t_ZGVnM1v_foo} 1 } } */
> +/* { dg-final { scan-assembler-times {\.variant_pcs\t_ZGVnM2v_foo} 1 } } */
> +/* { dg-final { scan-assembler-times {\.variant_pcs\t_ZGVnN1v_foo} 1 } } */
> +/* { dg-final { scan-assembler-times {\.variant_pcs\t_ZGVnN2v_foo} 1 } } */
> diff --git a/gcc/testsuite/gcc.target/aarch64/simd_pcs_attribute.c b/gcc/testsuite/gcc.target/aarch64/simd_pcs_attribute.c
> index e69de29bb2d..eddcef3597c 100644
> --- a/gcc/testsuite/gcc.target/aarch64/simd_pcs_attribute.c
> +++ b/gcc/testsuite/gcc.target/aarch64/simd_pcs_attribute.c
> @@ -0,0 +1,16 @@
> +/* { dg-do compile } */
> +/* { dg-options "-Ofast" } */
> +
> +__attribute__ ((__simd__ ("notinbranch")))
> +__attribute__ ((__nothrow__ , __leaf__ , __const__))
> +extern double log (double __x);
> +
> +void foo(double *f, int n)
> +{
> +	int i;
> +	for (i = 0; i < n; i++)
> +		f[i] = log(f[i]);
> +}
> +
> +/* { dg-final { scan-assembler-not {\.variant_pcs\tlog} } } */
> +/* { dg-final { scan-assembler-times {\.variant_pcs\t_ZGVnN2v_log} 1 } } */

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH] Fix simd attribute handling on aarch64 (version 2)
  2019-07-30 13:35         ` Richard Sandiford
@ 2019-07-30 22:14           ` Steve Ellcey
  2019-07-31  7:34             ` Richard Sandiford
  0 siblings, 1 reply; 12+ messages in thread
From: Steve Ellcey @ 2019-07-30 22:14 UTC (permalink / raw)
  To: richard.sandiford; +Cc: gcc-patches, jakub, Richard.Earnshaw, Szabolcs.Nagy

[-- Attachment #1: Type: text/plain, Size: 1012 bytes --]

On Tue, 2019-07-30 at 14:31 +0100, Richard Sandiford wrote:
> 
> > -
> > -      tree new_type = build_distinct_type_copy (TREE_TYPE (node-
> > >decl));
> > -      TYPE_ARG_TYPES (new_type) = new_reversed;
> 
> I think you still need this line, just applied to the existing type
> rather than a new one.
> 
> > -      TREE_TYPE (node->decl) = new_type;
> > -
> >        adjustments.release ();
> >      }

OK, I fixed that and retested with no regressions.

Steve Ellcey
sellcey@marvell.com


2019-07-30  Steve Ellcey  <sellcey@marvell.com>

	* omp-simd-clone.c (simd_clone_adjust_return_type): Remove call to
	build_distinct_type_copy.
	(simd_clone_adjust_argument_types): Ditto.
	(simd_clone_adjust): Call build_distinct_type_copy here.
	(expand_simd_clones): Ditto.

2019-07-30  Steve Ellcey  <sellcey@marvell.com>

	* gcc.target/aarch64/simd_pcs_attribute.c: New test.
	* gcc.target/aarch64/simd_pcs_attribute-2.c: Ditto.
	* gcc.target/aarch64/simd_pcs_attribute-3.c: Ditto.

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: gcc-simd.patch --]
[-- Type: text/x-patch; name="gcc-simd.patch", Size: 1574 bytes --]

diff --git a/gcc/omp-simd-clone.c b/gcc/omp-simd-clone.c
index caa8da3cba5..c8c528471a3 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;
@@ -724,11 +723,7 @@ simd_clone_adjust_argument_types (struct cgraph_node *node)
 	  else
 	    new_reversed = void_list_node;
 	}
-
-      tree new_type = build_distinct_type_copy (TREE_TYPE (node->decl));
-      TYPE_ARG_TYPES (new_type) = new_reversed;
-      TREE_TYPE (node->decl) = new_type;
-
+      TYPE_ARG_TYPES (TREE_TYPE (node->decl)) = new_reversed;
       adjustments.release ();
     }
   args.release ();
@@ -1164,6 +1159,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 +1733,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);

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #3: gcc-simd-test.patch --]
[-- Type: text/x-patch; name="gcc-simd-test.patch", Size: 2439 bytes --]

diff --git a/gcc/testsuite/gcc.target/aarch64/simd_pcs_attribute-2.c b/gcc/testsuite/gcc.target/aarch64/simd_pcs_attribute-2.c
index e69de29bb2d..8eec6824f37 100644
--- a/gcc/testsuite/gcc.target/aarch64/simd_pcs_attribute-2.c
+++ b/gcc/testsuite/gcc.target/aarch64/simd_pcs_attribute-2.c
@@ -0,0 +1,16 @@
+/* { dg-do compile } */
+/* { dg-options "-Ofast" } */
+
+__attribute__ ((__simd__ ("notinbranch")))
+__attribute__ ((__nothrow__ , __leaf__ , __const__))
+extern double foo (double x);
+
+void bar(double * f, int n)
+{
+	int i;
+	for (i = 0; i < n; i++)
+		f[i] = foo(f[i]);
+}
+
+/* { dg-final { scan-assembler-not {\.variant_pcs\tfoo} } } */
+/* { dg-final { scan-assembler-times {\.variant_pcs\t_ZGVnN2v_foo} 1 } } */
diff --git a/gcc/testsuite/gcc.target/aarch64/simd_pcs_attribute-3.c b/gcc/testsuite/gcc.target/aarch64/simd_pcs_attribute-3.c
index e69de29bb2d..95f6a6803e8 100644
--- a/gcc/testsuite/gcc.target/aarch64/simd_pcs_attribute-3.c
+++ b/gcc/testsuite/gcc.target/aarch64/simd_pcs_attribute-3.c
@@ -0,0 +1,24 @@
+/* { dg-do compile } */
+/* { dg-options "-Ofast" } */
+
+__attribute__ ((__simd__))
+__attribute__ ((__nothrow__ , __leaf__ , __const__))
+double foo (double x);
+
+void bar(double *f, int n)
+{
+	int i;
+	for (i = 0; i < n; i++)
+		f[i] = foo(f[i]);
+}
+
+double foo(double x)
+{
+	return x * x / 3.0;
+}
+
+/* { dg-final { scan-assembler-not {\.variant_pcs\tfoo} } } */
+/* { dg-final { scan-assembler-times {\.variant_pcs\t_ZGVnM1v_foo} 1 } } */
+/* { dg-final { scan-assembler-times {\.variant_pcs\t_ZGVnM2v_foo} 1 } } */
+/* { dg-final { scan-assembler-times {\.variant_pcs\t_ZGVnN1v_foo} 1 } } */
+/* { dg-final { scan-assembler-times {\.variant_pcs\t_ZGVnN2v_foo} 1 } } */
diff --git a/gcc/testsuite/gcc.target/aarch64/simd_pcs_attribute.c b/gcc/testsuite/gcc.target/aarch64/simd_pcs_attribute.c
index e69de29bb2d..eddcef3597c 100644
--- a/gcc/testsuite/gcc.target/aarch64/simd_pcs_attribute.c
+++ b/gcc/testsuite/gcc.target/aarch64/simd_pcs_attribute.c
@@ -0,0 +1,16 @@
+/* { dg-do compile } */
+/* { dg-options "-Ofast" } */
+
+__attribute__ ((__simd__ ("notinbranch")))
+__attribute__ ((__nothrow__ , __leaf__ , __const__))
+extern double log (double __x);
+
+void foo(double *f, int n)
+{
+	int i;
+	for (i = 0; i < n; i++)
+		f[i] = log(f[i]);
+}
+
+/* { dg-final { scan-assembler-not {\.variant_pcs\tlog} } } */
+/* { dg-final { scan-assembler-times {\.variant_pcs\t_ZGVnN2v_log} 1 } } */

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH] Fix simd attribute handling on aarch64 (version 2)
  2019-07-30 22:14           ` Steve Ellcey
@ 2019-07-31  7:34             ` Richard Sandiford
  2019-08-07 10:51               ` Szabolcs Nagy
  0 siblings, 1 reply; 12+ messages in thread
From: Richard Sandiford @ 2019-07-31  7:34 UTC (permalink / raw)
  To: Steve Ellcey; +Cc: gcc-patches, jakub, Richard.Earnshaw, Szabolcs.Nagy

Steve Ellcey <sellcey@marvell.com> writes:
> On Tue, 2019-07-30 at 14:31 +0100, Richard Sandiford wrote:
>> 
>> > -
>> > -      tree new_type = build_distinct_type_copy (TREE_TYPE (node-
>> > >decl));
>> > -      TYPE_ARG_TYPES (new_type) = new_reversed;
>> 
>> I think you still need this line, just applied to the existing type
>> rather than a new one.
>> 
>> > -      TREE_TYPE (node->decl) = new_type;
>> > -
>> >        adjustments.release ();
>> >      }
>
> OK, I fixed that and retested with no regressions.
>
> Steve Ellcey
> sellcey@marvell.com
>
>
> 2019-07-30  Steve Ellcey  <sellcey@marvell.com>
>
> 	* omp-simd-clone.c (simd_clone_adjust_return_type): Remove call to
> 	build_distinct_type_copy.
> 	(simd_clone_adjust_argument_types): Ditto.
> 	(simd_clone_adjust): Call build_distinct_type_copy here.
> 	(expand_simd_clones): Ditto.
>
> 2019-07-30  Steve Ellcey  <sellcey@marvell.com>
>
> 	* gcc.target/aarch64/simd_pcs_attribute.c: New test.
> 	* gcc.target/aarch64/simd_pcs_attribute-2.c: Ditto.
> 	* gcc.target/aarch64/simd_pcs_attribute-3.c: Ditto.

OK if there are no objections in 48 hours.

Thanks,
Richard

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH] Fix simd attribute handling on aarch64 (version 2)
  2019-07-31  7:34             ` Richard Sandiford
@ 2019-08-07 10:51               ` Szabolcs Nagy
  2019-08-07 18:16                 ` Steve Ellcey
  0 siblings, 1 reply; 12+ messages in thread
From: Szabolcs Nagy @ 2019-08-07 10:51 UTC (permalink / raw)
  To: Steve Ellcey, gcc-patches, jakub, Richard Earnshaw, Richard Sandiford; +Cc: nd

On 31/07/2019 08:25, Richard Sandiford wrote:
> Steve Ellcey <sellcey@marvell.com> writes:
>>
>> 2019-07-30  Steve Ellcey  <sellcey@marvell.com>
>>
>> 	* omp-simd-clone.c (simd_clone_adjust_return_type): Remove call to
>> 	build_distinct_type_copy.
>> 	(simd_clone_adjust_argument_types): Ditto.
>> 	(simd_clone_adjust): Call build_distinct_type_copy here.
>> 	(expand_simd_clones): Ditto.
>>
>> 2019-07-30  Steve Ellcey  <sellcey@marvell.com>
>>
>> 	* gcc.target/aarch64/simd_pcs_attribute.c: New test.
>> 	* gcc.target/aarch64/simd_pcs_attribute-2.c: Ditto.
>> 	* gcc.target/aarch64/simd_pcs_attribute-3.c: Ditto.
> 
> OK if there are no objections in 48 hours.

i think this should be backported to gcc-9 too.

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH] Fix simd attribute handling on aarch64 (version 2)
  2019-08-07 10:51               ` Szabolcs Nagy
@ 2019-08-07 18:16                 ` Steve Ellcey
  0 siblings, 0 replies; 12+ messages in thread
From: Steve Ellcey @ 2019-08-07 18:16 UTC (permalink / raw)
  To: gcc-patches, Richard.Sandiford, jakub, Richard.Earnshaw, Szabolcs.Nagy; +Cc: nd

On Wed, 2019-08-07 at 10:40 +0000, Szabolcs Nagy wrote:
> -------------------------------------------
> ---
> On 31/07/2019 08:25, Richard Sandiford wrote:
> > Steve Ellcey <sellcey@marvell.com> writes:
> > > 
> > > 2019-07-30  Steve Ellcey  <sellcey@marvell.com>
> > > 
> > > 	* omp-simd-clone.c (simd_clone_adjust_return_type): Remove call
> > > to
> > > 	build_distinct_type_copy.
> > > 	(simd_clone_adjust_argument_types): Ditto.
> > > 	(simd_clone_adjust): Call build_distinct_type_copy here.
> > > 	(expand_simd_clones): Ditto.
> > > 
> > > 2019-07-30  Steve Ellcey  <sellcey@marvell.com>
> > > 
> > > 	* gcc.target/aarch64/simd_pcs_attribute.c: New test.
> > > 	* gcc.target/aarch64/simd_pcs_attribute-2.c: Ditto.
> > > 	* gcc.target/aarch64/simd_pcs_attribute-3.c: Ditto.
> > 
> > OK if there are no objections in 48 hours.
> 
> i think this should be backported to gcc-9 too.

Yes, I agree.  The 9.X branch is frozen right now for the 9.2 release,
I will backport it to the branch after it reopens assuming there are no
objections.

Steve Ellcey
sellcey@marvell.com

^ permalink raw reply	[flat|nested] 12+ messages in thread

end of thread, other threads:[~2019-08-07 17:20 UTC | newest]

Thread overview: 12+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-07-17 21:29 [PATCH] Fix simd attribute handling on aarch64 Steve Ellcey
2019-07-18  9:22 ` Richard Sandiford
2019-07-18 16:04   ` Steve Ellcey
2019-07-19 15:57   ` [PATCH] Fix simd attribute handling on aarch64 (version 2) Steve Ellcey
2019-07-19 18:26     ` Richard Sandiford
2019-07-22 18:26       ` Steve Ellcey
2019-07-29 23:00         ` Steve Ellcey
2019-07-30 13:35         ` Richard Sandiford
2019-07-30 22:14           ` Steve Ellcey
2019-07-31  7:34             ` Richard Sandiford
2019-08-07 10:51               ` Szabolcs Nagy
2019-08-07 18:16                 ` Steve Ellcey

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).