On 8/28/25 10:22 AM, Alfie Richards wrote:
The 08/28/2025 15:56, Jason Merrill wrote:
On 8/28/25 8:59 AM, Alfie Richards wrote:
The 08/28/2025 14:43, Jason Merrill wrote:
On 8/28/25 5:49 AM, [email protected] wrote:
From: Alfie Richards <[email protected]>

Adds the target_version and target_clones attributes to diagnostic messages
for target_version semantics.

This is because the target_version/target_clones attributes affect the identity
of the decls, so need to be represented in diagnostics for them.

This also requires making maybe_print_whitespace available to c++ code so

pp_c_maybe_whitespace

But why do we need to call it in C++ when we don't in C?

   (Resending as I forgot to reply all)

Because I use it to print the white space after the attribute which isn't needed
in the C diagnostic.

The difficulty is I want C++ to do:

'float foo [[target_version("sve")]] ()'
(with a space before and after)

and

'float foo()'
with no spaces (unchanged from current formatting)

But for C I want:

'foo [[target_version("sve")]]'

Notably, without a space after the attribute.

The slightly ugly way I achieved this is that pp_c_function_target_version
sets pp->set_padding (pp_before) if it prints the attribute, so then
pp_cpp_maybe_whitespace can be used only in the C++ code to add the extra
white space only if the attribute is added.

I'm not happy with this way of achieving this, if you know a simpler
way to achieve this I would be happy to change it.

Hmm, I guess this approach makes sense since pp_c_left_paren/dump_parameters
don't ever call _maybe_whitespace.

But the _whitespace call between _version and _clones seems wrong; if we
didn't print _version then it won't do anything, if we did and are going to
also print _clones it's an extra space, if we did and aren't going to print
_clones it's redundant with the second _whitespace call.

Get agreed, I put it in on the offchance of a decl with both target_clones and
target_version, but that case is explicitly disallowed, and formatting them
without a space between would be fine anyway. I will remove it.

Is this okay with that change?

Yes.


we can control if whitespace is needed cosistantly between c and c++

"consistently"

diagnostics.

After this change diagnostics look like:

c:
```
test.c:6:8: error: redefinition of ‘foo [[target_version("sve")]]’
       6 | float  foo  () {return 1;}
         |        ^~~
test.c:3:8: note: previous definition of ‘foo [[target_version("sve")]]’ with 
type ‘float(void)’
       3 | float  foo  () {return 2;}
         |        ^~~
test.c:12:8: error: redefinition of ‘bar [[target_clones("sve")]]’
      12 | float  bar () {return 1;}
         |        ^~~
test.c:9:8: note: previous definition of ‘bar [[target_clones("sve")]]’ with 
type ‘float(void)’
       9 | float  bar () {return 2;}
         |        ^~~
```

c++:
```
test.cpp:6:8: error: redefinition of ‘float foo [[target_version("sve")]] ()’
       6 | float  foo  () {return 1;}
         |        ^~~
test.cpp:3:8: note: ‘float foo [[target_version("sve")]] ()’ previously defined 
here
       3 | float  foo  () {return 2;}
         |        ^~~
test.cpp:12:8: error: redefinition of ‘float bar [[target_clones("sve")]] ()’
      12 | float  bar () {return 1;}
         |        ^~~
test.cpp:9:8: note: ‘float bar [[target_clones("sve")]] ()’ previously defined 
here
       9 | float  bar () {return 2;}
         |        ^~~
```

This only affects targets which use target_version (aarch64 and riscv).

gcc/c-family/ChangeLog:

        * c-pretty-print.cc (pp_c_function_target_version): New function.
        (pp_c_function_target_clones): New function.
        (pp_c_maybe_whitespace): Move to c-pretty-print.h.
        * c-pretty-print.h (pp_c_function_target_version): New function.
        (pp_c_function_target_clones): New function.
        (pp_c_maybe_whitespace): Moved here from c-pretty-print.cc.

gcc/c/ChangeLog:

        * c-objc-common.cc (c_tree_printer): Add printing of target_clone and
        target_version in decl diagnostics.

gcc/cp/ChangeLog:

        * cxx-pretty-print.h (pp_cxx_function_target_version): New macro.
        (pp_cxx_function_target_clones): Ditto.
        (pp_cxx_maybe_whitespace): Ditto.
        * error.cc (dump_function_decl): Add printing of target_clone and
        target_version in decl diagnostics.
---
    gcc/c-family/c-pretty-print.cc | 77 +++++++++++++++++++++++++++++++---
    gcc/c-family/c-pretty-print.h  |  8 ++++
    gcc/c/c-objc-common.cc         |  6 +++
    gcc/cp/cxx-pretty-print.h      |  5 +++
    gcc/cp/error.cc                |  8 ++++
    5 files changed, 98 insertions(+), 6 deletions(-)

diff --git a/gcc/c-family/c-pretty-print.cc b/gcc/c-family/c-pretty-print.cc
index fad6b5eb9b0..5adb13b9784 100644
--- a/gcc/c-family/c-pretty-print.cc
+++ b/gcc/c-family/c-pretty-print.cc
@@ -36,6 +36,7 @@ along with GCC; see the file COPYING3.  If not see
    #include "function.h"
    #include "basic-block.h"
    #include "gimple.h"
+#include "tm.h"
    /* The pretty-printer code is primarily designed to closely follow
       (GNU) C and C++ grammars.  That is to be contrasted with spaghetti
@@ -45,12 +46,6 @@ along with GCC; see the file COPYING3.  If not see
       takes expression or declaration contexts into account.  */
-#define pp_c_maybe_whitespace(PP)            \
-   do {                                      \
-     if ((PP)->get_padding () == pp_before)  \
-       pp_c_whitespace (PP);                 \
-   } while (0)
-
    /* literal  */
    static void pp_c_char (c_pretty_printer *, int);
@@ -3054,6 +3049,76 @@ pp_c_tree_decl_identifier (c_pretty_printer *pp, tree t)
      pp_c_identifier (pp, name);
    }
+/* Prints "[version: VERSION]" for a versioned function decl.
+   This will only print for targets with target_version semantics.  */
+void
+pp_c_function_target_version (c_pretty_printer *pp, tree t)
+{
+  if (TARGET_HAS_FMV_TARGET_ATTRIBUTE)
+    return;
+
+  string_slice version = get_target_version (t);
+  if (!version.is_valid ())
+    return;
+
+  pp_c_whitespace (pp);
+
+  pp_c_left_bracket (pp);
+  pp_c_left_bracket (pp);
+  pp_string (pp, "target_version");
+  pp_c_left_paren (pp);
+  pp_doublequote (pp);
+  pp_string_n (pp, version.begin (), version.size ());
+  pp_doublequote (pp);
+  pp_c_right_paren (pp);
+  pp_c_right_bracket (pp);
+  pp_c_right_bracket (pp);
+
+  pp->set_padding (pp_before);
+}
+
+/* Prints "[clones: VERSION, +]" for a versioned function decl.
+   This only works for targets with target_version semantics.  */
+void
+pp_c_function_target_clones (c_pretty_printer *pp, tree t)
+{
+  /* Only print for target_version semantics.
+     This is because for target FMV semantics a target_clone always defines
+     the entire FMV set.  target_version semantics can mix target_clone and
+     target_version decls in the definition of a FMV set and so the
+     target_clone becomes a part of the identity of the declaration.  */
+  if (TARGET_HAS_FMV_TARGET_ATTRIBUTE)
+    return;
+
+  auto_vec<string_slice> versions = get_clone_versions (t, NULL, false);
+  if (versions.is_empty ())
+    return;
+
+  pp_c_whitespace (pp);
+
+  string_slice final_version = versions.pop ();
+  pp_c_left_bracket (pp);
+  pp_c_left_bracket (pp);
+  pp_string (pp, "target_clones");
+  pp_c_left_paren (pp);
+  for (string_slice version : versions)
+    {
+      pp_doublequote (pp);
+      pp_string_n (pp, version.begin (), version.size ());
+      pp_doublequote (pp);
+      pp_string (pp, ",");
+      pp_c_whitespace (pp);
+    }
+  pp_doublequote (pp);
+  pp_string_n (pp, final_version.begin (), final_version.size ());
+  pp_doublequote (pp);
+  pp_c_right_paren (pp);
+  pp_c_right_bracket (pp);
+  pp_c_right_bracket (pp);
+
+  pp->set_padding (pp_before);
+}
+
    #if CHECKING_P
    namespace selftest {
diff --git a/gcc/c-family/c-pretty-print.h b/gcc/c-family/c-pretty-print.h
index c8fb6789991..5e00b952c81 100644
--- a/gcc/c-family/c-pretty-print.h
+++ b/gcc/c-family/c-pretty-print.h
@@ -102,6 +102,12 @@ public:
    #define pp_ptr_operator(PP, D)          (PP)->ptr_operator (PP, D)
    #define pp_parameter_list(PP, T)        (PP)->parameter_list (PP, T)
+#define pp_c_maybe_whitespace(PP)            \
+   do {                                      \
+     if ((PP)->get_padding () == pp_before)  \
+       pp_c_whitespace (PP);                 \
+   } while (0)
+
    void pp_c_whitespace (c_pretty_printer *);
    void pp_c_left_paren (c_pretty_printer *);
    void pp_c_right_paren (c_pretty_printer *);
@@ -138,6 +144,8 @@ void pp_c_ws_string (c_pretty_printer *, const char *);
    void pp_c_identifier (c_pretty_printer *, const char *);
    void pp_c_string_literal (c_pretty_printer *, tree);
    void pp_c_integer_constant (c_pretty_printer *, tree);
+void pp_c_function_target_version (c_pretty_printer *, tree);
+void pp_c_function_target_clones (c_pretty_printer *, tree);
    void print_c_tree (FILE *file, tree t, dump_flags_t);
diff --git a/gcc/c/c-objc-common.cc b/gcc/c/c-objc-common.cc
index 5c50983544d..109a8cb33ee 100644
--- a/gcc/c/c-objc-common.cc
+++ b/gcc/c/c-objc-common.cc
@@ -23,6 +23,7 @@ along with GCC; see the file COPYING3.  If not see
    #include "c-tree.h"
    #include "intl.h"
    #include "c-family/c-pretty-print.h"
+#include "tree-core.h"
    #include "tree-pretty-print.h"
    #include "tree-pretty-print-markup.h"
    #include "gimple-pretty-print.h"
@@ -359,6 +360,11 @@ c_tree_printer (pretty_printer *pp, text_info *text, const 
char *spec,
          if (DECL_NAME (t))
        {
          pp_identifier (cpp, lang_hooks.decl_printable_name (t, 2));
+         if (TREE_CODE (t) == FUNCTION_DECL)
+           {
+             pp_c_function_target_version (cpp, t);
+             pp_c_function_target_clones (cpp, t);
+           }
          return true;
        }
          break;
diff --git a/gcc/cp/cxx-pretty-print.h b/gcc/cp/cxx-pretty-print.h
index 86e0ce7d85f..356fd6e4d23 100644
--- a/gcc/cp/cxx-pretty-print.h
+++ b/gcc/cp/cxx-pretty-print.h
@@ -81,8 +81,13 @@ public:
    #define pp_cxx_ws_string(PP, I)             pp_c_ws_string (PP, I)
    #define pp_cxx_identifier(PP, I)    pp_c_identifier (PP, I)
+#define pp_cxx_maybe_whitespace(PP)    pp_c_maybe_whitespace (PP)
    #define pp_cxx_tree_identifier(PP, T) \
      pp_c_tree_identifier (PP, T)
+#define pp_cxx_function_target_version(PP, T) \
+  pp_c_function_target_version (PP, T)
+#define pp_cxx_function_target_clones(PP, T) \
+  pp_c_function_target_clones (PP, T)
    void pp_cxx_begin_template_argument_list (cxx_pretty_printer *);
    void pp_cxx_end_template_argument_list (cxx_pretty_printer *);
diff --git a/gcc/cp/error.cc b/gcc/cp/error.cc
index 8ef9f9ee458..16dfeafc07a 100644
--- a/gcc/cp/error.cc
+++ b/gcc/cp/error.cc
@@ -2008,6 +2008,14 @@ dump_function_decl (cxx_pretty_printer *pp, tree t, int 
flags)
      dump_function_name (pp, t, dump_function_name_flags);
+  /* By default we need no padding here, but if we emit target_version or
+     target_clones then we need some.  */
+  pp->set_padding (pp_none);
+  pp_cxx_function_target_version (pp, t);
+  pp_cxx_maybe_whitespace (pp);
+  pp_cxx_function_target_clones (pp, t);
+  pp_cxx_maybe_whitespace (pp);
+
      if (!(flags & TFF_NO_FUNCTION_ARGUMENTS))
        {
          int const parm_flags





Reply via email to