[RFC PATCH v0] PPC64: Implement POWER Architecure Vector Function ABI.

[RFC PATCH v0] PPC64: Implement POWER Architecure Vector Function ABI.

[GT]

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

Function rs6000_simd_clone_adjust, even though it's body is empty,
cannot simply be removed. I tried it. It resulted in ICE. In my
view, leaving it empty is preferable to modifying other files
unrelated to rs6000.c in order to avoid having a function whose
body is empty.

Bert.

[-- Attachment #2: 0001-PPC64-Implement-POWER-Architecure-Vector-Function-AB.patch --]
[-- Type: application/octet-stream, Size: 6306 bytes --]

From 1e8feec5e90ff1a879849714c8d2ea143e77e154 Mon Sep 17 00:00:00 2001
From: Bert Tenjy <bert.tenjy@gmail.com>
Date: Fri, 14 Feb 2020 13:31:53 -0600
Subject: [RFC PATCH v0] PPC64: Implement POWER Architecure Vector Function
 ABI.

The Vector Function ABI document is tentatively located at:
<https://github.com/power8-abi-doc/vector-function-abi>

Bill Schmidt of IBM Linux Tech. Center has committed to eventually
integrating this ABI into the official POWER Architecture specifications.
He is a GCC and Toolchain Architect so this should ease concerns over
how much to trust an ABI which is not an official release.

The implementation is very similar to those of x86_64 SSE and Aarch64.

The major test of this patch autovectorizes math functions and so requires
libmvec. PPC64 libmvec functionality is only available on GLIBC branch
tuliom/libmvec. Until that branch is merged to master, testing this ABI
will mean checking out GLIBC branch tuliom/libmvec, building and installing
to a non-system directory. Likewise, GCC will have to be built then installed
so that it doesn't interfere with the system's GCC.

Compiling with newly-built GCC against newly-built GLIBC requires these
options to GCC:
-L "${glibc_install_dir}/lib"
-I "${glibc_install_dir}/include"
-Wl,--rpath="${glibc_install_dir}/lib"
-Wl,--dynamic-linker="${glibc_install_dir}/lib/ld64.so.2"
---
 gcc/config/rs6000/rs6000.c | 152 +++++++++++++++++++++++++++++++++++++
 1 file changed, 152 insertions(+)

diff --git a/gcc/config/rs6000/rs6000.c b/gcc/config/rs6000/rs6000.c
index fc36bb6714b..3329c96e6cc 100644
--- a/gcc/config/rs6000/rs6000.c
+++ b/gcc/config/rs6000/rs6000.c
@@ -1266,6 +1266,147 @@ static const struct attribute_spec rs6000_attribute_table[] =
 #endif
   { NULL,        0, 0, false, false, false, false, NULL, NULL }
 };
+
+/* Implement TARGET_SIMD_CLONE_COMPUTE_VECSIZE_AND_SIMDLEN.  */
+
+static int
+rs6000_simd_clone_compute_vecsize_and_simdlen (struct cgraph_node *node,
+                                        struct cgraph_simd_clone *clonei,
+                                        tree base_type, int num)
+{
+  int ret = 1;
+
+  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;
+    }
+
+  tree ret_type = TREE_TYPE (TREE_TYPE (node->decl));
+  if (TREE_CODE (ret_type) != VOID_TYPE)
+    switch (TYPE_MODE (ret_type))
+      {
+      case E_QImode:
+      case E_HImode:
+      case E_SImode:
+      case E_DImode:
+      case E_SFmode:
+      case E_DFmode:
+      /* case E_SCmode: */
+      /* case E_DCmode: */
+	if (!AGGREGATE_TYPE_P (ret_type))
+	  break;
+	/* FALLTHRU */
+      default:
+	warning_at (DECL_SOURCE_LOCATION (node->decl), 0,
+		    "unsupported return type %qT for simd", ret_type);
+	return 0;
+      }
+
+  tree t;
+  int i;
+  tree type_arg_types = TYPE_ARG_TYPES (TREE_TYPE (node->decl));
+  bool decl_arg_p = (node->definition || type_arg_types == NULL_TREE);
+
+  for (t = (decl_arg_p ? DECL_ARGUMENTS (node->decl) : type_arg_types), i = 0;
+       t && t != void_list_node; t = TREE_CHAIN (t), i++)
+    {
+      tree arg_type = decl_arg_p ? TREE_TYPE (t) : TREE_VALUE (t);
+      switch (TYPE_MODE (arg_type))
+	{
+	case E_QImode:
+	case E_HImode:
+	case E_SImode:
+	case E_DImode:
+	case E_SFmode:
+	case E_DFmode:
+	/* case E_SCmode: */
+	/* case E_DCmode: */
+	  if (!AGGREGATE_TYPE_P (arg_type))
+	    break;
+	  /* FALLTHRU */
+	default:
+	  if (clonei->args[i].arg_type == SIMD_CLONE_ARG_TYPE_UNIFORM)
+	    break;
+	  warning_at (DECL_SOURCE_LOCATION (node->decl), 0,
+		      "unsupported argument type %qT for simd", arg_type);
+	  return 0;
+	}
+    }
+
+  if (TARGET_VSX)
+    {
+      clonei->vecsize_mangle = 'b';
+      ret = 1;
+    }
+  clonei->mask_mode = VOIDmode;
+  switch (clonei->vecsize_mangle)
+    {
+    case 'b':
+      clonei->vecsize_int = 128;
+      clonei->vecsize_float = 128;
+      break;
+    default:
+      gcc_unreachable ();
+    }
+  if (clonei->simdlen == 0)
+    {
+      if (SCALAR_INT_MODE_P (TYPE_MODE (base_type)))
+	clonei->simdlen = clonei->vecsize_int;
+      else
+	clonei->simdlen = clonei->vecsize_float;
+      clonei->simdlen /= GET_MODE_BITSIZE (TYPE_MODE (base_type));
+    }
+  else
+    {
+      tree ctype = ret_type;
+      if (TREE_CODE (ret_type) == VOID_TYPE)
+	ctype = base_type;
+      int cnt = GET_MODE_BITSIZE (TYPE_MODE (ctype)) * clonei->simdlen;
+      if (SCALAR_INT_MODE_P (TYPE_MODE (ctype)))
+	cnt /= clonei->vecsize_int;
+      else
+	cnt /= clonei->vecsize_float;
+      if (cnt > 12)
+	{
+	  warning_at (DECL_SOURCE_LOCATION (node->decl), 0,
+		      "unsupported simdlen %d", clonei->simdlen);
+	  return 0;
+	}
+      }
+  return ret;
+}
+
+/* Add target attribute to SIMD clone NODE if needed.  */
+
+void
+rs6000_simd_clone_adjust (struct cgraph_node *node)
+{
+}
+
+/* If SIMD clone NODE can't be used in a vectorized loop
+   in current function, return -1, otherwise return a badness of using it
+   (0 if it is most desirable from vecsize_mangle point of view, 1
+   slightly less desirable, etc.).  */
+
+static int
+rs6000_simd_clone_usable (struct cgraph_node *node)
+{
+  switch (node->simdclone->vecsize_mangle)
+    {
+    case 'b':
+      if (!TARGET_VSX)
+        return -1;
+      return 0;
+    default:
+      gcc_unreachable ();
+    }
+}
+
 \f
 #ifndef TARGET_PROFILE_KERNEL
 #define TARGET_PROFILE_KERNEL 0
@@ -1274,6 +1415,17 @@ static const struct attribute_spec rs6000_attribute_table[] =
 /* Initialize the GCC target structure.  */
 #undef TARGET_ATTRIBUTE_TABLE
 #define TARGET_ATTRIBUTE_TABLE rs6000_attribute_table
+
+#undef TARGET_SIMD_CLONE_COMPUTE_VECSIZE_AND_SIMDLEN
+#define TARGET_SIMD_CLONE_COMPUTE_VECSIZE_AND_SIMDLEN \
+  rs6000_simd_clone_compute_vecsize_and_simdlen
+
+#undef TARGET_SIMD_CLONE_ADJUST
+#define TARGET_SIMD_CLONE_ADJUST rs6000_simd_clone_adjust
+
+#undef TARGET_SIMD_CLONE_USABLE
+#define TARGET_SIMD_CLONE_USABLE rs6000_simd_clone_usable
+
 #undef TARGET_SET_DEFAULT_TYPE_ATTRIBUTES
 #define TARGET_SET_DEFAULT_TYPE_ATTRIBUTES rs6000_set_default_type_attributes
 #undef TARGET_ATTRIBUTE_TAKES_IDENTIFIER_P
-- 
2.20.1

Re: [RFC PATCH v0] PPC64: Implement POWER Architecure Vector Function ABI.

[Jakub Jelinek]

On Fri, Feb 14, 2020 at 08:24:30PM +0000, GT wrote:
> Function rs6000_simd_clone_adjust, even though it's body is empty,
> cannot simply be removed. I tried it. It resulted in ICE. In my
> view, leaving it empty is preferable to modifying other files
> unrelated to rs6000.c in order to avoid having a function whose
> body is empty.

So shouldn't the callback set target attribute (on definitions) to "vsx"?

	Jakub

Re: [RFC PATCH v0] PPC64: Implement POWER Architecure Vector Function ABI.

[GT]

‐‐‐‐‐‐‐ Original Message ‐‐‐‐‐‐‐
On Friday, February 14, 2020 3:38 PM, Jakub Jelinek <jakub@redhat.com> wrote:

> On Fri, Feb 14, 2020 at 08:24:30PM +0000, GT wrote:
>
> > Function rs6000_simd_clone_adjust, even though it's body is empty,
> > cannot simply be removed. I tried it. It resulted in ICE. In my
> > view, leaving it empty is preferable to modifying other files
> > unrelated to rs6000.c in order to avoid having a function whose
> > body is empty.
>
> So shouldn't the callback set target attribute (on definitions) to "vsx"?
>

I did consider doing something similar to aarch64_simd_clone_adjust. But the reason
Aarch64 has a new attribute aarch64_vector_pcs is that they implemented a modified
function calling sequence for vector functions. PPC64 vector functions use the existing
function calling sequence spelled out in the 64-bit ELFv2 ABI. So with no new attribute
here, the function body ends up empty.

Have I missed something crucial?

Bert.

Re: [RFC PATCH v0] PPC64: Implement POWER Architecure Vector Function ABI.

[Jakub Jelinek]

On Fri, Feb 14, 2020 at 10:02:39PM +0000, GT wrote:
> > > Function rs6000_simd_clone_adjust, even though it's body is empty,
> > > cannot simply be removed. I tried it. It resulted in ICE. In my
> > > view, leaving it empty is preferable to modifying other files
> > > unrelated to rs6000.c in order to avoid having a function whose
> > > body is empty.
> >
> > So shouldn't the callback set target attribute (on definitions) to "vsx"?
> >
> 
> I did consider doing something similar to aarch64_simd_clone_adjust. But the reason
> Aarch64 has a new attribute aarch64_vector_pcs is that they implemented a modified
> function calling sequence for vector functions. PPC64 vector functions use the existing
> function calling sequence spelled out in the 64-bit ELFv2 ABI. So with no new attribute
> here, the function body ends up empty.
> 
> Have I missed something crucial?

I haven't seen anything in the patch that would only enable it for ELFv2,
and while powerpc64le-linux probably assumes TARGET_VSX unconditionally
(haven't verified), powerpc64-linux or powerpc-linux certainly doesn't.
And it is just fine to have the ABI for those pass/return vectors in VSX
registers too, after all, it won't be used if the vectorized caller isn't
TARGET_VSX, the definitions of the declare simd functions could be compiled
with different ISA options.  And, if the ABI sais that the 'b' stuff assumes
certain ISA extensions, if the declare simd function definition is compiled
with e.g. -mno-vsx -mno-altivec, it would either not be able to get the
arguments/return values at all, or wouldn't benefit from the ISA guarantees
the ABI gives to it.

BTW, in the ABI document there isn't just 'b', but also 'c' ABI, it is
unclear if one needs to always emit both (e.g. like on x86 we emit 'b', 'c',
'd' and 'e') and then let the vectorized callers choose based on what ISA
options it is compiled with.

	Jakub

Re: [RFC PATCH v0] PPC64: Implement POWER Architecure Vector Function ABI.

[GT]

‐‐‐‐‐‐‐ Original Message ‐‐‐‐‐‐‐
On Friday, February 14, 2020 5:09 PM, Jakub Jelinek jakub@redhat.com wrote:

> On Fri, Feb 14, 2020 at 10:02:39PM +0000, GT wrote:
>
> > > > Function rs6000_simd_clone_adjust, even though it's body is empty,
> > > > cannot simply be removed. I tried it. It resulted in ICE. In my
> > > > view, leaving it empty is preferable to modifying other files
> > > > unrelated to rs6000.c in order to avoid having a function whose
> > > > body is empty.
> > >
> > > So shouldn't the callback set target attribute (on definitions) to "vsx"?
> >
> > I did consider doing something similar to aarch64_simd_clone_adjust. But the reason
> > Aarch64 has a new attribute aarch64_vector_pcs is that they implemented a modified
> > function calling sequence for vector functions. PPC64 vector functions use the existing
> > function calling sequence spelled out in the 64-bit ELFv2 ABI. So with no new attribute
> > here, the function body ends up empty.
> > Have I missed something crucial?
>
> I haven't seen anything in the patch that would only enable it for ELFv2,
>

The idea is that the vector functionality defined in the ABI is guaranteed only
on systems that implement the ELFv2 ABI. It's possible that the functionality also
works on ELFv1 Big-Endian PPC64. I'll check if that's the case. If so, then the ABI
will need modification.

> and while powerpc64le-linux probably assumes TARGET_VSX unconditionally
> (haven't verified), powerpc64-linux or powerpc-linux certainly doesn't.
>

The last function in the patch, rs6000_simd_clone_usable, returns a value that will
disable use of vector variants if TARGET_VSX is undefined.

> And it is just fine to have the ABI for those pass/return vectors in VSX
> registers too, after all, it won't be used if the vectorized caller isn't
> TARGET_VSX,

Don't quite understand the comment here. Are you stating the possibility of
a system that has VSX hardware but does not define macro TARGET_VSX?

> the definitions of the declare simd functions could be compiled
> with different ISA options.

Do you mean the 'b' vs 'c' in the ABI's vector function name mangling?

> And, if the ABI sais that the 'b' stuff assumes
> certain ISA extensions, if the declare simd function definition is compiled
> with e.g. -mno-vsx -mno-altivec, it would either not be able to get the
> arguments/return values at all, or wouldn't benefit from the ISA guarantees
> the ABI gives to it.
>

Not sure if you expect a response here.

> BTW, in the ABI document there isn't just 'b', but also 'c' ABI, it is
> unclear if one needs to always emit both (e.g. like on x86 we emit 'b', 'c',
> 'd' and 'e') and then let the vectorized callers choose based on what ISA
> options it is compiled with.
>

The reason 'c' was added to the ABI is this mailing list discussion:

https://sourceware.org/ml/libc-alpha/2019-11/msg00765.html

As long as 'b' specifies that the VSX functionality is that specified in ISA v2.07,
I suggest that we delete the reference to 'c' in the ABI. Bill, Tulio?

Bert.

Re: [RFC PATCH v0] PPC64: Implement POWER Architecure Vector Function ABI.

[GT]

‐‐‐‐‐‐‐ Original Message ‐‐‐‐‐‐‐
On Sunday, February 16, 2020 3:10 PM, GT <tnggil@protonmail.com> wrote:

> ‐‐‐‐‐‐‐ Original Message ‐‐‐‐‐‐‐
> On Friday, February 14, 2020 5:09 PM, Jakub Jelinek jakub@redhat.com wrote:
>
> > On Fri, Feb 14, 2020 at 10:02:39PM +0000, GT wrote:
> >
> > > > > Function rs6000_simd_clone_adjust, even though it's body is empty,
> > > > > cannot simply be removed. I tried it. It resulted in ICE. In my
> > > > > view, leaving it empty is preferable to modifying other files
> > > > > unrelated to rs6000.c in order to avoid having a function whose
> > > > > body is empty.
> > > >
> > > > So shouldn't the callback set target attribute (on definitions) to "vsx"?
> > >
> > > I did consider doing something similar to aarch64_simd_clone_adjust. But the reason
> > > Aarch64 has a new attribute aarch64_vector_pcs is that they implemented a modified
> > > function calling sequence for vector functions. PPC64 vector functions use the existing
> > > function calling sequence spelled out in the 64-bit ELFv2 ABI. So with no new attribute
> > > here, the function body ends up empty.
> > > Have I missed something crucial?
> >
> > I haven't seen anything in the patch that would only enable it for ELFv2,
>
> The idea is that the vector functionality defined in the ABI is guaranteed only
> on systems that implement the ELFv2 ABI. It's possible that the functionality also
> works on ELFv1 Big-Endian PPC64. I'll check if that's the case. If so, then the ABI
> will need modification.
>
> > and while powerpc64le-linux probably assumes TARGET_VSX unconditionally
> > (haven't verified), powerpc64-linux or powerpc-linux certainly doesn't.
>
> The last function in the patch, rs6000_simd_clone_usable, returns a value that will
> disable use of vector variants if TARGET_VSX is undefined.
>
> > And it is just fine to have the ABI for those pass/return vectors in VSX
> > registers too, after all, it won't be used if the vectorized caller isn't
> > TARGET_VSX,
>
> Don't quite understand the comment here. Are you stating the possibility of
> a system that has VSX hardware but does not define macro TARGET_VSX?
>
> > the definitions of the declare simd functions could be compiled
> > with different ISA options.
>
> Do you mean the 'b' vs 'c' in the ABI's vector function name mangling?
>
> > And, if the ABI sais that the 'b' stuff assumes
> > certain ISA extensions, if the declare simd function definition is compiled
> > with e.g. -mno-vsx -mno-altivec, it would either not be able to get the
> > arguments/return values at all, or wouldn't benefit from the ISA guarantees
> > the ABI gives to it.
>
> Not sure if you expect a response here.
>
> > BTW, in the ABI document there isn't just 'b', but also 'c' ABI, it is
> > unclear if one needs to always emit both (e.g. like on x86 we emit 'b', 'c',
> > 'd' and 'e') and then let the vectorized callers choose based on what ISA
> > options it is compiled with.
>
> The reason 'c' was added to the ABI is this mailing list discussion:
>
> https://sourceware.org/ml/libc-alpha/2019-11/msg00765.html
>
> As long as 'b' specifies that the VSX functionality is that specified in ISA v2.07,
> I suggest that we delete the reference to 'c' in the ABI. Bill, Tulio?
>

Jakub: Are there any specific changes you would like made to the patch?

Bert.

Re: [RFC PATCH v0] PPC64: Implement POWER Architecure Vector Function ABI.

[Bill Schmidt]

Sorry I missed this discussion until now, I have been out of the office 
much of the last week.

On 2/16/20 2:10 PM, GT wrote:
> ‐‐‐‐‐‐‐ Original Message ‐‐‐‐‐‐‐
> On Friday, February 14, 2020 5:09 PM, Jakub Jelinek jakub@redhat.com wrote:
>
>> On Fri, Feb 14, 2020 at 10:02:39PM +0000, GT wrote:
>>
>>>>> Function rs6000_simd_clone_adjust, even though it's body is empty,
>>>>> cannot simply be removed. I tried it. It resulted in ICE. In my
>>>>> view, leaving it empty is preferable to modifying other files
>>>>> unrelated to rs6000.c in order to avoid having a function whose
>>>>> body is empty.
>>>> So shouldn't the callback set target attribute (on definitions) to "vsx"?
>>> I did consider doing something similar to aarch64_simd_clone_adjust. But the reason
>>> Aarch64 has a new attribute aarch64_vector_pcs is that they implemented a modified
>>> function calling sequence for vector functions. PPC64 vector functions use the existing
>>> function calling sequence spelled out in the 64-bit ELFv2 ABI. So with no new attribute
>>> here, the function body ends up empty.
>>> Have I missed something crucial?
>> I haven't seen anything in the patch that would only enable it for ELFv2,
>>
> The idea is that the vector functionality defined in the ABI is guaranteed only
> on systems that implement the ELFv2 ABI. It's possible that the functionality also
> works on ELFv1 Big-Endian PPC64. I'll check if that's the case. If so, then the ABI
> will need modification.
>
>> and while powerpc64le-linux probably assumes TARGET_VSX unconditionally
>> (haven't verified), powerpc64-linux or powerpc-linux certainly doesn't.
>>
> The last function in the patch, rs6000_simd_clone_usable, returns a value that will
> disable use of vector variants if TARGET_VSX is undefined.
>
>> And it is just fine to have the ABI for those pass/return vectors in VSX
>> registers too, after all, it won't be used if the vectorized caller isn't
>> TARGET_VSX,
> Don't quite understand the comment here. Are you stating the possibility of
> a system that has VSX hardware but does not define macro TARGET_VSX?
>
>> the definitions of the declare simd functions could be compiled
>> with different ISA options.
> Do you mean the 'b' vs 'c' in the ABI's vector function name mangling?
>
>> And, if the ABI sais that the 'b' stuff assumes
>> certain ISA extensions, if the declare simd function definition is compiled
>> with e.g. -mno-vsx -mno-altivec, it would either not be able to get the
>> arguments/return values at all, or wouldn't benefit from the ISA guarantees
>> the ABI gives to it.
>>
> Not sure if you expect a response here.
>
>> BTW, in the ABI document there isn't just 'b', but also 'c' ABI, it is
>> unclear if one needs to always emit both (e.g. like on x86 we emit 'b', 'c',
>> 'd' and 'e') and then let the vectorized callers choose based on what ISA
>> options it is compiled with.
>>
> The reason 'c' was added to the ABI is this mailing list discussion:
>
> https://sourceware.org/ml/libc-alpha/2019-11/msg00765.html
>
> As long as 'b' specifies that the VSX functionality is that specified in ISA v2.07,
> I suggest that we delete the reference to 'c' in the ABI. Bill, Tulio?


No, I don't think that's the right call.  We want to leverage ISA 3.0
instructionsin vector implementations when they are available, so we
need the 'c' ABI for that purpose.  In future we are likely to add a
'd' ABI for a future processor if it adds more vector capability.  So
emitting both and letting the vectorized callers choose, as Jakub
suggests, seems like the right way to go.  This is true even if the
current implementations are identical (i.e., don't exploit any ISA
3.0 instructions).

Again, sorry for the tardy response!

Bill

>
> Bert.

Re: [RFC PATCH v0] PPC64: Implement POWER Architecure Vector Function ABI.

[GT]

‐‐‐‐‐‐‐ Original Message ‐‐‐‐‐‐‐
On Wednesday, February 19, 2020 12:33 PM, Bill Schmidt <wschmidt@linux.ibm.com> wrote:

> >
> > The reason 'c' was added to the ABI is this mailing list discussion:
> > https://sourceware.org/ml/libc-alpha/2019-11/msg00765.html
> > As long as 'b' specifies that the VSX functionality is that specified in ISA v2.07,
> > I suggest that we delete the reference to 'c' in the ABI. Bill, Tulio?
>
> No, I don't think that's the right call.  We want to leverage ISA 3.0
> instructionsin vector implementations when they are available, so we
> need the 'c' ABI for that purpose.  In future we are likely to add a
> 'd' ABI for a future processor if it adds more vector capability.  So
> emitting both and letting the vectorized callers choose, as Jakub
> suggests, seems like the right way to go.  This is true even if the
> current implementations are identical (i.e., don't exploit any ISA
> 3.0 instructions).
>

Here are proposed modifications:

1. In the Vector Function ABI document, under section "Vector Function Name Mangling",
state that all <isa> vector variants will be created by the compiler. And that it will
be up to the caller of vectorized functions to select the preferred version ('b' or 'c'
are the only choices presently).

2. Change rs6000_simd_clone_usable so that it more closely resembles ix86_simd_clone_usable.
The switch statement in that function will add a clause for 'c' to the existing one for 'b'.
I'm not sure what to test for in the 'c' clause. In x86_64, they have TARGET_SSE2, TARGET_AVX,
TARGET_AVX2 and TARGET_AVX512. PPC64 has only TARGET_VSX as best I can determine. There are
macros PPC_FEATURE2_ARCH_2_07 and PPC_FEATURE2_ARCH_3_00 in ppc-auxv.h. Can we use these 2
macros where x86_64 uses the macros with prefix TARGET_ ?

Bert.

Re: [RFC PATCH v0] PPC64: Implement POWER Architecure Vector Function ABI.

[Bill Schmidt]

On 2/19/20 1:10 PM, GT wrote:
> ‐‐‐‐‐‐‐ Original Message ‐‐‐‐‐‐‐
> On Wednesday, February 19, 2020 12:33 PM, Bill Schmidt <wschmidt@linux.ibm.com> wrote:
>
>>> The reason 'c' was added to the ABI is this mailing list discussion:
>>> https://sourceware.org/ml/libc-alpha/2019-11/msg00765.html
>>> As long as 'b' specifies that the VSX functionality is that specified in ISA v2.07,
>>> I suggest that we delete the reference to 'c' in the ABI. Bill, Tulio?
>> No, I don't think that's the right call.  We want to leverage ISA 3.0
>> instructionsin vector implementations when they are available, so we
>> need the 'c' ABI for that purpose.  In future we are likely to add a
>> 'd' ABI for a future processor if it adds more vector capability.  So
>> emitting both and letting the vectorized callers choose, as Jakub
>> suggests, seems like the right way to go.  This is true even if the
>> current implementations are identical (i.e., don't exploit any ISA
>> 3.0 instructions).
>>
> Here are proposed modifications:
>
> 1. In the Vector Function ABI document, under section "Vector Function Name Mangling",
> state that all <isa> vector variants will be created by the compiler. And that it will
> be up to the caller of vectorized functions to select the preferred version ('b' or 'c'
> are the only choices presently).
>
> 2. Change rs6000_simd_clone_usable so that it more closely resembles ix86_simd_clone_usable.
> The switch statement in that function will add a clause for 'c' to the existing one for 'b'.
> I'm not sure what to test for in the 'c' clause. In x86_64, they have TARGET_SSE2, TARGET_AVX,
> TARGET_AVX2 and TARGET_AVX512. PPC64 has only TARGET_VSX as best I can determine. There are
> macros PPC_FEATURE2_ARCH_2_07 and PPC_FEATURE2_ARCH_3_00 in ppc-auxv.h. Can we use these 2
> macros where x86_64 uses the macros with prefix TARGET_ ?


You can use TARGET_P9_VECTOR for this.

Thanks,
Bill

>
> Bert.

Re: [RFC PATCH v0] PPC64: Implement POWER Architecure Vector Function ABI.

[Joseph Myers]

On Wed, 19 Feb 2020, GT wrote:

> 1. In the Vector Function ABI document, under section "Vector Function 
> Name Mangling", state that all <isa> vector variants will be created by 
> the compiler. And that it will be up to the caller of vectorized 
> functions to select the preferred version ('b' or 'c' are the only 
> choices presently).

A reminder, "all" needs to be a particular fixed set that will not be 
expanded in future versions of the ABI, so that a glibc installed now 
continues to work with future compilers (rather than a future compiler 
wrongly taking a pragma / attribute in existing glibc headers as meaning 
some future vector ISA variant, unknown when that glibc header was 
written, is available).  Any future vector ISA variants will need some new 
pragma or attribute, naming the variant explicitly, to declare to the 
compiler that the library also provides the function for the future ISA 
variant.  (See <https://gcc.gnu.org/ml/gcc-patches/2019-11/msg01288.html> 
for a proposal for extending the simd attribute to allow explicitly 
declaring a particular variant to be available.)

-- 
Joseph S. Myers
joseph@codesourcery.com

Re: [RFC PATCH v0] PPC64: Implement POWER Architecure Vector Function ABI.

[GT]

‐‐‐‐‐‐‐ Original Message ‐‐‐‐‐‐‐
On Wednesday, February 19, 2020 5:52 PM, Joseph Myers <joseph@codesourcery.com> wrote:

> On Wed, 19 Feb 2020, GT wrote:
>
> > 1.  In the Vector Function ABI document, under section "Vector Function
> >     Name Mangling", state that all <isa> vector variants will be created by
> >     the compiler. And that it will be up to the caller of vectorized
> >     functions to select the preferred version ('b' or 'c' are the only
> >     choices presently).
> >
>
> A reminder, "all" needs to be a particular fixed set that will not be
> expanded in future versions of the ABI, so that a glibc installed now
> continues to work with future compilers (rather than a future compiler
> wrongly taking a pragma / attribute in existing glibc headers as meaning
> some future vector ISA variant, unknown when that glibc header was
> written, is available).

What more needs to be done other than documenting the GLIBC and GCC versions for
which 'b' and 'c' vector versions are available? It is how x86_64 explained
the differences between Examples 1 and 2 at https://sourceware.org/glibc/wiki/libmvec

Bert.

Re: [RFC PATCH v0] PPC64: Implement POWER Architecure Vector Function ABI.

[Joseph Myers]

On Thu, 20 Feb 2020, GT wrote:

> What more needs to be done other than documenting the GLIBC and GCC 
> versions for which 'b' and 'c' vector versions are available? It is how 
> x86_64 explained the differences between Examples 1 and 2 at 
> https://sourceware.org/glibc/wiki/libmvec

This is about ISA variants in the ABI.  
<https://sourceware.org/glibc/wiki/libmvec?action=AttachFile&do=view&target=VectorABI.txt> 
says "Compiler implementations must not generate calls to version of other 
ISAs unless some non-standard pragma or clause is used to declare those 
other versions are available.".  The same needs to apply to the POWER ABI 
- any pragma or attribute declaring vector functions to be available must 
have a fixed set of ISA variants the ABI says it declares to be available, 
with any ISA variants added in future needing a new pragma or attribute to 
declare them to be available, to avoid GCC 20 misinterpreting glibc 2.32 
headers as implying function variants are available for ISAs that didn't 
exist when those glibc 2.32 headers were written.

-- 
Joseph S. Myers
joseph@codesourcery.com

Re: [RFC PATCH v0] PPC64: Implement POWER Architecure Vector Function ABI.

[GT]

‐‐‐‐‐‐‐ Original Message ‐‐‐‐‐‐‐
On Wednesday, February 19, 2020 12:33 PM, Bill Schmidt <wschmidt@linux.ibm.com> wrote:

> > The reason 'c' was added to the ABI is this mailing list discussion:
> > https://sourceware.org/ml/libc-alpha/2019-11/msg00765.html
> > As long as 'b' specifies that the VSX functionality is that specified in ISA v2.07,
> > I suggest that we delete the reference to 'c' in the ABI. Bill, Tulio?
>
> No, I don't think that's the right call.  We want to leverage ISA 3.0
> instructionsin vector implementations when they are available, so we
> need the 'c' ABI for that purpose.  In future we are likely to add a
> 'd' ABI for a future processor if it adds more vector capability.  So
> emitting both and letting the vectorized callers choose, as Jakub
> suggests, seems like the right way to go.  This is true even if the
> current implementations are identical (i.e., don't exploit any ISA
> 3.0 instructions).
>

Because of the issue at https://gcc.gnu.org/ml/gcc-patches/2020-02/msg01171.html, I
am coming back to whether or not to include VSX extensions for ISA 3.0 in the Vector
Function ABI Specification.

If we retain 'c' in the ABI Spec., then GCC will expect libmvec functions such as
_ZGVcN2v_sin. The changes made to GLIBC for POWER libmvec don't have these functions
with <isa> == 'c'. Only those with <isa> == 'b' have been implemented. So we have to
do either of:

1. Create all those 'c' variants in GLIBC libmvec, even though they will be identical
to the existing 'b' versions.
2. Remove all references to 'c' in the ABI Specification, and leave GCC expecting to
find only 'b' variants in libmvec.

If/when it becomes necessary to have 'c' variants of functions, then a new version of
the Vector Function ABI document will be created. And GLIBC and GCC modifications to
comply with that new ABI will be made then.

Bert.

Re: [RFC PATCH v0] PPC64: Implement POWER Architecure Vector Function ABI.

[Bill Schmidt]

On 2/20/20 1:14 PM, GT wrote:
> ‐‐‐‐‐‐‐ Original Message ‐‐‐‐‐‐‐
> On Wednesday, February 19, 2020 12:33 PM, Bill Schmidt <wschmidt@linux.ibm.com> wrote:
>
>>> The reason 'c' was added to the ABI is this mailing list discussion:
>>> https://sourceware.org/ml/libc-alpha/2019-11/msg00765.html
>>> As long as 'b' specifies that the VSX functionality is that specified in ISA v2.07,
>>> I suggest that we delete the reference to 'c' in the ABI. Bill, Tulio?
>> No, I don't think that's the right call.  We want to leverage ISA 3.0
>> instructionsin vector implementations when they are available, so we
>> need the 'c' ABI for that purpose.  In future we are likely to add a
>> 'd' ABI for a future processor if it adds more vector capability.  So
>> emitting both and letting the vectorized callers choose, as Jakub
>> suggests, seems like the right way to go.  This is true even if the
>> current implementations are identical (i.e., don't exploit any ISA
>> 3.0 instructions).
>>
> Because of the issue at https://gcc.gnu.org/ml/gcc-patches/2020-02/msg01171.html, I
> am coming back to whether or not to include VSX extensions for ISA 3.0 in the Vector
> Function ABI Specification.
>
> If we retain 'c' in the ABI Spec., then GCC will expect libmvec functions such as
> _ZGVcN2v_sin. The changes made to GLIBC for POWER libmvec don't have these functions
> with <isa> == 'c'. Only those with <isa> == 'b' have been implemented. So we have to
> do either of:
>
> 1. Create all those 'c' variants in GLIBC libmvec, even though they will be identical
> to the existing 'b' versions.
> 2. Remove all references to 'c' in the ABI Specification, and leave GCC expecting to
> find only 'b' variants in libmvec.
>
> If/when it becomes necessary to have 'c' variants of functions, then a new version of
> the Vector Function ABI document will be created. And GLIBC and GCC modifications to
> comply with that new ABI will be made then.

Though I'm usually uncomfortable with kicking the can down the road on these
sorts of things, I can probably be convinced in this case.  Tulio and I were
wondering why the libmvec interface doesn't make use of ifunc capability for
this sort of thing.  Something to look into when more advanced implementations
are added later, I guess, and a valid reason to not lock ourselves into the
'c' ABI today.

Tulio, any concerns?

Bill

>
> Bert.

Re: [RFC PATCH v0] PPC64: Implement POWER Architecure Vector Function ABI.

[Jakub Jelinek]

On Sun, Feb 23, 2020 at 10:55:53AM -0600, Bill Schmidt wrote:
> > If/when it becomes necessary to have 'c' variants of functions, then a new version of
> > the Vector Function ABI document will be created. And GLIBC and GCC modifications to
> > comply with that new ABI will be made then.
> 
> Though I'm usually uncomfortable with kicking the can down the road on these
> sorts of things, I can probably be convinced in this case.  Tulio and I were
> wondering why the libmvec interface doesn't make use of ifunc capability for
> this sort of thing.  Something to look into when more advanced implementations
> are added later, I guess, and a valid reason to not lock ourselves into the
> 'c' ABI today.

Whether simd attribute or declare simd emits by default just 'b' or both 'b'
and 'c' or whatever other set is part of the ABI, so if/when that will
change, it will be an ABI break.
From this POV, handling faster (higher ISA), but same ABI, power9 optimized
code through ifuncs rather than a separate set of entrypoints might be
better.
It is true that it won't affect that way non-glibc code which won't use
ifunc manually like libmvec would, on the other side, OpenMP 5.0 has now
ways to handle such specialization through #pragma omp declare variant.

	Jakub

Re: [RFC PATCH v0] PPC64: Implement POWER Architecure Vector Function ABI.

[Tulio Magno Quites Machado Filho]

Jakub Jelinek <jakub@redhat.com> writes:

> On Sun, Feb 23, 2020 at 10:55:53AM -0600, Bill Schmidt wrote:
>> > If/when it becomes necessary to have 'c' variants of functions, then a new version of
>> > the Vector Function ABI document will be created. And GLIBC and GCC modifications to
>> > comply with that new ABI will be made then.
>> 
>> Though I'm usually uncomfortable with kicking the can down the road on these
>> sorts of things, I can probably be convinced in this case.  Tulio and I were
>> wondering why the libmvec interface doesn't make use of ifunc capability for
>> this sort of thing.  Something to look into when more advanced implementations
>> are added later, I guess, and a valid reason to not lock ourselves into the
>> 'c' ABI today.
>
> Tulio, any concerns?

No. I agree to remove 'c' from the ABI now.

Thanks!

-- 
Tulio Magno

Re: [RFC PATCH v0] PPC64: Implement POWER Architecure Vector Function ABI.

[Segher Boessenkool]

On Sun, Feb 23, 2020 at 10:55:53AM -0600, Bill Schmidt wrote:
> Though I'm usually uncomfortable with kicking the can down the road on these
> sorts of things, I can probably be convinced in this case.  Tulio and I were
> wondering why the libmvec interface doesn't make use of ifunc capability for
> this sort of thing.  Something to look into when more advanced 
> implementations
> are added later, I guess, and a valid reason to not lock ourselves into the
> 'c' ABI today.

I have some questions, too.  Why is this called an ABI at all?  Why will
it not work with *any* underlying ABI?  What *is* this, what is it *for*,
where is the documentation, where is the design documentation?  Etc.


Segher

Re: [RFC PATCH v0] PPC64: Implement POWER Architecure Vector Function ABI.

[Bill Schmidt]

On 2/23/20 1:12 PM, Segher Boessenkool wrote:
> On Sun, Feb 23, 2020 at 10:55:53AM -0600, Bill Schmidt wrote:
>> Though I'm usually uncomfortable with kicking the can down the road on these
>> sorts of things, I can probably be convinced in this case.  Tulio and I were
>> wondering why the libmvec interface doesn't make use of ifunc capability for
>> this sort of thing.  Something to look into when more advanced
>> implementations
>> are added later, I guess, and a valid reason to not lock ourselves into the
>> 'c' ABI today.
> I have some questions, too.  Why is this called an ABI at all?  Why will
> it not work with *any* underlying ABI?  What *is* this, what is it *for*,
> where is the documentation, where is the design documentation?  Etc.

So, I can answer a small amount of this, but I will say that overall, design
or implementation documentation seems to be between lacking and nonexistent.

This has to do with "#pragma omp simd" and providing the rules for vectorizing
functions into calls to libmvec.  I tend to agree that it is a bit more like
API than ABI, but we are just fitting into existing frameworks and using the
same nomenclature.

The ABI that Bert has produced for us is available at
https://sourceware.org/glibc/wiki/HomePage?action=AttachFile&do=view&target=powerarchvectfuncabi.html.

It is based on a similar document for x86.  I believe this is the most up-to-
date version:
https://software.intel.com/sites/default/files/managed/b4/c8/Intel-Vector-Function-ABI.pdf

There's been ongoing work in the same area for AArch64, with the most recent
version available here, I believe:
https://developer.arm.com/docs/101129/latest

I am not certain of the completion status of the AArch64 implementation.
The libmvec implementation for Intel has been around for a long time.

Hope that at least starts to help,
Bill

>
> Segher

Re: [RFC PATCH v0] PPC64: Implement POWER Architecure Vector Function ABI.

[GT]

‐‐‐‐‐‐‐ Original Message ‐‐‐‐‐‐‐
On Monday, February 24, 2020 10:20 AM, Bill Schmidt <wschmidt@linux.ibm.com> wrote:

> So, I can answer a small amount of this, but I will say that overall, design
> or implementation documentation seems to be between lacking and nonexistent.
>
> This has to do with "#pragma omp simd" and providing the rules for vectorizing
> functions into calls to libmvec.  I tend to agree that it is a bit more like
> API than ABI, but we are just fitting into existing frameworks and using the
> same nomenclature.
>
> The ABI that Bert has produced for us is available at
> https://sourceware.org/glibc/wiki/HomePage?action=AttachFile&do=view&target=powerarchvectfuncabi.html.
>
> It is based on a similar document for x86.  I believe this is the most up-to-
> date version:
> https://software.intel.com/sites/default/files/managed/b4/c8/Intel-Vector-Function-ABI.pdf
>
> There's been ongoing work in the same area for AArch64, with the most recent
> version available here, I believe:
> https://developer.arm.com/docs/101129/latest
>
> I am not certain of the completion status of the AArch64 implementation.
> The libmvec implementation for Intel has been around for a long time.
>
> Hope that at least starts to help,
> Bill

Additionally, here is GLIBC's introduction of libmvec: https://sourceware.org/glibc/wiki/libmvec

Bert.

Re: [RFC PATCH v0] PPC64: Implement POWER Architecure Vector Function ABI.

[Bill Schmidt]

On 2/14/20 4:09 PM, Jakub Jelinek wrote:
> On Fri, Feb 14, 2020 at 10:02:39PM +0000, GT wrote:
>>>> Function rs6000_simd_clone_adjust, even though it's body is empty,
>>>> cannot simply be removed. I tried it. It resulted in ICE. In my
>>>> view, leaving it empty is preferable to modifying other files
>>>> unrelated to rs6000.c in order to avoid having a function whose
>>>> body is empty.
>>> So shouldn't the callback set target attribute (on definitions) to "vsx"?
>>>
>> I did consider doing something similar to aarch64_simd_clone_adjust. But the reason
>> Aarch64 has a new attribute aarch64_vector_pcs is that they implemented a modified
>> function calling sequence for vector functions. PPC64 vector functions use the existing
>> function calling sequence spelled out in the 64-bit ELFv2 ABI. So with no new attribute
>> here, the function body ends up empty.
>>
>> Have I missed something crucial?
> I haven't seen anything in the patch that would only enable it for ELFv2,
> and while powerpc64le-linux probably assumes TARGET_VSX unconditionally
> (haven't verified), powerpc64-linux or powerpc-linux certainly doesn't.
> And it is just fine to have the ABI for those pass/return vectors in VSX
> registers too, after all, it won't be used if the vectorized caller isn't
> TARGET_VSX, the definitions of the declare simd functions could be compiled
> with different ISA options.  And, if the ABI sais that the 'b' stuff assumes
> certain ISA extensions, if the declare simd function definition is compiled
> with e.g. -mno-vsx -mno-altivec, it would either not be able to get the
> arguments/return values at all, or wouldn't benefit from the ISA guarantees
> the ABI gives to it.

It's a problem with the patch that it doesn't limit the ABI to ELFv2.  That is
necessary, because there are aspects of the vector ABI that are incompatible
with ELFv1.  In particular, ELFv1 doesn't support returning homogeneous
aggregates of vectors in vector registers, which is called for in the proposed
sincos interface, and would also be needed for vectorized complex functions.

Bill

>
> BTW, in the ABI document there isn't just 'b', but also 'c' ABI, it is
> unclear if one needs to always emit both (e.g. like on x86 we emit 'b', 'c',
> 'd' and 'e') and then let the vectorized callers choose based on what ISA
> options it is compiled with.
>
> 	Jakub
>

Re: [RFC PATCH v0] PPC64: Implement POWER Architecure Vector Function ABI.

[Jakub Jelinek]

On Sun, Feb 23, 2020 at 10:42:17AM -0600, Bill Schmidt wrote:
> > > Have I missed something crucial?
> > I haven't seen anything in the patch that would only enable it for ELFv2,
> > and while powerpc64le-linux probably assumes TARGET_VSX unconditionally
> > (haven't verified), powerpc64-linux or powerpc-linux certainly doesn't.
> > And it is just fine to have the ABI for those pass/return vectors in VSX
> > registers too, after all, it won't be used if the vectorized caller isn't
> > TARGET_VSX, the definitions of the declare simd functions could be compiled
> > with different ISA options.  And, if the ABI sais that the 'b' stuff assumes
> > certain ISA extensions, if the declare simd function definition is compiled
> > with e.g. -mno-vsx -mno-altivec, it would either not be able to get the
> > arguments/return values at all, or wouldn't benefit from the ISA guarantees
> > the ABI gives to it.
> 
> It's a problem with the patch that it doesn't limit the ABI to ELFv2.  That is
> necessary, because there are aspects of the vector ABI that are incompatible
> with ELFv1.  In particular, ELFv1 doesn't support returning homogeneous
> aggregates of vectors in vector registers, which is called for in the proposed
> sincos interface, and would also be needed for vectorized complex functions.

Is it really a problem?  I mean, it is perfectly fine if the declare simd
variants have a different ABI from the normal ABI, just for #pragma omp
declare variant it will be desirable if there is some attribute for such
different (or just slightly amended) ABI.  And vector complex is not an
issue right now, we punt on those on all architectures.

	Jakub

Re: [RFC PATCH v0] PPC64: Implement POWER Architecure Vector Function ABI.

[Bill Schmidt]

On 2/23/20 11:33 AM, Jakub Jelinek wrote:
> On Sun, Feb 23, 2020 at 10:42:17AM -0600, Bill Schmidt wrote:
>>>> Have I missed something crucial?
>>> I haven't seen anything in the patch that would only enable it for ELFv2,
>>> and while powerpc64le-linux probably assumes TARGET_VSX unconditionally
>>> (haven't verified), powerpc64-linux or powerpc-linux certainly doesn't.
>>> And it is just fine to have the ABI for those pass/return vectors in VSX
>>> registers too, after all, it won't be used if the vectorized caller isn't
>>> TARGET_VSX, the definitions of the declare simd functions could be compiled
>>> with different ISA options.  And, if the ABI sais that the 'b' stuff assumes
>>> certain ISA extensions, if the declare simd function definition is compiled
>>> with e.g. -mno-vsx -mno-altivec, it would either not be able to get the
>>> arguments/return values at all, or wouldn't benefit from the ISA guarantees
>>> the ABI gives to it.
>> It's a problem with the patch that it doesn't limit the ABI to ELFv2.  That is
>> necessary, because there are aspects of the vector ABI that are incompatible
>> with ELFv1.  In particular, ELFv1 doesn't support returning homogeneous
>> aggregates of vectors in vector registers, which is called for in the proposed
>> sincos interface, and would also be needed for vectorized complex functions.
> Is it really a problem?  I mean, it is perfectly fine if the declare simd
> variants have a different ABI from the normal ABI, just for #pragma omp
> declare variant it will be desirable if there is some attribute for such
> different (or just slightly amended) ABI.  And vector complex is not an
> issue right now, we punt on those on all architectures.

Well, it's a problem in the sense that the ABI as written does not work on
ELFv1, so until we have an ABI that does, we shouldn't allow it for anything
but ELFv2.  (Sections that don't apply to ELFv1 are Calling Convention, Vector
Length, Ordering of Vector Arguments.)

I know vector complex isn't currently an issue; just thinking ahead for
future potential improvements.

Bill

>
> 	Jakub
>

Re: [RFC PATCH v0] PPC64: Implement POWER Architecure Vector Function ABI.

[Segher Boessenkool]

On Fri, Feb 14, 2020 at 08:24:30PM +0000, GT wrote:
> Function rs6000_simd_clone_adjust, even though it's body is empty,
> cannot simply be removed. I tried it. It resulted in ICE. In my
> view, leaving it empty is preferable to modifying other files
> unrelated to rs6000.c in order to avoid having a function whose
> body is empty.

Please Cc: the rs6000 maintainers on rs6000 patches, you will get a
reply faster, and more reliably.

Please don't use binary attachments, it takes effort to reply to those.


Segher

Re: [RFC PATCH v0] PPC64: Implement POWER Architecure Vector Function ABI.

[GT]

‐‐‐‐‐‐‐ Original Message ‐‐‐‐‐‐‐
On Friday, February 14, 2020 6:46 PM, Segher Boessenkool <segher@kernel.crashing.org> wrote:

> On Fri, Feb 14, 2020 at 08:24:30PM +0000, GT wrote:
>
> > Function rs6000_simd_clone_adjust, even though it's body is empty,
> > cannot simply be removed. I tried it. It resulted in ICE. In my
> > view, leaving it empty is preferable to modifying other files
> > unrelated to rs6000.c in order to avoid having a function whose
> > body is empty.
>
> Please Cc: the rs6000 maintainers on rs6000 patches, you will get a
> reply faster, and more reliably.
>

File MAINTAINERS has you, David Edelsohn and Aldy Hernandez listed as maintainers
of various rs6000 aspects. Is that who you say I should "Cc:" or is there a
separate mailing list for rs6000?

> Please don't use binary attachments, it takes effort to reply to those.
>

I have not been able to configure protonmail for either git imap-send or send-email.
Will try pasting the .patch inline as plain text and see if that works.

Bert.

Re: [RFC PATCH v0] PPC64: Implement POWER Architecure Vector Function ABI.

[Segher Boessenkool]

On Sat, Feb 15, 2020 at 05:22:09PM +0000, GT wrote:
> > > Function rs6000_simd_clone_adjust, even though it's body is empty,
> > > cannot simply be removed. I tried it. It resulted in ICE. In my
> > > view, leaving it empty is preferable to modifying other files
> > > unrelated to rs6000.c in order to avoid having a function whose
> > > body is empty.
> >
> > Please Cc: the rs6000 maintainers on rs6000 patches, you will get a
> > reply faster, and more reliably.
> 
> File MAINTAINERS has you, David Edelsohn and Aldy Hernandez listed as maintainers
> of various rs6000 aspects. Is that who you say I should "Cc:" or is there a
> separate mailing list for rs6000?

I meant me and David.

> I have not been able to configure protonmail for either git imap-send or send-email.

Do you use git format-patch?  You should, as the manual pages for both
git imap-send and git send-email explain.

> Will try pasting the .patch inline as plain text and see if that works.

That doesn't work, unless you take extreme care.  It is fine for showing
small snippets, but it does not result in patches people can apply.


Segher

Re: [RFC PATCH v0] PPC64: Implement POWER Architecure Vector Function ABI.

[GT]

‐‐‐‐‐‐‐ Original Message ‐‐‐‐‐‐‐
On Sunday, February 16, 2020 7:06 PM, Segher Boessenkool <segher@kernel.crashing.org> wrote:

> On Sat, Feb 15, 2020 at 05:22:09PM +0000, GT wrote:

> > I have not been able to configure protonmail for either git imap-send or send-email.
>
> Do you use git format-patch? You should, as the manual pages for both
> git imap-send and git send-email explain.
>
> > Will try pasting the .patch inline as plain text and see if that works.
>
> That doesn't work, unless you take extreme care. It is fine for showing
> small snippets, but it does not result in patches people can apply.
>

I have more patches I'd like to post and I have been unable to find an alternative to
sending them as binary attachments.

1. The only system I have access to is a Windows 10 machine. I am not allowed to modify
it in any way. So I can't install cygwin (or any other software), if having that would
have solved my issues.

2. I am creating the patches on a VM in one of the clouds IBM makes available for POWER
software development. To access the VM the only option I have thus far is to login using
Openstack from a browser. Then open a console in the interface. To get patches out of the
VM, I push them onto Github. Then download them onto the Windows PC to send as email
attachments.

3. On the VM git imap-send and send-email are not usable for me at the moment. Gmail now requires
2-factor authentication for them. And that in turn requires a cell phone. I can't say when
I will have a new phone.

Anyone have ideas on how to avoid the binary attachements?

Bert.

Re: [RFC PATCH v0] PPC64: Implement POWER Architecure Vector Function ABI.

[Bill Schmidt]

On 2/14/20 2:24 PM, GT wrote:
> Function rs6000_simd_clone_adjust, even though it's body is empty,
> cannot simply be removed. I tried it. It resulted in ICE. In my
> view, leaving it empty is preferable to modifying other files
> unrelated to rs6000.c in order to avoid having a function whose
> body is empty.
>
> Bert.
> From 1e8feec5e90ff1a879849714c8d2ea143e77e154 Mon Sep 17 00:00:00 2001
> From: Bert Tenjy <bert.tenjy@gmail.com>
> Date: Fri, 14 Feb 2020 13:31:53 -0600
> Subject: [RFC PATCH v0] PPC64: Implement POWER Architecure Vector Function
>  ABI.
>
> The Vector Function ABI document is tentatively located at:
> <https://github.com/power8-abi-doc/vector-function-abi>
>
> Bill Schmidt of IBM Linux Tech. Center has committed to eventually
> integrating this ABI into the official POWER Architecture specifications.
> He is a GCC and Toolchain Architect so this should ease concerns over
> how much to trust an ABI which is not an official release.
>
> The implementation is very similar to those of x86_64 SSE and Aarch64.
>
> The major test of this patch autovectorizes math functions and so requires
> libmvec. PPC64 libmvec functionality is only available on GLIBC branch
> tuliom/libmvec. Until that branch is merged to master, testing this ABI
> will mean checking out GLIBC branch tuliom/libmvec, building and 
> installing
> to a non-system directory. Likewise, GCC will have to be built then 
> installed
> so that it doesn't interfere with the system's GCC.
>
> Compiling with newly-built GCC against newly-built GLIBC requires these
> options to GCC:
> -L "${glibc_install_dir}/lib"
> -I "${glibc_install_dir}/include"
> -Wl,--rpath="${glibc_install_dir}/lib"
> -Wl,--dynamic-linker="${glibc_install_dir}/lib/ld64.so.2"
> ---
>  gcc/config/rs6000/rs6000.c | 152 +++++++++++++++++++++++++++++++++++++
>  1 file changed, 152 insertions(+)
>
> diff --git a/gcc/config/rs6000/rs6000.c b/gcc/config/rs6000/rs6000.c
> index fc36bb6714b..3329c96e6cc 100644
> --- a/gcc/config/rs6000/rs6000.c
> +++ b/gcc/config/rs6000/rs6000.c
> @@ -1266,6 +1266,147 @@ static const struct attribute_spec 
> rs6000_attribute_table[] =
>  #endif
>    { NULL,        0, 0, false, false, false, false, NULL, NULL }
>  };
> +
> +/* Implement TARGET_SIMD_CLONE_COMPUTE_VECSIZE_AND_SIMDLEN.  */
> +
> +static int
> +rs6000_simd_clone_compute_vecsize_and_simdlen (struct cgraph_node *node,
> +                                        struct cgraph_simd_clone *clonei,
> +                                        tree base_type, int num)
> +{
> +  int ret = 1;
> +
> +  if (clonei->simdlen
> +      && (clonei->simdlen < 2
> +      || clonei->simdlen > 1024

Assuming that clonei->simdlen matches "vector length" in the ABI, 1024 is
too large a number.  We can have at most 8 vector registers containing
a homogeneous aggregate, each having up to 16 elements, so the correct
limit would be 128.

> +      || (clonei->simdlen & (clonei->simdlen - 1)) != 0))
> +    {
> +      warning_at (DECL_SOURCE_LOCATION (node->decl), 0,
> +          "unsupported simdlen %d", clonei->simdlen);
> +      return 0;
> +    }
> +
> +  tree ret_type = TREE_TYPE (TREE_TYPE (node->decl));
> +  if (TREE_CODE (ret_type) != VOID_TYPE)
> +    switch (TYPE_MODE (ret_type))
> +      {
> +      case E_QImode:
> +      case E_HImode:
> +      case E_SImode:
> +      case E_DImode:
> +      case E_SFmode:
> +      case E_DFmode:
> +      /* case E_SCmode: */
> +      /* case E_DCmode: */

Remove the two preceding lines.

> +    if (!AGGREGATE_TYPE_P (ret_type))
> +      break;
> +    /* FALLTHRU */
> +      default:
> +    warning_at (DECL_SOURCE_LOCATION (node->decl), 0,
> +            "unsupported return type %qT for simd", ret_type);
> +    return 0;
> +      }
> +
> +  tree t;
> +  int i;
> +  tree type_arg_types = TYPE_ARG_TYPES (TREE_TYPE (node->decl));
> +  bool decl_arg_p = (node->definition || type_arg_types == NULL_TREE);
> +
> +  for (t = (decl_arg_p ? DECL_ARGUMENTS (node->decl) : 
> type_arg_types), i = 0;
> +       t && t != void_list_node; t = TREE_CHAIN (t), i++)
> +    {
> +      tree arg_type = decl_arg_p ? TREE_TYPE (t) : TREE_VALUE (t);
> +      switch (TYPE_MODE (arg_type))
> +    {
> +    case E_QImode:
> +    case E_HImode:
> +    case E_SImode:
> +    case E_DImode:
> +    case E_SFmode:
> +    case E_DFmode:
> +    /* case E_SCmode: */
> +    /* case E_DCmode: */

Again, remove the two preceding lines.

> +      if (!AGGREGATE_TYPE_P (arg_type))
> +        break;
> +      /* FALLTHRU */
> +    default:
> +      if (clonei->args[i].arg_type == SIMD_CLONE_ARG_TYPE_UNIFORM)
> +        break;
> +      warning_at (DECL_SOURCE_LOCATION (node->decl), 0,
> +              "unsupported argument type %qT for simd", arg_type);
> +      return 0;
> +    }
> +    }
> +
> +  if (TARGET_VSX)
> +    {
> +      clonei->vecsize_mangle = 'b';
> +      ret = 1;
> +    }
> +  clonei->mask_mode = VOIDmode;
> +  switch (clonei->vecsize_mangle)
> +    {
> +    case 'b':
> +      clonei->vecsize_int = 128;
> +      clonei->vecsize_float = 128;
> +      break;
> +    default:
> +      gcc_unreachable ();
> +    }
> +  if (clonei->simdlen == 0)
> +    {
> +      if (SCALAR_INT_MODE_P (TYPE_MODE (base_type)))
> +    clonei->simdlen = clonei->vecsize_int;
> +      else
> +    clonei->simdlen = clonei->vecsize_float;
> +      clonei->simdlen /= GET_MODE_BITSIZE (TYPE_MODE (base_type));
> +    }
> +  else
> +    {
> +      tree ctype = ret_type;
> +      if (TREE_CODE (ret_type) == VOID_TYPE)
> +    ctype = base_type;
> +      int cnt = GET_MODE_BITSIZE (TYPE_MODE (ctype)) * clonei->simdlen;
> +      if (SCALAR_INT_MODE_P (TYPE_MODE (ctype)))
> +    cnt /= clonei->vecsize_int;
> +      else
> +    cnt /= clonei->vecsize_float;
> +      if (cnt > 12)

Only up to 8 vectors can be returned in registers as part of a homogeneous
aggregate.  Change to 8, please.

Otherwise LGTM, though I cannot approve the patch.

As discussed elsewhere, changing back to having just 'b' mangling in the
ABI is sensible, and this patch reflects that.  We will plan to use ifunc
for distinguishing between more performant ISA versions with the same
vector size.  We will only add a 'c' mangling at such time (if ever) that
a larger vector size is added to the PowerPC architecture.

Thanks,
Bill

> +    {
> +      warning_at (DECL_SOURCE_LOCATION (node->decl), 0,
> +              "unsupported simdlen %d", clonei->simdlen);
> +      return 0;
> +    }
> +      }
> +  return ret;
> +}
> +
> +/* Add target attribute to SIMD clone NODE if needed.  */
> +
> +void
> +rs6000_simd_clone_adjust (struct cgraph_node *node)
> +{
> +}
> +
> +/* If SIMD clone NODE can't be used in a vectorized loop
> +   in current function, return -1, otherwise return a badness of using it
> +   (0 if it is most desirable from vecsize_mangle point of view, 1
> +   slightly less desirable, etc.).  */
> +
> +static int
> +rs6000_simd_clone_usable (struct cgraph_node *node)
> +{
> +  switch (node->simdclone->vecsize_mangle)
> +    {
> +    case 'b':
> +      if (!TARGET_VSX)
> +        return -1;
> +      return 0;
> +    default:
> +      gcc_unreachable ();
> +    }
> +}
> +
>  \f
>  #ifndef TARGET_PROFILE_KERNEL
>  #define TARGET_PROFILE_KERNEL 0
> @@ -1274,6 +1415,17 @@ static const struct attribute_spec 
> rs6000_attribute_table[] =
>  /* Initialize the GCC target structure.  */
>  #undef TARGET_ATTRIBUTE_TABLE
>  #define TARGET_ATTRIBUTE_TABLE rs6000_attribute_table
> +
> +#undef TARGET_SIMD_CLONE_COMPUTE_VECSIZE_AND_SIMDLEN
> +#define TARGET_SIMD_CLONE_COMPUTE_VECSIZE_AND_SIMDLEN \
> +  rs6000_simd_clone_compute_vecsize_and_simdlen
> +
> +#undef TARGET_SIMD_CLONE_ADJUST
> +#define TARGET_SIMD_CLONE_ADJUST rs6000_simd_clone_adjust
> +
> +#undef TARGET_SIMD_CLONE_USABLE
> +#define TARGET_SIMD_CLONE_USABLE rs6000_simd_clone_usable
> +
>  #undef TARGET_SET_DEFAULT_TYPE_ATTRIBUTES
>  #define TARGET_SET_DEFAULT_TYPE_ATTRIBUTES 
> rs6000_set_default_type_attributes
>  #undef TARGET_ATTRIBUTE_TAKES_IDENTIFIER_P
> -- 
> 2.20.1
>

Re: [RFC PATCH v0] PPC64: Implement POWER Architecure Vector Function ABI.

[Jakub Jelinek]

On Mon, Feb 24, 2020 at 11:04:55AM -0600, Bill Schmidt wrote:
> > +  if (clonei->simdlen
> > +      && (clonei->simdlen < 2
> > +      || clonei->simdlen > 1024
> 
> Assuming that clonei->simdlen matches "vector length" in the ABI, 1024 is
> too large a number.  We can have at most 8 vector registers containing
> a homogeneous aggregate, each having up to 16 elements, so the correct
> limit would be 128.

Well, further arguments can be passed on the stack...

	Jakub

Re: [RFC PATCH v0] PPC64: Implement POWER Architecure Vector Function ABI.

[Bill Schmidt]

On 2/24/20 11:08 AM, Jakub Jelinek wrote:
> On Mon, Feb 24, 2020 at 11:04:55AM -0600, Bill Schmidt wrote:
>>> +  if (clonei->simdlen
>>> +      && (clonei->simdlen < 2
>>> +      || clonei->simdlen > 1024
>> Assuming that clonei->simdlen matches "vector length" in the ABI, 1024 is
>> too large a number.  We can have at most 8 vector registers containing
>> a homogeneous aggregate, each having up to 16 elements, so the correct
>> limit would be 128.
> Well, further arguments can be passed on the stack...

Well, ELFv2 doesn't define such a thing as a qualified homogeneous aggregate.
See rs6000_discover_homogeneous_aggregate and "Parameter Passing in
Registers" in ELFv2.  So the entire aggregate would be passed in memory,
not just the excess after 128 bytes.  I don't think this is necessarily
something we want to encourage in an interface intended to improve
performance.  Is there any reason we need to permit a larger value?  Do we
need to add this constraint to rs6000_simd_clone_usable?

Thanks,
Bill

>
> 	Jakub
>