| Message ID | AANLkTi=WqqzyfhVYYhsz96jNjQqeALq+LjTordVs90Ew@mail.gmail.com |
|---|---|
| State | New |
| Headers | show |
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 >
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
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
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.
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
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);