Patchwork [i386,c,c++] : PR/12171 - calling convention omitted in error message

login
register
mail settings
Submitter Kai Tietz
Date Dec. 22, 2010, 6:24 p.m.
Message ID <AANLkTi=WqqzyfhVYYhsz96jNjQqeALq+LjTordVs90Ew@mail.gmail.com>
Download mbox | patch
Permalink /patch/76438/
State New
Headers show

Comments

Kai Tietz - Dec. 22, 2010, 6:24 p.m.
Hello,

sorry, I sent patch under wrong bug-report. If there is common
agreement for displaying for such types all attributes without
filtering for ABI modifying ones, I will rework that patch.

2010-12-22  Kai Tietz

	PR target/12171
	* c-family/c-pretty-print.c: Add target.h include.
	(pp_c_specifier_qualifier_list): Call
	pp_c_calling_convention_attributes for (*).
	(pp_c_calling_convention_attributes): New.
	* c-family/c-pretty-print.h (pp_c_calling_convention_attributes):
	New prototype.
	* cp/error.c (dump_type_prefix): Call
	pp_c_calling_convention_attributes for (*).
	* cp/cxx-pretty-print.c (pp_cxx_ptr_operator):
	Likewise.
	* config/i386/i386.c (ix86_attribute_affects_calling_convention):
	New hook.
	(TARGET_ATTRIBUTE_AFFECTS_CALLING_CONVENTION): Define.
	* doc/tm.texi.in (TARGET_ATTRIBUTE_AFFECTS_CALLING_CONVENTION): Add.
	* doc/tm.texi: Regenerated.

Regards,
Kai
Gabriel Dos Reis - Dec. 23, 2010, 2:29 a.m.
On Wed, Dec 22, 2010 at 12:24 PM, Kai Tietz <ktietz70@googlemail.com> wrote:
> Hello,
>
> sorry, I sent patch under wrong bug-report. If there is common
> agreement for displaying for such types all attributes without
> filtering for ABI modifying ones, I will rework that patch.
>
> 2010-12-22  Kai Tietz
>
>        PR target/12171
>        * c-family/c-pretty-print.c: Add target.h include.
>        (pp_c_specifier_qualifier_list): Call
>        pp_c_calling_convention_attributes for (*).
>        (pp_c_calling_convention_attributes): New.
>        * c-family/c-pretty-print.h (pp_c_calling_convention_attributes):
>        New prototype.
>        * cp/error.c (dump_type_prefix): Call
>        pp_c_calling_convention_attributes for (*).
>        * cp/cxx-pretty-print.c (pp_cxx_ptr_operator):
>        Likewise.
>        * config/i386/i386.c (ix86_attribute_affects_calling_convention):
>        New hook.
>        (TARGET_ATTRIBUTE_AFFECTS_CALLING_CONVENTION): Define.
>        * doc/tm.texi.in (TARGET_ATTRIBUTE_AFFECTS_CALLING_CONVENTION): Add.
>        * doc/tm.texi: Regenerated.

I'm satisfied with the patch.
I'm opposed to printing all attributes -- the resulting sea of
irrelevant information,
for most cases, is counterproductive.

>
> Regards,
> Kai
>
Jason Merrill - Dec. 23, 2010, 2:36 a.m.
On 12/22/2010 09:29 PM, Gabriel Dos Reis wrote:
> I'm opposed to printing all attributes -- the resulting sea of
> irrelevant information, for most cases, is counterproductive.

How often do types have anything in TYPE_ATTRIBUTES, much less 
irrelevant attributes?  I can't imagine such a change making any 
difference for most code, and it's likely to help for any code that it 
does affect.

(Adding Ian to the CC list since he apparently had an opinion)

Jason
Ian Taylor - Dec. 23, 2010, 7:10 a.m.
Jason Merrill <jason@redhat.com> writes:

> On 12/22/2010 09:29 PM, Gabriel Dos Reis wrote:
>> I'm opposed to printing all attributes -- the resulting sea of
>> irrelevant information, for most cases, is counterproductive.
>
> How often do types have anything in TYPE_ATTRIBUTES, much less
> irrelevant attributes?  I can't imagine such a change making any
> difference for most code, and it's likely to help for any code that it
> does affect.
>
> (Adding Ian to the CC list since he apparently had an opinion)

I am definitely opposed to printing all attributes for functions.

I'm not excited about printing attributes for types, although I agree
that it matters less there.  It seems wrong to print attributes like
"unused" or "may_alias" for types.  Note that "may_alias" is used in
<mmintrin.h> and friends.

Ian
Joseph S. Myers - Dec. 23, 2010, 12:25 p.m.
On Wed, 22 Dec 2010, Gabriel Dos Reis wrote:

> I'm satisfied with the patch.

I still maintain that even if you do wish to print only some attributes 
(in which case I'd say the hook should be a hook to say what target 
attributes to print, not one saying what's a calling convention 
attribute, since the two sets need not be the same) the duplication 
between ix86_attribute_affects_calling_convention and other functions in 
knowing what attributes are calling convention attributes is best avoided.  
And manually handling the possibility of leading and trailing __ in a back 
end is certainly a bad idea; we have a perfectly good is_attribute_p 
function to handle this in one place only.

Even better than a hook saying whether to print an attribute would be an 
additional field in struct attribute_spec saying whether an attribute 
(target or not) should be printed.  That way the information - if you do 
want to print only some attributes - would be provided via an existing 
data hook rather than needing a new function hook at all, and you can 
easily start printing some target-independent attributes as well if 
desired.
Gabriel Dos Reis - Dec. 23, 2010, 2:59 p.m.
On Thu, Dec 23, 2010 at 6:25 AM, Joseph S. Myers
<joseph@codesourcery.com> wrote:
> On Wed, 22 Dec 2010, Gabriel Dos Reis wrote:
>
>> I'm satisfied with the patch.
>
> I still maintain that even if you do wish to print only some attributes
> (in which case I'd say the hook should be a hook to say what target
> attributes to print, not one saying what's a calling convention
> attribute, since the two sets need not be the same) the duplication
> between ix86_attribute_affects_calling_convention and other functions in
> knowing what attributes are calling convention attributes is best avoided.
> And manually handling the possibility of leading and trailing __ in a back
> end is certainly a bad idea; we have a perfectly good is_attribute_p
> function to handle this in one place only.
>
> Even better than a hook saying whether to print an attribute would be an
> additional field in struct attribute_spec saying whether an attribute
> (target or not) should be printed.  That way the information - if you do
> want to print only some attributes - would be provided via an existing
> data hook rather than needing a new function hook at all, and you can
> easily start printing some target-independent attributes as well if
> desired.

I agree that code duplication is not a good idea, and your suggestion of
is_attribute_p is a very good one.

On ther other hand, we should not embark on the road of printing
all possible attributes in a type, so having a general purpose
hook for that does not strike me as a good think.  That leaves the
possibility of adding a field to attribute_spec which I like.

-- Gaby

Patch

Index: gcc/gcc/c-family/c-pretty-print.c
===================================================================
--- gcc.orig/gcc/c-family/c-pretty-print.c	2010-12-22 16:29:55.231892000 +0100
+++ gcc/gcc/c-family/c-pretty-print.c	2010-12-22 16:39:57.923249700 +0100
@@ -29,6 +29,7 @@  along with GCC; see the file COPYING3.  
 #include "tree-pretty-print.h"
 #include "tree-iterator.h"
 #include "diagnostic.h"
+#include "target.h"
 
 /* Translate if being used for diagnostics, but not for dump files or
    __PRETTY_FUNCTION.  */
@@ -460,6 +461,7 @@  pp_c_specifier_qualifier_list (c_pretty_
 	  {
 	    pp_c_whitespace (pp);
 	    pp_c_left_paren (pp);
+	    pp_c_calling_convention_attributes (pp, TYPE_ATTRIBUTES (pointee));
 	  }
 	else if (!c_dialect_cxx ())
 	  pp_c_whitespace (pp);
@@ -790,6 +792,45 @@  pp_c_attributes (c_pretty_printer *pp, t
   pp_c_right_paren (pp);
 }
 
+/* Pretty-print ATTRIBUTES using GNU C extension syntax for calling
+   convention affecting attributes.  */
+
+void
+pp_c_calling_convention_attributes (c_pretty_printer *pp, tree a)
+{
+  bool is_first = true;
+
+  if (a == NULL_TREE)
+    return;
+
+  for (; a != NULL_TREE; a = TREE_CHAIN (a))
+    {
+      if (!targetm.attribute_affects_calling_convention (TREE_PURPOSE (a)))
+        continue;
+      if (is_first)
+	{
+	  pp_c_ws_string (pp, "__attribute__");
+	  pp_c_left_paren (pp);
+	  pp_c_left_paren (pp);
+	  is_first = false;
+	}
+      else
+	{
+	  pp_separate_with (pp, ',');
+	}
+      pp_tree_identifier (pp, TREE_PURPOSE (a));
+      if (TREE_VALUE (a))
+	pp_c_call_argument_list (pp, TREE_VALUE (a));
+    }
+
+  if (!is_first)
+    {
+      pp_c_right_paren (pp);
+      pp_c_right_paren (pp);
+      pp_c_whitespace (pp);
+    }
+}
+
 /* function-definition:
       declaration-specifiers declarator compound-statement  */
 
Index: gcc/gcc/cp/error.c
===================================================================
--- gcc.orig/gcc/cp/error.c	2010-12-22 16:29:55.237892000 +0100
+++ gcc/gcc/cp/error.c	2010-12-22 16:39:58.001375200 +0100
@@ -661,6 +661,8 @@  dump_type_prefix (tree t, int flags)
 	  {
 	    pp_cxx_whitespace (cxx_pp);
 	    pp_cxx_left_paren (cxx_pp);
+	    pp_c_calling_convention_attributes (pp_c_base (cxx_pp),
+						TYPE_ATTRIBUTES (sub));
 	  }
 	if (TREE_CODE (t) == POINTER_TYPE)
 	  pp_character(cxx_pp, '*');
Index: gcc/gcc/c-family/c-pretty-print.h
===================================================================
--- gcc.orig/gcc/c-family/c-pretty-print.h	2010-12-22 16:29:55.232892000 +0100
+++ gcc/gcc/c-family/c-pretty-print.h	2010-12-22 16:39:58.032625400 +0100
@@ -176,6 +176,7 @@  void pp_c_space_for_pointer_operator (c_
 void pp_c_tree_decl_identifier (c_pretty_printer *, tree);
 void pp_c_function_definition (c_pretty_printer *, tree);
 void pp_c_attributes (c_pretty_printer *, tree);
+void pp_c_calling_convention_attributes (c_pretty_printer *, tree);
 void pp_c_cv_qualifiers (c_pretty_printer *pp, int qualifiers, bool func_type);
 void pp_c_type_qualifier_list (c_pretty_printer *, tree);
 void pp_c_parameter_type_list (c_pretty_printer *, tree);
Index: gcc/gcc/config/i386/i386.c
===================================================================
--- gcc.orig/gcc/config/i386/i386.c	2010-12-22 16:29:55.235892000 +0100
+++ gcc/gcc/config/i386/i386.c	2010-12-22 16:39:58.048250500 +0100
@@ -5116,6 +5116,46 @@  ix86_function_ok_for_sibcall (tree decl,
   return true;
 }
 
+static bool
+ix86_attribute_affects_calling_convention (tree name)
+{
+  int ident_len;
+  const char *p;
+
+  gcc_assert (TREE_CODE (name) == IDENTIFIER_NODE);
+  p = IDENTIFIER_POINTER (name);
+  ident_len = IDENTIFIER_LENGTH (name);
+
+  if (ident_len > 4 && p[0] == '_' && p[1] == '_' && p[ident_len - 1] == '_'
+      && p[ident_len - 2] == '_')
+    {
+      ident_len -= 4;
+      p += 2;
+    }
+  switch (ident_len)
+    {
+    case 5:
+      if (!strncmp (p, "cdecl", 5))
+        return true;
+      break;
+    case 6:
+      if (!strncmp (p, "ms_abi", 6))
+        return true;
+      break;
+    case 7:
+      if (!strncmp (p, "regparm", 7) || !strncmp (p, "stdcall", 7))
+        return true;
+      break;
+    case 8:
+      if (!strncmp (p, "thiscall", 8) || !strncmp (p, "fastcall", 8)
+          || !strncmp (p, "sysv_abi", 8))
+        return true;
+      break;
+    }
+
+  return false;
+}
+
 /* Handle "cdecl", "stdcall", "fastcall", "regparm", "thiscall",
    and "sseregparm" calling convention attributes;
    arguments as in struct attribute_spec.handler.  */
@@ -34813,6 +34853,10 @@  ix86_autovectorize_vector_sizes (void)
 #undef TARGET_CONDITIONAL_REGISTER_USAGE
 #define TARGET_CONDITIONAL_REGISTER_USAGE ix86_conditional_register_usage
 
+#undef TARGET_ATTRIBUTE_AFFECTS_CALLING_CONVENTION
+#define TARGET_ATTRIBUTE_AFFECTS_CALLING_CONVENTION \
+  ix86_attribute_affects_calling_convention
+
 struct gcc_target targetm = TARGET_INITIALIZER;
 
 #include "gt-i386.h"
Index: gcc/gcc/doc/tm.texi
===================================================================
--- gcc.orig/gcc/doc/tm.texi	2010-12-22 16:29:55.239892000 +0100
+++ gcc/gcc/doc/tm.texi	2010-12-22 16:39:58.157626200 +0100
@@ -9765,6 +9765,10 @@  to perform initial processing of the @sa
 @file{i386/i386.c}, for example.
 @end deftypefn
 
+@deftypefn {Target Hook} bool TARGET_ATTRIBUTE_AFFECTS_CALLING_CONVENTION (const_tree @var{name})
+Returns @code{true} if given attribute by its @var{name} affects calling convention, otherwise @code{false}.
+@end deftypefn
+
 @deftypefn {Target Hook} bool TARGET_VALID_DLLIMPORT_ATTRIBUTE_P (const_tree @var{decl})
 @var{decl} is a variable or function with @code{__attribute__((dllimport))} specified.  Use this hook if the target needs to add extra validation checks to @code{handle_dll_attribute}.
 @end deftypefn
Index: gcc/gcc/doc/tm.texi.in
===================================================================
--- gcc.orig/gcc/doc/tm.texi.in	2010-12-22 16:29:55.241892000 +0100
+++ gcc/gcc/doc/tm.texi.in	2010-12-22 16:39:58.282627000 +0100
@@ -9729,6 +9729,8 @@  to perform initial processing of the @sa
 @file{i386/i386.c}, for example.
 @end deftypefn
 
+@hook TARGET_ATTRIBUTE_AFFECTS_CALLING_CONVENTION
+
 @hook TARGET_VALID_DLLIMPORT_ATTRIBUTE_P
 
 @defmac TARGET_DECLSPEC
Index: gcc/gcc/target.def
===================================================================
--- gcc.orig/gcc/target.def	2010-12-22 16:29:55.243892000 +0100
+++ gcc/gcc/target.def	2010-12-22 16:39:58.282627000 +0100
@@ -1113,6 +1113,14 @@  DEFHOOK
  tree, (tree olddecl, tree newdecl),
  merge_decl_attributes)
 
+/* Return true iff attribute NAME affects calling convention. */
+DEFHOOK
+(attribute_affects_calling_convention,
+ "Returns @code{true} if given attribute by its @var{name} affects calling\
+ convention, otherwise @code{false}.",
+ bool, (const_tree name),
+ hook_bool_const_tree_false)
+
 /* Given two types, merge their attributes and return the result.  */
 DEFHOOK
 (merge_type_attributes,
Index: gcc/gcc/cp/cxx-pretty-print.c
===================================================================
--- gcc.orig/gcc/cp/cxx-pretty-print.c	2010-12-22 12:10:20.000000000 +0100
+++ gcc/gcc/cp/cxx-pretty-print.c	2010-12-22 16:48:14.020174700 +0100
@@ -1323,6 +1323,9 @@  pp_cxx_ptr_operator (cxx_pretty_printer 
       if (TREE_CODE (TREE_TYPE (t)) == POINTER_TYPE
 	  || TYPE_PTR_TO_MEMBER_P (TREE_TYPE (t)))
 	pp_cxx_ptr_operator (pp, TREE_TYPE (t));
+      pp_c_calling_convention_attributes (pp_c_base (pp),
+					  TYPE_ATTRIBUTES (TREE_TYPE (t)));
+
       if (TREE_CODE (t) == POINTER_TYPE)
 	{
 	  pp_star (pp);