public inbox for gcc-patches@gcc.gnu.org
 help / color / mirror / Atom feed
* [PATCH][RFC] Canonize names of attributes.
@ 2017-06-13 12:32 Martin Liška
  2017-06-13 13:20 ` Richard Biener
  2017-06-14 17:24 ` Jason Merrill
  0 siblings, 2 replies; 37+ messages in thread
From: Martin Liška @ 2017-06-13 12:32 UTC (permalink / raw)
  To: gcc-patches; +Cc: Richard Biener

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

Hello.

After some discussions with Richi, I would like to propose patch that will
come up with a canonical name of attribute names. That means __attribute__((__abi_tag__))
will be given 'abi_tag' as IDENTIFIER_NAME of the attribute. The change can improve
attribute name lookup and we can delete all the ugly code that compares strlen(i1)
== strlen(i2) + 4, etc.

Patch can bootstrap on ppc64le-redhat-linux and survives regression tests (w/ default
languages). I'm currently testing objc, obj-c++ and go.

Ready to be installed?
Martin


gcc/cp/ChangeLog:

2017-06-09  Martin Liska  <mliska@suse.cz>

	* parser.c (cp_parser_gnu_attribute_list): Canonize attribute
	names.
	(cp_parser_std_attribute): Likewise.

gcc/go/ChangeLog:

2017-06-09  Martin Liska  <mliska@suse.cz>

	* go-gcc.cc (Gcc_backend::function): Use no_split_stack
	instead of __no_split_stack__.

gcc/c/ChangeLog:

2017-06-09  Martin Liska  <mliska@suse.cz>

	* c-parser.c (c_parser_attributes): Canonize attribute names.

gcc/c-family/ChangeLog:

2017-06-09  Martin Liska  <mliska@suse.cz>

	* c-format.c (cmp_attribs): Simplify comparison of attributes.
	* c-lex.c (c_common_has_attribute): Canonize attribute names.

gcc/ChangeLog:

2017-06-09  Martin Liska  <mliska@suse.cz>

	* tree.c (cmp_attrib_identifiers): Simplify comparison of attributes.
	(private_is_attribute_p): Likewise.
	(private_lookup_attribute): Likewise.
	(private_lookup_attribute_by_prefix): Likewise.
	(remove_attribute): Likewise.
	(canonize_attr_name): New function.
	* tree.h: Declared here.

gcc/testsuite/ChangeLog:

2017-06-09  Martin Liska  <mliska@suse.cz>

	* g++.dg/cpp0x/pr65558.C: Change expected warning.
	* gcc.dg/parm-impl-decl-1.c: Likewise.
	* gcc.dg/parm-impl-decl-3.c: Likewise.
---
 gcc/c-family/c-format.c                 |  13 ++--
 gcc/c-family/c-lex.c                    |   1 +
 gcc/c/c-parser.c                        |   9 +++
 gcc/cp/parser.c                         |  11 +++-
 gcc/go/go-gcc.cc                        |   2 +-
 gcc/testsuite/g++.dg/cpp0x/pr65558.C    |   2 +-
 gcc/testsuite/gcc.dg/parm-impl-decl-1.c |   2 +-
 gcc/testsuite/gcc.dg/parm-impl-decl-3.c |   2 +-
 gcc/tree.c                              | 108 +++++++++++---------------------
 gcc/tree.h                              |   4 ++
 10 files changed, 69 insertions(+), 85 deletions(-)



[-- Attachment #2: 0001-Canonize-names-of-attributes.patch --]
[-- Type: text/x-patch, Size: 12371 bytes --]

diff --git a/gcc/c-family/c-format.c b/gcc/c-family/c-format.c
index 732339b9b5e..30f60d42cca 100644
--- a/gcc/c-family/c-format.c
+++ b/gcc/c-family/c-format.c
@@ -3982,15 +3982,10 @@ cmp_attribs (const char *tattr_name, const char *attr_name)
 {
   int alen = strlen (attr_name);
   int slen = (tattr_name ? strlen (tattr_name) : 0);
-  if (alen > 4 && attr_name[0] == '_' && attr_name[1] == '_'
-      && attr_name[alen - 1] == '_' && attr_name[alen - 2] == '_')
-    {
-      attr_name += 2;
-      alen -= 4;
-    }
-  if (alen != slen || strncmp (tattr_name, attr_name, alen) != 0)
-    return false;
-  return true;
+  gcc_checking_assert (alen == 0 || attr_name[0] != '_');
+  gcc_checking_assert (slen == 0 || tattr_name[0] != '_');
+
+  return (alen == slen && strncmp (tattr_name, attr_name, alen) == 0);
 }
 
 /* Handle a "format" attribute; arguments as in
diff --git a/gcc/c-family/c-lex.c b/gcc/c-family/c-lex.c
index e1c8bdff986..6d0e9279ed6 100644
--- a/gcc/c-family/c-lex.c
+++ b/gcc/c-family/c-lex.c
@@ -316,6 +316,7 @@ c_common_has_attribute (cpp_reader *pfile)
     {
       attr_name = get_identifier ((const char *)
 				  cpp_token_as_text (pfile, token));
+      attr_name = canonize_attr_name (attr_name);
       if (c_dialect_cxx ())
 	{
 	  int idx = 0;
diff --git a/gcc/c/c-parser.c b/gcc/c/c-parser.c
index 6f954f21fa2..400b65380e2 100644
--- a/gcc/c/c-parser.c
+++ b/gcc/c/c-parser.c
@@ -4168,9 +4168,11 @@ c_parser_attributes (c_parser *parser)
 	  attr_name = c_parser_attribute_any_word (parser);
 	  if (attr_name == NULL)
 	    break;
+	  attr_name = canonize_attr_name (attr_name);
 	  if (is_cilkplus_vector_p (attr_name))
 	    {
 	      c_token *v_token = c_parser_peek_token (parser);
+	      v_token->value = canonize_attr_name (v_token->value);
 	      c_parser_cilk_simd_fn_vector_attrs (parser, *v_token);
 	      /* If the next token isn't a comma, we're done.  */
 	      if (!c_parser_next_token_is (parser, CPP_COMMA))
@@ -4234,6 +4236,13 @@ c_parser_attributes (c_parser *parser)
 		  release_tree_vector (expr_list);
 		}
 	    }
+
+	  if (attr_args
+	      && TREE_VALUE (attr_args)
+	      && TREE_CODE (TREE_VALUE (attr_args)) == IDENTIFIER_NODE)
+	    TREE_VALUE (attr_args)
+	      = canonize_attr_name (TREE_VALUE (attr_args));
+
 	  attr = build_tree_list (attr_name, attr_args);
 	  if (c_parser_next_token_is (parser, CPP_CLOSE_PAREN))
 	    c_parser_consume_token (parser);
diff --git a/gcc/cp/parser.c b/gcc/cp/parser.c
index d02ad360d16..ea6b9a61390 100644
--- a/gcc/cp/parser.c
+++ b/gcc/cp/parser.c
@@ -24612,7 +24612,8 @@ cp_parser_gnu_attribute_list (cp_parser* parser)
 	       parsed identifier.  */
 	    ? ridpointers[(int) token->keyword]
 	    : id_token->u.value;
-	  
+
+	  identifier = canonize_attr_name (identifier);
 	  attribute = build_tree_list (identifier, NULL_TREE);
 
 	  /* Peek at the next token.  */
@@ -24638,6 +24639,11 @@ cp_parser_gnu_attribute_list (cp_parser* parser)
 	      else
 		{
 		  arguments = build_tree_list_vec (vec);
+		  tree tv;
+		  if (arguments != NULL_TREE
+		      && ((tv = TREE_VALUE (arguments)) != NULL_TREE)
+		      && TREE_CODE (tv) == IDENTIFIER_NODE)
+		      TREE_VALUE (arguments) = canonize_attr_name (tv);
 		  release_tree_vector (vec);
 		}
 	      /* Save the arguments away.  */
@@ -24758,6 +24764,8 @@ cp_parser_std_attribute (cp_parser *parser, tree attr_ns)
 		    "expected an identifier for the attribute name");
 	  return error_mark_node;
 	}
+
+      attr_id = canonize_attr_name (attr_id);
       attribute = build_tree_list (build_tree_list (attr_ns, attr_id),
 				   NULL_TREE);
       token = cp_lexer_peek_token (parser->lexer);
@@ -24767,6 +24775,7 @@ cp_parser_std_attribute (cp_parser *parser, tree attr_ns)
 				 NULL_TREE);
   else
     {
+      attr_id = canonize_attr_name (attr_id);
       attribute = build_tree_list (build_tree_list (NULL_TREE, attr_id),
 				   NULL_TREE);
       /* C++11 noreturn attribute is equivalent to GNU's.  */
diff --git a/gcc/go/go-gcc.cc b/gcc/go/go-gcc.cc
index a7977fe03c1..04912f0ed01 100644
--- a/gcc/go/go-gcc.cc
+++ b/gcc/go/go-gcc.cc
@@ -3046,7 +3046,7 @@ Gcc_backend::function(Btype* fntype, const std::string& name,
     DECL_UNINLINABLE(decl) = 1;
   if (disable_split_stack)
     {
-      tree attr = get_identifier("__no_split_stack__");
+      tree attr = get_identifier ("no_split_stack");
       DECL_ATTRIBUTES(decl) = tree_cons(attr, NULL_TREE, NULL_TREE);
     }
   if (in_unique_section)
diff --git a/gcc/testsuite/g++.dg/cpp0x/pr65558.C b/gcc/testsuite/g++.dg/cpp0x/pr65558.C
index d294c95a657..12946b35eda 100644
--- a/gcc/testsuite/g++.dg/cpp0x/pr65558.C
+++ b/gcc/testsuite/g++.dg/cpp0x/pr65558.C
@@ -2,6 +2,6 @@
 // { dg-do compile { target c++11 } }
 
 inline namespace
-__attribute__((__abi_tag__)) // { dg-warning "ignoring .__abi_tag__. attribute on anonymous namespace" }
+__attribute__((__abi_tag__)) // { dg-warning "ignoring .abi_tag. attribute on anonymous namespace" }
 {
 }
diff --git a/gcc/testsuite/gcc.dg/parm-impl-decl-1.c b/gcc/testsuite/gcc.dg/parm-impl-decl-1.c
index 5c7ddb0a259..c1219273c75 100644
--- a/gcc/testsuite/gcc.dg/parm-impl-decl-1.c
+++ b/gcc/testsuite/gcc.dg/parm-impl-decl-1.c
@@ -7,7 +7,7 @@
 /* Implicit function declaration in attribute in definition (testcase
    from bug).  */
 int
-foo (int __attribute__ ((__mode__ (vector_size(8)))) i) /* { dg-warning "'__mode__' attribute ignored" } */
+foo (int __attribute__ ((__mode__ (vector_size(8)))) i) /* { dg-warning "'mode' attribute ignored" } */
 {
   return (long long) i;
 }
diff --git a/gcc/testsuite/gcc.dg/parm-impl-decl-3.c b/gcc/testsuite/gcc.dg/parm-impl-decl-3.c
index 904295258d7..20197b52402 100644
--- a/gcc/testsuite/gcc.dg/parm-impl-decl-3.c
+++ b/gcc/testsuite/gcc.dg/parm-impl-decl-3.c
@@ -4,7 +4,7 @@
 /* { dg-options "-g -std=gnu89" } */
 
 int
-foo (int __attribute__ ((__mode__ (vector_size(8)))) i) /* { dg-warning "'__mode__' attribute ignored" } */
+foo (int __attribute__ ((__mode__ (vector_size(8)))) i) /* { dg-warning "'mode' attribute ignored" } */
 {
   return (long long) i;
 }
diff --git a/gcc/tree.c b/gcc/tree.c
index 260280317bc..542fe0fa9bc 100644
--- a/gcc/tree.c
+++ b/gcc/tree.c
@@ -4943,34 +4943,16 @@ cmp_attrib_identifiers (const_tree attr1, const_tree attr2)
   if (attr1 == attr2)
     return true;
 
-  /* If they are not equal, they may still be one in the form
-     'text' while the other one is in the form '__text__'.  TODO:
-     If we were storing attributes in normalized 'text' form, then
-     this could all go away and we could take full advantage of
-     the fact that we're comparing identifiers. :-)  */
   const size_t attr1_len = IDENTIFIER_LENGTH (attr1);
   const size_t attr2_len = IDENTIFIER_LENGTH (attr2);
 
-  if (attr2_len == attr1_len + 4)
-    {
-      const char *p = IDENTIFIER_POINTER (attr2);
-      const char *q = IDENTIFIER_POINTER (attr1);
-      if (p[0] == '_' && p[1] == '_'
-	  && p[attr2_len - 2] == '_' && p[attr2_len - 1] == '_'
-	  && strncmp (q, p + 2, attr1_len) == 0)
-	return true;;
-    }
-  else if (attr2_len + 4 == attr1_len)
-    {
-      const char *p = IDENTIFIER_POINTER (attr2);
-      const char *q = IDENTIFIER_POINTER (attr1);
-      if (q[0] == '_' && q[1] == '_'
-	  && q[attr1_len - 2] == '_' && q[attr1_len - 1] == '_'
-	  && strncmp (q + 2, p, attr2_len) == 0)
-	return true;
-    }
+  if (attr1_len != attr2_len)
+    return false;
+  gcc_checking_assert (IDENTIFIER_POINTER (attr1)[0] != '_');
+  gcc_checking_assert (IDENTIFIER_POINTER (attr2)[0] != '_');
 
-  return false;
+  return strncmp (IDENTIFIER_POINTER (attr1), IDENTIFIER_POINTER (attr2),
+		  attr1_len) == 0;
 }
 
 /* Compare two attributes for their value identity.  Return true if the
@@ -6047,24 +6029,9 @@ bool
 private_is_attribute_p (const char *attr_name, size_t attr_len, const_tree ident)
 {
   size_t ident_len = IDENTIFIER_LENGTH (ident);
+  gcc_checking_assert (ident_len == 0 || IDENTIFIER_POINTER (ident)[0] != '_');
 
-  if (ident_len == attr_len)
-    {
-      if (id_equal (ident, attr_name))
-	return true;
-    }
-  else if (ident_len == attr_len + 4)
-    {
-      /* There is the possibility that ATTR is 'text' and IDENT is
-	 '__text__'.  */
-      const char *p = IDENTIFIER_POINTER (ident);      
-      if (p[0] == '_' && p[1] == '_'
-	  && p[ident_len - 2] == '_' && p[ident_len - 1] == '_'
-	  && strncmp (attr_name, p + 2, attr_len) == 0)
-	return true;
-    }
-
-  return false;
+  return ident_len == attr_len && id_equal (ident, attr_name);
 }
 
 /* The backbone of lookup_attribute().  ATTR_LEN is the string length
@@ -6074,25 +6041,13 @@ private_lookup_attribute (const char *attr_name, size_t attr_len, tree list)
 {
   while (list)
     {
-      size_t ident_len = IDENTIFIER_LENGTH (get_attribute_name (list));
-
-      if (ident_len == attr_len)
-	{
-	  if (!strcmp (attr_name,
-		       IDENTIFIER_POINTER (get_attribute_name (list))))
+      tree attr = get_attribute_name (list);
+      size_t ident_len = IDENTIFIER_LENGTH (attr);
+      gcc_checking_assert (ident_len == 0
+			   || IDENTIFIER_POINTER (attr)[0] != '_');
+      if (ident_len == attr_len
+	  && strcmp (attr_name, IDENTIFIER_POINTER (attr)) == 0)
 	    break;
-	}
-      /* TODO: If we made sure that attributes were stored in the
-	 canonical form without '__...__' (ie, as in 'text' as opposed
-	 to '__text__') then we could avoid the following case.  */
-      else if (ident_len == attr_len + 4)
-	{
-	  const char *p = IDENTIFIER_POINTER (get_attribute_name (list));
-	  if (p[0] == '_' && p[1] == '_'
-	      && p[ident_len - 2] == '_' && p[ident_len - 1] == '_'
-	      && strncmp (attr_name, p + 2, attr_len) == 0)
-	    break;
-	}
       list = TREE_CHAIN (list);
     }
 
@@ -6119,17 +6074,11 @@ private_lookup_attribute_by_prefix (const char *attr_name, size_t attr_len,
 	}
 
       const char *p = IDENTIFIER_POINTER (get_attribute_name (list));
+      gcc_checking_assert (attr_len == 0 || p[0] != '_');
 
       if (strncmp (attr_name, p, attr_len) == 0)
 	break;
 
-      /* TODO: If we made sure that attributes were stored in the
-	 canonical form without '__...__' (ie, as in 'text' as opposed
-	 to '__text__') then we could avoid the following case.  */
-      if (p[0] == '_' && p[1] == '_' &&
-	  strncmp (attr_name, p + 2, attr_len) == 0)
-	break;
-
       list = TREE_CHAIN (list);
     }
 
@@ -6175,16 +6124,16 @@ tree
 remove_attribute (const char *attr_name, tree list)
 {
   tree *p;
-  size_t attr_len = strlen (attr_name);
-
   gcc_checking_assert (attr_name[0] != '_');
 
   for (p = &list; *p; )
     {
       tree l = *p;
-      /* TODO: If we were storing attributes in normalized form, here
-	 we could use a simple strcmp().  */
-      if (private_is_attribute_p (attr_name, attr_len, get_attribute_name (l)))
+
+      tree attr = get_attribute_name (l);
+      gcc_checking_assert (IDENTIFIER_LENGTH (attr) == 0
+			   || IDENTIFIER_POINTER (attr)[0] != '_');
+      if (strcmp (attr_name, IDENTIFIER_POINTER (attr)) == 0)
 	*p = TREE_CHAIN (l);
       else
 	p = &TREE_CHAIN (l);
@@ -14544,6 +14493,23 @@ get_nonnull_args (const_tree fntype)
   return argmap;
 }
 
+tree
+canonize_attr_name (tree attr_name)
+{
+  const size_t l = IDENTIFIER_LENGTH (attr_name);
+  const char *s = IDENTIFIER_POINTER (attr_name);
+
+  if (l > 4 && s[0] == '_')
+    {
+      gcc_assert (s[1] == '_');
+      gcc_assert (s[l - 2] == '_');
+      gcc_assert (s[l - 1] == '_');
+      return get_identifier_with_length (s + 2, l - 4);
+    }
+
+  return attr_name;
+}
+
 #if CHECKING_P
 
 namespace selftest {
diff --git a/gcc/tree.h b/gcc/tree.h
index bfe83f7c0a2..e58d362a216 100644
--- a/gcc/tree.h
+++ b/gcc/tree.h
@@ -4186,6 +4186,10 @@ extern tree private_lookup_attribute (const char *, size_t, tree);
    of lookup_attribute_by_prefix() and you should never call it directly.  */
 extern tree private_lookup_attribute_by_prefix (const char *, size_t, tree);
 
+/* For a given IDENTIFIER_NODE, strip leading and trailing '_' characters
+   so that we have a canonized shape or attribute names.  */
+extern tree canonize_attr_name (tree attr_name);
+
 /* Given an attribute name ATTR_NAME and a list of attributes LIST,
    return a pointer to the attribute's list element if the attribute
    is part of the list, or NULL_TREE if not found.  If the attribute


^ permalink raw reply	[flat|nested] 37+ messages in thread
end of thread, other threads:[~2017-09-12 14:25 UTC | newest]

Thread overview: 37+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-06-13 12:32 [PATCH][RFC] Canonize names of attributes Martin Liška
2017-06-13 13:20 ` Richard Biener
2017-06-14  7:48   ` Martin Liška
2017-06-14  9:07     ` Richard Biener
2017-06-14 11:03       ` Martin Liška
2017-06-14 11:19         ` Richard Biener
2017-06-14 16:40       ` Joseph Myers
2017-06-28 14:45         ` Martin Liška
2017-06-14 17:24 ` Jason Merrill
2017-06-16  0:07   ` Martin Sebor
2017-06-28 14:46   ` Martin Liška
2017-06-28 16:06     ` Joseph Myers
2017-06-28 19:01       ` Jason Merrill
2017-06-30  9:23         ` [PATCH v2][RFC] " Martin Liška
2017-06-30 19:35           ` Jason Merrill
2017-07-03  9:52             ` Martin Liška
2017-07-03 21:00               ` Jason Merrill
2017-07-11 13:38                 ` Martin Liška
2017-07-11 15:52                   ` Jason Merrill
2017-07-13 13:48                     ` Martin Liška
2017-07-13 13:51                       ` [RFC][PATCH] Do refactoring of attribute functions and move them to attribs.[hc] Martin Liška
2017-07-14  7:23                         ` Jeff Law
2017-07-14  7:40                           ` Martin Liška
2017-08-04 13:53                           ` Martin Liška
2017-08-08  4:37                             ` Martin Liška
2017-08-08  9:14                               ` Tom de Vries
2017-09-12  7:55                         ` Jakub Jelinek
2017-09-12 11:31                           ` Martin Liška
2017-09-12 11:40                             ` Jakub Jelinek
2017-09-12 14:25                               ` Martin Liška
2017-07-27 12:57                       ` [PATCH v2][RFC] Canonize names of attributes Martin Liška
2017-08-02 11:25                       ` Joseph Myers
2017-08-04 13:43                         ` Martin Liška
2017-08-04 16:54                           ` Jason Merrill
2017-08-07 16:44                             ` [PATCH][OBVIOUS] Fix missing include of header file in mips.c Martin Liška
2017-08-07 17:10                               ` [PATCH][OBVIOUS] Add missing header file attribs.h to couple of targets Martin Liška
2017-08-02 19:42                       ` [PATCH v2][RFC] Canonize names of attributes Jason Merrill

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