diff mbox series

fix diagnostic quoting/spelling in rs6000

Message ID 611a47aa-3f45-a4be-3aff-03879cf43c69@suse.cz
State New
Headers show
Series fix diagnostic quoting/spelling in rs6000 | expand

Commit Message

Martin Liška May 20, 2019, 9:48 a.m. UTC
Hi.

The patch is about fallout of -Wformat-diag for ppc64le target where
I added quoting of various warnings and errors.

Patch can bootstrap on ppc64le-linux-gnu and survives regression tests.

Ready to be installed?
Thanks,
Martin

gcc/ChangeLog:

2019-05-16  Martin Liska  <mliska@suse.cz>

	* config/rs6000/driver-rs6000.c (elf_platform): Do not use
	an extra newline.
	* config/rs6000/rs6000-c.c (SYNTAX_ERROR): Wrap pragma in %<%>.
	(rs6000_pragma_longcall): Likewise/
	(altivec_resolve_overloaded_builtin): Likewise for vec_lvs*.
	* config/rs6000/rs6000.c (rs6000_option_override_internal):
	Quote a C type.
	(rs6000_function_arg): Likewise.
	(rs6000_expand_set_fpscr_drn_builtin): Remove trailing dot.
	(rs6000_expand_ternop_builtin): Use interval syntax.
	(get_element_number): Likewise.
	(altivec_expand_builtin): Likewise.
	(rs6000_get_function_versions_dispatcher): Quote target_clones.

Fix test-suite.

gcc/testsuite/ChangeLog:

2019-05-17  Martin Liska  <mliska@suse.cz>

	* gcc.target/powerpc/ppc64-abi-warn-1.c: Wrap a type.
	* gcc.target/powerpc/pr80315-1.c: Use new interval format.
	* gcc.target/powerpc/pr80315-2.c: Likewise.
	* gcc.target/powerpc/pr80315-3.c: Likewise.
	* gcc.target/powerpc/pr80315-4.c: Likewise.
	* gcc.target/powerpc/warn-lvsl-lvsr.c: Wrap builtin names.
---
 gcc/config/rs6000/driver-rs6000.c             |  2 +-
 gcc/config/rs6000/rs6000-c.c                  | 10 ++++-----
 gcc/config/rs6000/rs6000.c                    | 22 ++++++++++---------
 .../gcc.target/powerpc/ppc64-abi-warn-1.c     |  2 +-
 gcc/testsuite/gcc.target/powerpc/pr80315-1.c  |  2 +-
 gcc/testsuite/gcc.target/powerpc/pr80315-2.c  |  2 +-
 gcc/testsuite/gcc.target/powerpc/pr80315-3.c  |  2 +-
 gcc/testsuite/gcc.target/powerpc/pr80315-4.c  |  2 +-
 .../gcc.target/powerpc/warn-lvsl-lvsr.c       |  4 ++--
 9 files changed, 25 insertions(+), 23 deletions(-)

Comments

Jakub Jelinek May 20, 2019, 10:06 a.m. UTC | #1
On Mon, May 20, 2019 at 11:48:45AM +0200, Martin Liška wrote:
> --- a/gcc/config/rs6000/driver-rs6000.c
> +++ b/gcc/config/rs6000/driver-rs6000.c
> @@ -266,7 +266,7 @@ elf_platform (void)
>  	  fatal_error (
>  	    input_location,
>  	    "Unsupported cpu name returned from kernel for "
> -	    "%<-mcpu=native%>: %s\n"
> +	    "%<-mcpu=native%>: %s.  "
>  	    "Please use an explicit cpu name.  Valid cpu names are: %s",
>  	    cpu, s);

This one is still incorrect, shouldn't start with capital letter and there
shouldn't be separate sentences and full stops.
Either it can be split into two diagnostics,
  error ("unsupported cpu name returned from kernel for %<-mcpu=native%>: %s", cpu);
  fatal_error (input_location, "please use an explicit cpu name; valid cpu names are: %s", s);
or reword somehow.

	Jakub
Martin Liška May 20, 2019, 10:36 a.m. UTC | #2
On 5/20/19 12:06 PM, Jakub Jelinek wrote:
> On Mon, May 20, 2019 at 11:48:45AM +0200, Martin Liška wrote:
>> --- a/gcc/config/rs6000/driver-rs6000.c
>> +++ b/gcc/config/rs6000/driver-rs6000.c
>> @@ -266,7 +266,7 @@ elf_platform (void)
>>  	  fatal_error (
>>  	    input_location,
>>  	    "Unsupported cpu name returned from kernel for "
>> -	    "%<-mcpu=native%>: %s\n"
>> +	    "%<-mcpu=native%>: %s.  "
>>  	    "Please use an explicit cpu name.  Valid cpu names are: %s",
>>  	    cpu, s);
> 
> This one is still incorrect, shouldn't start with capital letter and there
> shouldn't be separate sentences and full stops.
> Either it can be split into two diagnostics,
>   error ("unsupported cpu name returned from kernel for %<-mcpu=native%>: %s", cpu);
>   fatal_error (input_location, "please use an explicit cpu name; valid cpu names are: %s", s);
> or reword somehow.
> 
> 	Jakub
> 

You are right. I'm fixing that in updated patch.

Thanks,
Martin
Segher Boessenkool May 20, 2019, 10:52 a.m. UTC | #3
Hi!

On Mon, May 20, 2019 at 11:48:45AM +0200, Martin Liška wrote:
> 	* config/rs6000/driver-rs6000.c (elf_platform): Do not use
> 	an extra newline.
> 	* config/rs6000/rs6000-c.c (SYNTAX_ERROR): Wrap pragma in %<%>.

Please say this is in rs6000_pragma_longcall.  You might want to move the
macro to inside the function, too.  Or get rid of it, for bonus points :-)

> 	(rs6000_pragma_longcall): Likewise/

Typo (slash instead of dot).

> 	(altivec_resolve_overloaded_builtin): Likewise for vec_lvs*.

Please spell out lvsl and lvsr.

> 	* config/rs6000/rs6000.c (rs6000_option_override_internal):
> 	Quote a C type.
> 	(rs6000_function_arg): Likewise.
> 	(rs6000_expand_set_fpscr_drn_builtin): Remove trailing dot.
> 	(rs6000_expand_ternop_builtin): Use interval syntax.

I find that syntax less readable, btw.  Square brackets have a different
meaning already, in C programs.  Interval syntax isn't normally used for
integers, fwiw (you use set notation, like {0...15}, instead).  0..15
is all of shorter, more readable, and less confusing than [0,15] or
[0, 15] (ugh) or {0...15}.

So yeah, I disapprove of that.  But please do the same in rs6000 as in
the rest of the compiler ;-)


Okay for trunk, with the changelog nits, and Jakub's comment addressed.

Thanks,


Segher
Martin Liška May 20, 2019, 11:05 a.m. UTC | #4
On 5/20/19 12:52 PM, Segher Boessenkool wrote:
> Hi!
> 
> On Mon, May 20, 2019 at 11:48:45AM +0200, Martin Liška wrote:
>> 	* config/rs6000/driver-rs6000.c (elf_platform): Do not use
>> 	an extra newline.
>> 	* config/rs6000/rs6000-c.c (SYNTAX_ERROR): Wrap pragma in %<%>.
> 
> Please say this is in rs6000_pragma_longcall.  You might want to move the
> macro to inside the function, too.  Or get rid of it, for bonus points :-)

I've done the movement.

> 
>> 	(rs6000_pragma_longcall): Likewise/
> 
> Typo (slash instead of dot).

Fixed.

> 
>> 	(altivec_resolve_overloaded_builtin): Likewise for vec_lvs*.
> 
> Please spell out lvsl and lvsr.

Likewise.

> 
>> 	* config/rs6000/rs6000.c (rs6000_option_override_internal):
>> 	Quote a C type.
>> 	(rs6000_function_arg): Likewise.
>> 	(rs6000_expand_set_fpscr_drn_builtin): Remove trailing dot.
>> 	(rs6000_expand_ternop_builtin): Use interval syntax.
> 
> I find that syntax less readable, btw.  Square brackets have a different
> meaning already, in C programs.  Interval syntax isn't normally used for
> integers, fwiw (you use set notation, like {0...15}, instead).  0..15
> is all of shorter, more readable, and less confusing than [0,15] or
> [0, 15] (ugh) or {0...15}.
> 
> So yeah, I disapprove of that.  But please do the same in rs6000 as in
> the rest of the compiler ;-)

Yeah, the format was selected by Martin Sebor, where he transformed e.g.

-      error ("requested init_priority is out of range");
+      error ("requested %<init_priority%> %i is out of range [0, %i]",

I'm just following that and I bet there are multiple existing occurrences
of the range notation at different places.

> 
> 
> Okay for trunk, with the changelog nits, and Jakub's comment addressed.
> 
> Thanks,

Thanks for review.
Martin

> 
> 
> Segher
>
Joseph Myers May 20, 2019, 11:31 p.m. UTC | #5
Where you refer to 'homogeneous %<float%> aggregates', are such aggregates 
in the rs6000 case (or in the case where the ABI changed) really 
restricted to the type float, or do they apply more generally to some 
other floating-point types so that expanding "float" to a longer 
description would be the appropriate fix?
Segher Boessenkool May 21, 2019, 10:12 a.m. UTC | #6
On Mon, May 20, 2019 at 11:31:17PM +0000, Joseph Myers wrote:
> Where you refer to 'homogeneous %<float%> aggregates', are such aggregates 
> in the rs6000 case (or in the case where the ABI changed) really 
> restricted to the type float, or do they apply more generally to some 
> other floating-point types so that expanding "float" to a longer 
> description would be the appropriate fix?

See https://gcc.gnu.org/ml/gcc-patches/2014-07/msg00632.html .

It's about single precision floating point only.


Segher
diff mbox series

Patch

diff --git a/gcc/config/rs6000/driver-rs6000.c b/gcc/config/rs6000/driver-rs6000.c
index b290eea4bdd..b58c1c91fc0 100644
--- a/gcc/config/rs6000/driver-rs6000.c
+++ b/gcc/config/rs6000/driver-rs6000.c
@@ -266,7 +266,7 @@  elf_platform (void)
 	  fatal_error (
 	    input_location,
 	    "Unsupported cpu name returned from kernel for "
-	    "%<-mcpu=native%>: %s\n"
+	    "%<-mcpu=native%>: %s.  "
 	    "Please use an explicit cpu name.  Valid cpu names are: %s",
 	    cpu, s);
 	}
diff --git a/gcc/config/rs6000/rs6000-c.c b/gcc/config/rs6000/rs6000-c.c
index 7c28d4d8176..a898cf6c55a 100644
--- a/gcc/config/rs6000/rs6000-c.c
+++ b/gcc/config/rs6000/rs6000-c.c
@@ -49,7 +49,7 @@ 
 
 #define SYNTAX_ERROR(gmsgid) do {					\
   warning (OPT_Wpragmas, gmsgid);					\
-  warning (OPT_Wpragmas, "ignoring malformed #pragma longcall");	\
+  warning (OPT_Wpragmas, "ignoring malformed %<#pragma longcall%>");	\
   return;								\
 } while (0)
 
@@ -72,7 +72,7 @@  rs6000_pragma_longcall (cpp_reader *pfile ATTRIBUTE_UNUSED)
     SYNTAX_ERROR ("number must be 0 or 1");
 
   if (pragma_lex (&x) != CPP_EOF)
-    warning (OPT_Wpragmas, "junk at end of #pragma longcall");
+    warning (OPT_Wpragmas, "junk at end of %<#pragma longcall%>");
 
   rs6000_default_long_calls = (n == integer_one_node);
 }
@@ -6140,11 +6140,11 @@  altivec_resolve_overloaded_builtin (location_t loc, tree fndecl,
   /* vec_lvsl and vec_lvsr are deprecated for use with LE element order.  */
   if (fcode == ALTIVEC_BUILTIN_VEC_LVSL && !BYTES_BIG_ENDIAN)
     warning (OPT_Wdeprecated,
-	     "vec_lvsl is deprecated for little endian; use "
+	     "%<vec_lvsl%> is deprecated for little endian; use "
 	     "assignment for unaligned loads and stores");
   else if (fcode == ALTIVEC_BUILTIN_VEC_LVSR && !BYTES_BIG_ENDIAN)
     warning (OPT_Wdeprecated,
-	     "vec_lvsr is deprecated for little endian; use "
+	     "%<vec_lvsr%> is deprecated for little endian; use "
 	     "assignment for unaligned loads and stores");
 
   if (fcode == ALTIVEC_BUILTIN_VEC_MUL)
@@ -6846,7 +6846,7 @@  altivec_resolve_overloaded_builtin (location_t loc, tree fndecl,
 	{
           if (TYPE_READONLY (TREE_TYPE (type))
 	      && !TYPE_READONLY (TREE_TYPE (decl_type)))
-	    warning (0, "passing arg %d of %qE discards qualifiers from "
+	    warning (0, "passing argument %d of %qE discards qualifiers from "
 		        "pointer target type", n + 1, fndecl);
 	  type = build_pointer_type (build_qualified_type (TREE_TYPE (type),
 							   0));
diff --git a/gcc/config/rs6000/rs6000.c b/gcc/config/rs6000/rs6000.c
index 2992ba545d7..7e1b11ad43a 100644
--- a/gcc/config/rs6000/rs6000.c
+++ b/gcc/config/rs6000/rs6000.c
@@ -4275,7 +4275,7 @@  rs6000_option_override_internal (bool global_init_p)
       if (main_target_opt != NULL
 	  && (main_target_opt->x_rs6000_long_double_type_size
 	      != default_long_double_size))
-	error ("target attribute or pragma changes long double size");
+	error ("target attribute or pragma changes %<long double%> size");
       else
 	rs6000_long_double_type_size = default_long_double_size;
     }
@@ -4310,9 +4310,11 @@  rs6000_option_override_internal (bool global_init_p)
 	    {
 	      warned_change_long_double = true;
 	      if (TARGET_IEEEQUAD)
-		warning (OPT_Wpsabi, "Using IEEE extended precision long double");
+		warning (OPT_Wpsabi, "Using IEEE extended precision "
+			 "%<long double%>");
 	      else
-		warning (OPT_Wpsabi, "Using IBM extended precision long double");
+		warning (OPT_Wpsabi, "Using IBM extended precision "
+			 "%<long double%>");
 	    }
 	}
     }
@@ -11791,7 +11793,7 @@  rs6000_function_arg (cumulative_args_t cum_v, machine_mode mode,
 		{
 		  warned = true;
 		  inform (input_location,
-			  "the ABI of passing homogeneous float aggregates"
+			  "the ABI of passing homogeneous %<float%> aggregates"
 			  " has changed in GCC 5");
 		}
 	    }
@@ -13227,7 +13229,7 @@  rs6000_expand_set_fpscr_drn_builtin (enum insn_code icode, tree exp)
     /* Builtin not supported in 32-bit mode.  */
     fatal_error (input_location,
 		 "%<__builtin_set_fpscr_drn%> is not supported "
-		 "in 32-bit mode.");
+		 "in 32-bit mode");
 
   if (rs6000_isa_flags & OPTION_MASK_SOFT_FLOAT)
     {
@@ -14250,7 +14252,7 @@  rs6000_expand_ternop_builtin (enum insn_code icode, tree exp, rtx target)
       if (TREE_CODE (arg2) != INTEGER_CST
 	  || wi::geu_p (wi::to_wide (arg2), 16))
 	{
-	  error ("argument 3 must be in the range 0..15");
+	  error ("argument 3 must be in the range [0, 15]");
 	  return CONST0_RTX (tmode);
 	}
     }
@@ -14383,7 +14385,7 @@  get_element_number (tree vec_type, tree arg)
   if (!tree_fits_uhwi_p (arg)
       || (elt = tree_to_uhwi (arg), elt > max))
     {
-      error ("selector must be an integer constant in the range 0..%wi", max);
+      error ("selector must be an integer constant in the range [0, %wi]", max);
       return 0;
     }
 
@@ -14703,7 +14705,7 @@  altivec_expand_builtin (tree exp, rtx target, bool *expandedp)
 
       if (TREE_CODE (arg1) != INTEGER_CST || TREE_INT_CST_LOW (arg1) > 12)
 	{
-	  error ("second argument to %qs must be 0..12", "vec_vextract4b");
+	  error ("second argument to %qs must be [0, 12]", "vec_vextract4b");
 	  return expand_call (exp, target, false);
 	}
       break;
@@ -14718,7 +14720,7 @@  altivec_expand_builtin (tree exp, rtx target, bool *expandedp)
 
       if (TREE_CODE (arg2) != INTEGER_CST || TREE_INT_CST_LOW (arg2) > 12)
 	{
-	  error ("third argument to %qs must be 0..12", "vec_vinsert4b");
+	  error ("third argument to %qs must be [0, 12]", "vec_vinsert4b");
 	  return expand_call (exp, target, false);
 	}
       break;
@@ -37238,7 +37240,7 @@  rs6000_get_function_versions_dispatcher (void *decl)
 
 #ifndef TARGET_LIBC_PROVIDES_HWCAP_IN_TCB
   error_at (DECL_SOURCE_LOCATION (default_node->decl),
-	    "target_clones attribute needs GLIBC (2.23 and newer) that "
+	    "%<target_clones%> attribute needs GLIBC (2.23 and newer) that "
 	    "exports hardware capability bits");
 #else
 
diff --git a/gcc/testsuite/gcc.target/powerpc/ppc64-abi-warn-1.c b/gcc/testsuite/gcc.target/powerpc/ppc64-abi-warn-1.c
index c70c14c2858..ae7d5746b0d 100644
--- a/gcc/testsuite/gcc.target/powerpc/ppc64-abi-warn-1.c
+++ b/gcc/testsuite/gcc.target/powerpc/ppc64-abi-warn-1.c
@@ -6,7 +6,7 @@  struct f8
     float x[8];
   };
 
-void test (struct f8 a, struct f8 b) /* { dg-message "note: the ABI of passing homogeneous float aggregates has changed" } */
+void test (struct f8 a, struct f8 b) /* { dg-message "note: the ABI of passing homogeneous 'float' aggregates has changed" } */
 {
 }
 
diff --git a/gcc/testsuite/gcc.target/powerpc/pr80315-1.c b/gcc/testsuite/gcc.target/powerpc/pr80315-1.c
index d0fb6dc1c06..2f670a0c6c7 100644
--- a/gcc/testsuite/gcc.target/powerpc/pr80315-1.c
+++ b/gcc/testsuite/gcc.target/powerpc/pr80315-1.c
@@ -10,6 +10,6 @@  main()
   int mask;
 
   /* Argument 2 must be 0 or 1.  Argument 3 must be in range 0..15.  */
-  res = __builtin_crypto_vshasigmaw (test, 1, 0xff); /* { dg-error "argument 3 must be in the range 0..15" } */
+  res = __builtin_crypto_vshasigmaw (test, 1, 0xff); /* { dg-error "argument 3 must be in the range \\[0, 15\\]" } */
   return 0;
 }
diff --git a/gcc/testsuite/gcc.target/powerpc/pr80315-2.c b/gcc/testsuite/gcc.target/powerpc/pr80315-2.c
index 30df7f18658..e95187e2179 100644
--- a/gcc/testsuite/gcc.target/powerpc/pr80315-2.c
+++ b/gcc/testsuite/gcc.target/powerpc/pr80315-2.c
@@ -10,6 +10,6 @@  main ()
   int mask;
 
   /* Argument 2 must be 0 or 1.  Argument 3 must be in range 0..15.  */
-  res = __builtin_crypto_vshasigmad (test, 1, 0xff); /* { dg-error "argument 3 must be in the range 0..15" } */
+  res = __builtin_crypto_vshasigmad (test, 1, 0xff); /* { dg-error "argument 3 must be in the range \\[0, 15\\]" } */
   return 0;
 }
diff --git a/gcc/testsuite/gcc.target/powerpc/pr80315-3.c b/gcc/testsuite/gcc.target/powerpc/pr80315-3.c
index 5365f1d516c..f1a782663a1 100644
--- a/gcc/testsuite/gcc.target/powerpc/pr80315-3.c
+++ b/gcc/testsuite/gcc.target/powerpc/pr80315-3.c
@@ -12,6 +12,6 @@  main ()
   int mask;
 
   /* Argument 2 must be 0 or 1.  Argument 3 must be in range 0..15.  */
-  res = vec_shasigma_be (test, 1, 0xff); /* { dg-error "argument 3 must be in the range 0..15" } */
+  res = vec_shasigma_be (test, 1, 0xff); /* { dg-error "argument 3 must be in the range \\[0, 15\\]" } */
   return res;
 }
diff --git a/gcc/testsuite/gcc.target/powerpc/pr80315-4.c b/gcc/testsuite/gcc.target/powerpc/pr80315-4.c
index eb513639939..66a3efce9c5 100644
--- a/gcc/testsuite/gcc.target/powerpc/pr80315-4.c
+++ b/gcc/testsuite/gcc.target/powerpc/pr80315-4.c
@@ -12,6 +12,6 @@  main ()
   int mask;
 
   /* Argument 2 must be 0 or 1.  Argument 3 must be in range 0..15.  */
-  res = vec_shasigma_be (test, 1, 0xff); /* { dg-error "argument 3 must be in the range 0..15" } */
+  res = vec_shasigma_be (test, 1, 0xff); /* { dg-error "argument 3 must be in the range \\[0, 15\\]" } */
   return res;
 }
diff --git a/gcc/testsuite/gcc.target/powerpc/warn-lvsl-lvsr.c b/gcc/testsuite/gcc.target/powerpc/warn-lvsl-lvsr.c
index bf889aaa22b..62b37a9c335 100644
--- a/gcc/testsuite/gcc.target/powerpc/warn-lvsl-lvsr.c
+++ b/gcc/testsuite/gcc.target/powerpc/warn-lvsl-lvsr.c
@@ -9,6 +9,6 @@  float f[20];
 
 void foo ()
 {
-  vector unsigned char a = vec_lvsl (4, f); /* { dg-warning "vec_lvsl is deprecated for little endian; use assignment for unaligned loads and stores" } */
-  vector unsigned char b = vec_lvsr (8, f); /* { dg-warning "vec_lvsr is deprecated for little endian; use assignment for unaligned loads and stores" } */
+  vector unsigned char a = vec_lvsl (4, f); /* { dg-warning "'vec_lvsl' is deprecated for little endian; use assignment for unaligned loads and stores" } */
+  vector unsigned char b = vec_lvsr (8, f); /* { dg-warning "'vec_lvsr' is deprecated for little endian; use assignment for unaligned loads and stores" } */
 }