diff mbox

[VECTOR,ABI] Add __attribute__((__simd__)) to GCC.

Message ID 20151202124642.GA39390@msticlxl57.ims.intel.com
State New
Headers show

Commit Message

Kirill Yukhin Dec. 2, 2015, 12:46 p.m. UTC
Hello Jakub,

On 13 Nov 13:16, Jakub Jelinek wrote:
> > --- /dev/null
> > +++ b/gcc/testsuite/c-c++-common/attr-simd.c
> 
> Similarly.
> 
> Ok for trunk with those changes.
It turns out that current implementation of GLibC does not
contain masked variants of math routines. So, this attribute
is useless until it is capable to generation only [nonmasked|maked]
variants of the routines.

Patch in the bottom introduces `notinbranch' and `inbranch' flags to
the attribute.

Bootstrapped and regtested. New tests pass.

Is it ok for trunk (GCC v6)?

gcc/
	* c-family/c-common.c (c_common_attribute_table[]): Update max aerguments
	count for "simd" attribute.
	(handle_simd_attribute): Parse "notinbranch" and "inbranch" arguments.
	* doc/extend.texi ("simd"): Describe new flags.
gcc/testsuite/
	* c-c++-common/attr-simd-4.c: New test.
	* c-c++-common/attr-simd-5.c: New test.

--
Thanks, K

> 
> 	Jakub

commit cf458a0a00214022556498bdda94a07d0af70574
Author: Kirill Yukhin <kirill.yukhin@intel.com>
Date:   Mon Nov 30 16:24:39 2015 +0300

    [attr-simd] Add notinbranch/inbranch flags.

Comments

Jeff Law Dec. 2, 2015, 5:40 p.m. UTC | #1
On 12/02/2015 05:46 AM, Kirill Yukhin wrote:
> Hello Jakub,
>
> On 13 Nov 13:16, Jakub Jelinek wrote:
>>> --- /dev/null
>>> +++ b/gcc/testsuite/c-c++-common/attr-simd.c
>>
>> Similarly.
>>
>> Ok for trunk with those changes.
> It turns out that current implementation of GLibC does not
> contain masked variants of math routines. So, this attribute
> is useless until it is capable to generation only [nonmasked|maked]
> variants of the routines.
>
> Patch in the bottom introduces `notinbranch' and `inbranch' flags to
> the attribute.
>
> Bootstrapped and regtested. New tests pass.
>
> Is it ok for trunk (GCC v6)?
>
> gcc/
> 	* c-family/c-common.c (c_common_attribute_table[]): Update max aerguments
> 	count for "simd" attribute.
> 	(handle_simd_attribute): Parse "notinbranch" and "inbranch" arguments.
> 	* doc/extend.texi ("simd"): Describe new flags.
> gcc/testsuite/
> 	* c-c++-common/attr-simd-4.c: New test.
> 	* c-c++-common/attr-simd-5.c: New test.
Why not use "unmasked" and "masked" instead of "notinbranch" and 
"inbranch"?   If those terms come from OpenMP or are in use by other 
compilers (llvm, icc, whatever), then I guess we should stick with them. 
  Otherwise we should consider [un]masked which are consistent with the 
vector abi document.

So I think if [not]inbranch comes from OpenMP, then this patch is OK 
as-is.   Similarly if other compilers are using the [not]inbranch 
modifier.  If the modifiers are totally arbitrary, then consider using 
[un]masked which is consistent with the vector abi documentation and the 
patch would be pre-approved with that change.


Jeff
Jakub Jelinek Dec. 2, 2015, 5:42 p.m. UTC | #2
On Wed, Dec 02, 2015 at 10:40:13AM -0700, Jeff Law wrote:
> Why not use "unmasked" and "masked" instead of "notinbranch" and "inbranch"?
> If those terms come from OpenMP or are in use by other compilers (llvm, icc,
> whatever), then I guess we should stick with them.  Otherwise we should
> consider [un]masked which are consistent with the vector abi document.

notinbranch/inbranch are OpenMP clauses used for this purpose.

	Jakub
diff mbox

Patch

diff --git a/gcc/c-family/c-common.c b/gcc/c-family/c-common.c
index 369574f..0104306 100644
--- a/gcc/c-family/c-common.c
+++ b/gcc/c-family/c-common.c
@@ -818,7 +818,7 @@  const struct attribute_spec c_common_attribute_table[] =
 			      handle_omp_declare_simd_attribute, false },
   { "cilk simd function",     0, -1, true,  false, false,
 			      handle_omp_declare_simd_attribute, false },
-  { "simd",		      0, 0, true,  false, false,
+  { "simd",		      0, 1, true,  false, false,
 			      handle_simd_attribute, false },
   { "omp declare target",     0, 0, true, false, false,
 			      handle_omp_declare_target_attribute, false },
@@ -9032,7 +9032,7 @@  handle_omp_declare_simd_attribute (tree *, tree, tree, int, bool *)
 /* Handle a "simd" attribute.  */
 
 static tree
-handle_simd_attribute (tree *node, tree name, tree, int, bool *no_add_attrs)
+handle_simd_attribute (tree *node, tree name, tree args, int, bool *no_add_attrs)
 {
   if (TREE_CODE (*node) == FUNCTION_DECL)
     {
@@ -9045,9 +9045,41 @@  handle_simd_attribute (tree *node, tree name, tree, int, bool *no_add_attrs)
 	  *no_add_attrs = true;
 	}
       else
-	DECL_ATTRIBUTES (*node)
-	  = tree_cons (get_identifier ("omp declare simd"),
-		       NULL_TREE, DECL_ATTRIBUTES (*node));
+	{
+	  tree t = get_identifier ("omp declare simd");
+	  tree attr = NULL_TREE;
+	  if (args)
+	    {
+	      tree id = TREE_VALUE (args);
+
+	      if (TREE_CODE (id) != STRING_CST)
+		{
+		  error ("attribute %qE argument not a string", name);
+		  *no_add_attrs = true;
+		  return NULL_TREE;
+		}
+
+	      if (strcmp (TREE_STRING_POINTER (id), "notinbranch") == 0)
+		attr = build_omp_clause (DECL_SOURCE_LOCATION (*node),
+					 OMP_CLAUSE_NOTINBRANCH);
+	      else
+		if (strcmp (TREE_STRING_POINTER (id), "inbranch") == 0)
+		  attr = build_omp_clause (DECL_SOURCE_LOCATION (*node),
+					   OMP_CLAUSE_INBRANCH);
+		else
+		{
+		  error ("only %<inbranch%> and %<notinbranch%> flags are "
+			 "allowed for %<__simd__%> attribute");
+		  *no_add_attrs = true;
+		  return NULL_TREE;
+		}
+	    }
+
+	  DECL_ATTRIBUTES (*node) = tree_cons (t,
+					       build_tree_list (NULL_TREE,
+								attr),
+					       DECL_ATTRIBUTES (*node));
+	}
     }
   else
     {
diff --git a/gcc/doc/extend.texi b/gcc/doc/extend.texi
index 63fce0f..c517038 100644
--- a/gcc/doc/extend.texi
+++ b/gcc/doc/extend.texi
@@ -3144,6 +3144,7 @@  At the function call it will create resolver @code{ifunc}, that will
 dynamically call a clone suitable for current architecture.
 
 @item simd
+@itemx simd("@var{mask}")
 @cindex @code{simd} function attribute.
 This attribute enables creation of one or more function versions that
 can process multiple arguments using SIMD instructions from a
@@ -3158,6 +3159,9 @@  attribute on the same function.
 If the attribute is specified and @code{#pragma omp declare simd}
 present on a declaration and @code{-fopenmp} or @code{-fopenmp-simd}
 switch is specified, then the attribute is ignored.
+The optional argument @var{mask} may have "notinbranch" or "inbranch"
+value and instructs the compiler to generate non-masked or masked
+clones correspondingly. By default, all clones are generated.
 
 @item target (@var{options})
 @cindex @code{target} function attribute
diff --git a/gcc/testsuite/c-c++-common/attr-simd-4.c b/gcc/testsuite/c-c++-common/attr-simd-4.c
new file mode 100644
index 0000000..66cd8f1
--- /dev/null
+++ b/gcc/testsuite/c-c++-common/attr-simd-4.c
@@ -0,0 +1,38 @@ 
+/* { dg-do compile } */
+/* { dg-options "-fdump-tree-optimized" } */
+
+extern
+#ifdef __cplusplus
+"C"
+#endif
+__attribute__((__simd__("notinbranch")))
+int simd_attr (void)
+{
+  return 0;
+}
+
+/* { dg-final { scan-tree-dump "simd_attr\[ \\t\]simdclone|vector" "optimized" { target { i?86-*-* x86_64-*-* } } } } */
+/* { dg-final { scan-assembler-times "_ZGVbN4_simd_attr:" 1 { target { i?86-*-* x86_64-*-* } } } } */
+/* { dg-final { scan-assembler-times "_ZGVcN4_simd_attr:" 1 { target { i?86-*-* x86_64-*-* } } } } */
+/* { dg-final { scan-assembler-times "_ZGVdN8_simd_attr:" 1 { target { i?86-*-* x86_64-*-* } } } } */
+/* { dg-final { scan-assembler-not "_ZGVbM4_simd_attr:" { target { i?86-*-* x86_64-*-* } } } } */
+/* { dg-final { scan-assembler-not "_ZGVcM4_simd_attr:" { target { i?86-*-* x86_64-*-* } } } } */
+/* { dg-final { scan-assembler-not "_ZGVdM8_simd_attr:" { target { i?86-*-* x86_64-*-* } } } } */
+
+extern
+#ifdef __cplusplus
+"C"
+#endif
+__attribute__((simd("inbranch")))
+int simd_attr2 (void)
+{
+  return 0;
+}
+
+/* { dg-final { scan-tree-dump "simd_attr2\[ \\t\]simdclone|vector" "optimized" { target { i?86-*-* x86_64-*-* } } } } */
+/* { dg-final { scan-assembler-not "_ZGVbN4_simd_attr2:" { target { i?86-*-* x86_64-*-* } } } } */
+/* { dg-final { scan-assembler-not "_ZGVcN4_simd_attr2:" { target { i?86-*-* x86_64-*-* } } } } */
+/* { dg-final { scan-assembler-not "_ZGVdN8_simd_attr2:" { target { i?86-*-* x86_64-*-* } } } } */
+/* { dg-final { scan-assembler-times "_ZGVbM4_simd_attr2:" 1 { target { i?86-*-* x86_64-*-* } } } } */
+/* { dg-final { scan-assembler-times "_ZGVcM4_simd_attr2:" 1 { target { i?86-*-* x86_64-*-* } } } } */
+/* { dg-final { scan-assembler-times "_ZGVdM8_simd_attr2:" 1 { target { i?86-*-* x86_64-*-* } } } } */
diff --git a/gcc/testsuite/c-c++-common/attr-simd-5.c b/gcc/testsuite/c-c++-common/attr-simd-5.c
new file mode 100644
index 0000000..7bf3f2c
--- /dev/null
+++ b/gcc/testsuite/c-c++-common/attr-simd-5.c
@@ -0,0 +1,7 @@ 
+/* { dg-do compile } */
+
+__attribute__((__simd__("bug")))
+int simd_attr (void) { return 0; } /* { dg-error "only 'inbranch' and 'notinbranch'" } */
+
+__attribute__((__simd__("notinbranch", "inbranch")))
+int simd_attr2 (void) { return 0; } /* { dg-error "wrong number of arguments specified" } */