On 1/12/22 02:03, Martin Liška wrote:
Hello.

We've got -Wformat-diag for some time and I think we should start using it
in -Werror for GCC bootstrap. The following patch removes last pieces of the warning
for s390x target.

Ready to be installed?
Thanks,
Martin


gcc/ChangeLog:

     * config/s390/s390-c.c (s390_expand_overloaded_builtin): Wrap
     keyword in quotes.
     (s390_resolve_overloaded_builtin): Remove trailing dot.
     * config/s390/s390.c (s390_const_operand_ok): Use - for range.
     (s390_expand_builtin): Remove trailing dot.
     (s390_emit_prologue): Likewise, use semicolon.
     (s390_option_override_internal): Update keyword.
     * varasm.c (do_assemble_alias): Wrap keyword in quotes.
---
  gcc/config/s390/s390-c.c |  9 +++++----
  gcc/config/s390/s390.c   | 28 ++++++++++++++--------------
  gcc/varasm.c             |  2 +-
  3 files changed, 20 insertions(+), 19 deletions(-)

diff --git a/gcc/config/s390/s390-c.c b/gcc/config/s390/s390-c.c
index 600018421df..10bc6ac8900 100644
--- a/gcc/config/s390/s390-c.c
+++ b/gcc/config/s390/s390-c.c
@@ -484,7 +484,8 @@ s390_expand_overloaded_builtin (location_t loc,
      case S390_OVERLOADED_BUILTIN_s390_vec_step:
        if (TREE_CODE (TREE_TYPE ((*arglist)[0])) != VECTOR_TYPE)
      {
-      error_at (loc, "builtin vec_step can only be used on vector types.");
+      error_at (loc, "builtin %qs can only be used on vector types",
+            "vec_step ");

I'd have expected the warning to also trigger for the missing
hyphen in "builtin" (as per the GCC coding conventions) but it
looks like the code only looks for "builtin function".  I must
have done that because of the large number of misspellings.

Regardless, if the name of the function is __builtin_vec_step
(or __builtin_s390_vec_step?) then quoting the entire name of
the function as is conventionally done in the rest of GCC would
be one solution.  Rather than hardcoding the name as a string
there should be a way to do that by passing the right tree to
%qD (or %qF like below).  Based on my reading of the rest of
the file I wonder if this might be the decl:

s390_builtin_decls[bt_for_overloaded_builtin_var[S390_OVERLOADED_BUILTIN_s390_vec_step]]

Alternatively, if the preferred name to call the function with
is vec_step then simply vec_step without the "builtin" would
suffice.

(As an aside, there's a spurious space at the end of "vec_step "
above.)

        return error_mark_node;
      }
        return build_int_cst (NULL_TREE,
@@ -905,7 +906,7 @@ s390_resolve_overloaded_builtin (location_t loc,
        if (ob_flags & B_INT)
      {
        error_at (loc,
-            "builtin %qF is for GCC internal use only.",
+            "builtin %qF is for GCC internal use only",
              ob_fndecl);
        return error_mark_node;
      }
@@ -913,7 +914,7 @@ s390_resolve_overloaded_builtin (location_t loc,
      }

    if (ob_flags & B_DEP)
-    warning_at (loc, 0, "builtin %qF is deprecated.", ob_fndecl);
+    warning_at (loc, 0, "builtin %qF is deprecated", ob_fndecl);

    if (!TARGET_VX && (ob_flags & B_VX))
      {
@@ -1021,7 +1022,7 @@ s390_resolve_overloaded_builtin (location_t loc,
      }

    if (bflags_overloaded_builtin_var[last_match_index] & B_DEP)
-    warning_at (loc, 0, "%qs matching variant is deprecated.",
+    warning_at (loc, 0, "%qs matching variant is deprecated",
          IDENTIFIER_POINTER (DECL_NAME (ob_fndecl)));

    /* Overloaded variants which have MAX set as low level builtin are
diff --git a/gcc/config/s390/s390.c b/gcc/config/s390/s390.c
index 056002e4a4a..bf96cbf7588 100644
--- a/gcc/config/s390/s390.c
+++ b/gcc/config/s390/s390.c
@@ -766,7 +766,7 @@ s390_const_operand_ok (tree arg, int argnum, int op_flags, tree decl)
               argnum, decl, values);
          }
        else
-        error ("constant argument %d for builtin %qF is out of range (0..%wu)", +        error ("constant argument %d for builtin %qF is out of range (0-%wu)",
             argnum, decl, (HOST_WIDE_INT_1U << bitwidth) - 1);

        return false;
@@ -783,7 +783,7 @@ s390_const_operand_ok (tree arg, int argnum, int op_flags, tree decl)
        || tree_to_shwi (arg) > ((HOST_WIDE_INT_1 << (bitwidth - 1)) - 1))
      {
        error ("constant argument %d for builtin %qF is out of range "
-         "(%wd..%wd)", argnum, decl,
+         "(%wd-%wd)", argnum, decl,
           -(HOST_WIDE_INT_1 << (bitwidth - 1)),
           (HOST_WIDE_INT_1 << (bitwidth - 1)) - 1);
        return false;
@@ -832,25 +832,25 @@ s390_expand_builtin (tree exp, rtx target, rtx subtarget ATTRIBUTE_UNUSED,
        if ((bflags & B_HTM) && !TARGET_HTM)
      {
        error ("builtin %qF is not supported without %<-mhtm%> "
-         "(default with %<-march=zEC12%> and higher).", fndecl);
+         "(default with %<-march=zEC12%> and higher)", fndecl);
        return const0_rtx;
      }
        if (((bflags & B_VX) || (bflags & B_VXE)) && !TARGET_VX)
      {
        error ("builtin %qF requires %<-mvx%> "
-         "(default with %<-march=z13%> and higher).", fndecl);
+         "(default with %<-march=z13%> and higher)", fndecl);
        return const0_rtx;
      }

        if ((bflags & B_VXE) && !TARGET_VXE)
      {
-      error ("Builtin %qF requires z14 or higher.", fndecl);
+      error ("Builtin %qF requires z14 or higher", fndecl);

I'd expect the leading capital B to be flagged by the warning
after this change.  It's silent about leading capitals in full
sentences that end in a period (I think that's the GNU FORTRAN
style) but it complains about them otherwise.

Martin

        return const0_rtx;
      }

        if ((bflags & B_VXE2) && !TARGET_VXE2)
      {
-      error ("Builtin %qF requires z15 or higher.", fndecl);
+      error ("Builtin %qF requires z15 or higher", fndecl);
        return const0_rtx;
      }

@@ -11464,8 +11464,8 @@ s390_emit_prologue (void)
          {
            warning (0, "frame size of function %qs is %wd"
                 " bytes exceeding user provided stack limit of "
-               "%d bytes.  "
-               "An unconditional trap is added.",
+               "%d bytes; "
+               "an unconditional trap is added",
                 current_function_name(), cfun_frame_layout.frame_size,
                 s390_stack_size);
            emit_insn (gen_trap ());
@@ -11479,9 +11479,9 @@ s390_emit_prologue (void)
            if (stack_guard >= s390_stack_size)
          {
            warning (0, "frame size of function %qs is %wd"
-               " bytes which is more than half the stack size. "
-               "The dynamic check would not be reliable. "
-               "No check emitted for this function.",
+               " bytes which is more than half the stack size; "
+               "the dynamic check would not be reliable; "
+               "no check emitted for this function",
                 current_function_name(),
                 cfun_frame_layout.frame_size);
          }
@@ -15557,11 +15557,11 @@ s390_option_override_internal (struct gcc_options *opts,
        if (TARGET_HARD_DFP_P (opts_set->x_target_flags))
      {
        if (!TARGET_CPU_DFP_P (opts))
-        error ("hardware decimal floating point instructions"
+        error ("hardware decimal floating-point instructions"
             " not available on %s",
             processor_table[(int)opts->x_s390_arch].name);
        if (!TARGET_ZARCH_P (opts->x_target_flags))
-        error ("hardware decimal floating point instructions"
+        error ("hardware decimal floating-point instructions"
             " not available in ESA/390 mode");
      }
        else
@@ -15573,7 +15573,7 @@ s390_option_override_internal (struct gcc_options *opts,
      {
        if (TARGET_HARD_DFP_P (opts_set->x_target_flags)
        && TARGET_HARD_DFP_P (opts->x_target_flags))
-    error ("%<-mhard-dfp%> can%'t be used in conjunction with "
+    error ("%<-mhard-dfp%> cannot be used in conjunction with "
             "%<-msoft-float%>");

        opts->x_target_flags &= ~MASK_HARD_DFP;
diff --git a/gcc/varasm.c b/gcc/varasm.c
index 76574be191f..0cbec5fd7fd 100644
--- a/gcc/varasm.c
+++ b/gcc/varasm.c
@@ -6190,7 +6190,7 @@ do_assemble_alias (tree decl, tree target)
        if (!TARGET_SUPPORTS_WEAK)
      {
        error_at (DECL_SOURCE_LOCATION (decl),
-            "weakref is not supported in this configuration");
+            "%qs is not supported in this configuration", "weakref ");
        return;
      }
  #endif

Reply via email to