Re: [PATCH] Fix -Wformat-diag for rs6000 target.
Hi! On Wed, Jan 12, 2022 at 10:02:36AM +0100, Martin Liška wrote: > - error ("%qs requires ISA 3.0 IEEE 128-bit floating point", name); > + error ("%qs requires ISA 3.0 IEEE 128-bit floating-point", name); This change is incorrect. Floating point is not an adjective here. It is probably best to just rewrite this text, of course. Maybe "%qs requires quad-precision floating-point arithmetic" or something like that (that it was first introduced in Power ISA 3.0 is dated already, for example). > - error ("%<%s%> not supported with %<-msoft-float%>", > + error ("%qs not supported with %<-msoft-float%>", This change is trivial and obviously correct. Please just commit such changes, *not* mixed in with anything else though. (And if you aren't sure something is trivial or obviously correct, it is not!) > - return scalar_extract_exp (source);/* { dg-error "requires ISA 3.0 > IEEE > 128-bit floating point" } */ > + return scalar_extract_exp (source);/* { dg-error "requires ISA 3.0 > IEEE > 128-bit floating-point" } */ Your patch is malformed. Please fix your mailer. Segher
Re: [PATCH] Fix -Wformat-diag for rs6000 target.
On 1/13/22 13:55, Richard Sandiford wrote: Like you say in the linked message, we could add an explicit noun too. But the change seems OK as-is to me. May I consider it as an approval of the suggested patch? Thanks, Martin
Re: [PATCH] Fix -Wformat-diag for rs6000 target.
On 1/13/22 05:55, Richard Sandiford wrote: Martin Sebor via Gcc-patches writes: On 1/12/22 02:02, 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 rs6000 target. Ready to be installed? Thanks, Martin gcc/ChangeLog: * config/rs6000/rs6000-call.c (rs6000_invalid_builtin): Wrap keywords and use %qs instead of %<%s%>. (rs6000_expand_builtin): Likewise. gcc/testsuite/ChangeLog: * gcc.target/powerpc/bfp/scalar-extract-exp-5.c: Adjust scans in testcases. * gcc.target/powerpc/bfp/scalar-extract-sig-5.c: Likewise. * gcc.target/powerpc/bfp/scalar-insert-exp-11.c: Likewise. --- gcc/config/rs6000/rs6000-call.c | 8 .../gcc.target/powerpc/bfp/scalar-extract-exp-5.c | 2 +- .../gcc.target/powerpc/bfp/scalar-extract-sig-5.c | 2 +- .../gcc.target/powerpc/bfp/scalar-insert-exp-11.c | 2 +- 4 files changed, 7 insertions(+), 7 deletions(-) diff --git a/gcc/config/rs6000/rs6000-call.c b/gcc/config/rs6000/rs6000-call.c index c78b8b08c40..becdad73812 100644 --- a/gcc/config/rs6000/rs6000-call.c +++ b/gcc/config/rs6000/rs6000-call.c @@ -3307,7 +3307,7 @@ rs6000_invalid_builtin (enum rs6000_gen_builtins fncode) "-mvsx"); break; case ENB_IEEE128_HW: - error ("%qs requires ISA 3.0 IEEE 128-bit floating point", name); + error ("%qs requires ISA 3.0 IEEE 128-bit floating-point", name); The instances of the warning where floating point is at the end of a message aren't correct. The warning should be relaxed to allow unhyphenated floating point as a noun (as discussed briefly last March: https://gcc.gnu.org/pipermail/gcc-patches/2021-March/566881.html) Wouldn't it be fair to say that “floating point” in the message above is really an adjective modifying an implicit noun? The floating (decimal) point doesn't itself have 128 bits. Like you say in the linked message, we could add an explicit noun too. But the change seems OK as-is to me. I agree you could say that too. I didn't mean what I said as an objection to the change but more as an observation that it shouldn't be necessary (and an acknowledgment that I haven't yet done what I said I'd do). Martin Thanks, Richard
Re: [PATCH] Fix -Wformat-diag for rs6000 target.
Martin Sebor via Gcc-patches writes: > On 1/12/22 02:02, 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 rs6000 target. >> >> Ready to be installed? >> Thanks, >> Martin >> >> >> gcc/ChangeLog: >> >> * config/rs6000/rs6000-call.c (rs6000_invalid_builtin): Wrap >> keywords and use %qs instead of %<%s%>. >> (rs6000_expand_builtin): Likewise. >> >> gcc/testsuite/ChangeLog: >> >> * gcc.target/powerpc/bfp/scalar-extract-exp-5.c: Adjust scans in >> testcases. >> * gcc.target/powerpc/bfp/scalar-extract-sig-5.c: Likewise. >> * gcc.target/powerpc/bfp/scalar-insert-exp-11.c: Likewise. >> --- >> gcc/config/rs6000/rs6000-call.c | 8 >> .../gcc.target/powerpc/bfp/scalar-extract-exp-5.c | 2 +- >> .../gcc.target/powerpc/bfp/scalar-extract-sig-5.c | 2 +- >> .../gcc.target/powerpc/bfp/scalar-insert-exp-11.c | 2 +- >> 4 files changed, 7 insertions(+), 7 deletions(-) >> >> diff --git a/gcc/config/rs6000/rs6000-call.c >> b/gcc/config/rs6000/rs6000-call.c >> index c78b8b08c40..becdad73812 100644 >> --- a/gcc/config/rs6000/rs6000-call.c >> +++ b/gcc/config/rs6000/rs6000-call.c >> @@ -3307,7 +3307,7 @@ rs6000_invalid_builtin (enum rs6000_gen_builtins >> fncode) >> "-mvsx"); >> break; >> case ENB_IEEE128_HW: >> - error ("%qs requires ISA 3.0 IEEE 128-bit floating point", name); >> + error ("%qs requires ISA 3.0 IEEE 128-bit floating-point", name); > > The instances of the warning where floating point is at the end > of a message aren't correct. The warning should be relaxed to > allow unhyphenated floating point as a noun (as discussed briefly > last March: > https://gcc.gnu.org/pipermail/gcc-patches/2021-March/566881.html) Wouldn't it be fair to say that “floating point” in the message above is really an adjective modifying an implicit noun? The floating (decimal) point doesn't itself have 128 bits. Like you say in the linked message, we could add an explicit noun too. But the change seems OK as-is to me. Thanks, Richard
Re: [PATCH] Fix -Wformat-diag for rs6000 target.
On 1/12/22 02:02, 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 rs6000 target. Ready to be installed? Thanks, Martin gcc/ChangeLog: * config/rs6000/rs6000-call.c (rs6000_invalid_builtin): Wrap keywords and use %qs instead of %<%s%>. (rs6000_expand_builtin): Likewise. gcc/testsuite/ChangeLog: * gcc.target/powerpc/bfp/scalar-extract-exp-5.c: Adjust scans in testcases. * gcc.target/powerpc/bfp/scalar-extract-sig-5.c: Likewise. * gcc.target/powerpc/bfp/scalar-insert-exp-11.c: Likewise. --- gcc/config/rs6000/rs6000-call.c | 8 .../gcc.target/powerpc/bfp/scalar-extract-exp-5.c | 2 +- .../gcc.target/powerpc/bfp/scalar-extract-sig-5.c | 2 +- .../gcc.target/powerpc/bfp/scalar-insert-exp-11.c | 2 +- 4 files changed, 7 insertions(+), 7 deletions(-) diff --git a/gcc/config/rs6000/rs6000-call.c b/gcc/config/rs6000/rs6000-call.c index c78b8b08c40..becdad73812 100644 --- a/gcc/config/rs6000/rs6000-call.c +++ b/gcc/config/rs6000/rs6000-call.c @@ -3307,7 +3307,7 @@ rs6000_invalid_builtin (enum rs6000_gen_builtins fncode) "-mvsx"); break; case ENB_IEEE128_HW: - error ("%qs requires ISA 3.0 IEEE 128-bit floating point", name); + error ("%qs requires ISA 3.0 IEEE 128-bit floating-point", name); break; case ENB_DFP: error ("%qs requires the %qs option", name, "-mhard-dfp"); @@ -5589,20 +5589,20 @@ rs6000_expand_builtin (tree exp, rtx target, rtx /* subtarget */, if (bif_is_nosoft (*bifaddr) && rs6000_isa_flags & OPTION_MASK_SOFT_FLOAT) { - error ("%<%s%> not supported with %<-msoft-float%>", + error ("%qs not supported with %<-msoft-float%>", bifaddr->bifname); return const0_rtx; } if (bif_is_no32bit (*bifaddr) && TARGET_32BIT) { - error ("%<%s%> is not supported in 32-bit mode", bifaddr->bifname); + error ("%qs is not supported in 32-bit mode", bifaddr->bifname); return const0_rtx; } if (bif_is_ibmld (*bifaddr) && !FLOAT128_2REG_P (TFmode)) { - error ("%<%s%> requires % to be IBM 128-bit format", + error ("%qs requires % to be IBM 128-bit format", bifaddr->bifname); return const0_rtx; } diff --git a/gcc/testsuite/gcc.target/powerpc/bfp/scalar-extract-exp-5.c b/gcc/testsuite/gcc.target/powerpc/bfp/scalar-extract-exp-5.c index 34184812dc5..1225613b275 100644 --- a/gcc/testsuite/gcc.target/powerpc/bfp/scalar-extract-exp-5.c +++ b/gcc/testsuite/gcc.target/powerpc/bfp/scalar-extract-exp-5.c @@ -14,7 +14,7 @@ get_exponent (__ieee128 *p) { __ieee128 source = *p; - return scalar_extract_exp (source); /* { dg-error "requires ISA 3.0 IEEE 128-bit floating point" } */ + return scalar_extract_exp (source); /* { dg-error "requires ISA 3.0 IEEE 128-bit floating-point" } */ This instance of the warning isn't correct. It should be relaxed to allow unhyphenated floating point as a noun at the end of a message, as discussed briefly last March: https://gcc.gnu.org/pipermail/gcc-patches/2021-March/566881.html Martin } diff --git a/gcc/testsuite/gcc.target/powerpc/bfp/scalar-extract-sig-5.c b/gcc/testsuite/gcc.target/powerpc/bfp/scalar-extract-sig-5.c index 13c64fc3acf..adf0e4f99df 100644 --- a/gcc/testsuite/gcc.target/powerpc/bfp/scalar-extract-sig-5.c +++ b/gcc/testsuite/gcc.target/powerpc/bfp/scalar-extract-sig-5.c @@ -12,5 +12,5 @@ get_significand (__ieee128 *p) { __ieee128 source = *p; - return (long long int) __builtin_vec_scalar_extract_sig (source); /* { dg-error "requires ISA 3.0 IEEE 128-bit floating point" } */ + return (long long int) __builtin_vec_scalar_extract_sig (source); /* { dg-error "requires ISA 3.0 IEEE 128-bit floating-point" } */ } diff --git a/gcc/testsuite/gcc.target/powerpc/bfp/scalar-insert-exp-11.c b/gcc/testsuite/gcc.target/powerpc/bfp/scalar-insert-exp-11.c index a5dd852e60f..6787a67950b 100644 --- a/gcc/testsuite/gcc.target/powerpc/bfp/scalar-insert-exp-11.c +++ b/gcc/testsuite/gcc.target/powerpc/bfp/scalar-insert-exp-11.c @@ -16,5 +16,5 @@ insert_exponent (__ieee128 *significand_p, __ieee128 significand = *significand_p; unsigned long long int exponent = *exponent_p; - return scalar_insert_exp (significand, exponent); /* { dg-error "requires ISA 3.0 IEEE 128-bit floating point" } */ + return scalar_insert_exp (significand, exponent); /* { dg-error "requires ISA 3.0 IEEE 128-bit floating-point" } */ }
Re: [PATCH] Fix -Wformat-diag for rs6000 target.
On 1/12/22 02:02, 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 rs6000 target. Ready to be installed? Thanks, Martin gcc/ChangeLog: * config/rs6000/rs6000-call.c (rs6000_invalid_builtin): Wrap keywords and use %qs instead of %<%s%>. (rs6000_expand_builtin): Likewise. gcc/testsuite/ChangeLog: * gcc.target/powerpc/bfp/scalar-extract-exp-5.c: Adjust scans in testcases. * gcc.target/powerpc/bfp/scalar-extract-sig-5.c: Likewise. * gcc.target/powerpc/bfp/scalar-insert-exp-11.c: Likewise. --- gcc/config/rs6000/rs6000-call.c | 8 .../gcc.target/powerpc/bfp/scalar-extract-exp-5.c | 2 +- .../gcc.target/powerpc/bfp/scalar-extract-sig-5.c | 2 +- .../gcc.target/powerpc/bfp/scalar-insert-exp-11.c | 2 +- 4 files changed, 7 insertions(+), 7 deletions(-) diff --git a/gcc/config/rs6000/rs6000-call.c b/gcc/config/rs6000/rs6000-call.c index c78b8b08c40..becdad73812 100644 --- a/gcc/config/rs6000/rs6000-call.c +++ b/gcc/config/rs6000/rs6000-call.c @@ -3307,7 +3307,7 @@ rs6000_invalid_builtin (enum rs6000_gen_builtins fncode) "-mvsx"); break; case ENB_IEEE128_HW: - error ("%qs requires ISA 3.0 IEEE 128-bit floating point", name); + error ("%qs requires ISA 3.0 IEEE 128-bit floating-point", name); The instances of the warning where floating point is at the end of a message aren't correct. The warning should be relaxed to allow unhyphenated floating point as a noun (as discussed briefly last March: https://gcc.gnu.org/pipermail/gcc-patches/2021-March/566881.html) Martin break; case ENB_DFP: error ("%qs requires the %qs option", name, "-mhard-dfp"); @@ -5589,20 +5589,20 @@ rs6000_expand_builtin (tree exp, rtx target, rtx /* subtarget */, if (bif_is_nosoft (*bifaddr) && rs6000_isa_flags & OPTION_MASK_SOFT_FLOAT) { - error ("%<%s%> not supported with %<-msoft-float%>", + error ("%qs not supported with %<-msoft-float%>", bifaddr->bifname); return const0_rtx; } if (bif_is_no32bit (*bifaddr) && TARGET_32BIT) { - error ("%<%s%> is not supported in 32-bit mode", bifaddr->bifname); + error ("%qs is not supported in 32-bit mode", bifaddr->bifname); return const0_rtx; } if (bif_is_ibmld (*bifaddr) && !FLOAT128_2REG_P (TFmode)) { - error ("%<%s%> requires % to be IBM 128-bit format", + error ("%qs requires % to be IBM 128-bit format", bifaddr->bifname); return const0_rtx; } diff --git a/gcc/testsuite/gcc.target/powerpc/bfp/scalar-extract-exp-5.c b/gcc/testsuite/gcc.target/powerpc/bfp/scalar-extract-exp-5.c index 34184812dc5..1225613b275 100644 --- a/gcc/testsuite/gcc.target/powerpc/bfp/scalar-extract-exp-5.c +++ b/gcc/testsuite/gcc.target/powerpc/bfp/scalar-extract-exp-5.c @@ -14,7 +14,7 @@ get_exponent (__ieee128 *p) { __ieee128 source = *p; - return scalar_extract_exp (source); /* { dg-error "requires ISA 3.0 IEEE 128-bit floating point" } */ + return scalar_extract_exp (source); /* { dg-error "requires ISA 3.0 IEEE 128-bit floating-point" } */ } diff --git a/gcc/testsuite/gcc.target/powerpc/bfp/scalar-extract-sig-5.c b/gcc/testsuite/gcc.target/powerpc/bfp/scalar-extract-sig-5.c index 13c64fc3acf..adf0e4f99df 100644 --- a/gcc/testsuite/gcc.target/powerpc/bfp/scalar-extract-sig-5.c +++ b/gcc/testsuite/gcc.target/powerpc/bfp/scalar-extract-sig-5.c @@ -12,5 +12,5 @@ get_significand (__ieee128 *p) { __ieee128 source = *p; - return (long long int) __builtin_vec_scalar_extract_sig (source); /* { dg-error "requires ISA 3.0 IEEE 128-bit floating point" } */ + return (long long int) __builtin_vec_scalar_extract_sig (source); /* { dg-error "requires ISA 3.0 IEEE 128-bit floating-point" } */ } diff --git a/gcc/testsuite/gcc.target/powerpc/bfp/scalar-insert-exp-11.c b/gcc/testsuite/gcc.target/powerpc/bfp/scalar-insert-exp-11.c index a5dd852e60f..6787a67950b 100644 --- a/gcc/testsuite/gcc.target/powerpc/bfp/scalar-insert-exp-11.c +++ b/gcc/testsuite/gcc.target/powerpc/bfp/scalar-insert-exp-11.c @@ -16,5 +16,5 @@ insert_exponent (__ieee128 *significand_p, __ieee128 significand = *significand_p; unsigned long long int exponent = *exponent_p; - return scalar_insert_exp (significand, exponent); /* { dg-error "requires ISA 3.0 IEEE 128-bit floating point" } */ + return scalar_insert_exp (significand, exponent); /* { dg-error "requires ISA 3.0 IEEE 128-bit floating-point" } */ }
[PATCH] Fix -Wformat-diag for rs6000 target.
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 rs6000 target. Ready to be installed? Thanks, Martin gcc/ChangeLog: * config/rs6000/rs6000-call.c (rs6000_invalid_builtin): Wrap keywords and use %qs instead of %<%s%>. (rs6000_expand_builtin): Likewise. gcc/testsuite/ChangeLog: * gcc.target/powerpc/bfp/scalar-extract-exp-5.c: Adjust scans in testcases. * gcc.target/powerpc/bfp/scalar-extract-sig-5.c: Likewise. * gcc.target/powerpc/bfp/scalar-insert-exp-11.c: Likewise. --- gcc/config/rs6000/rs6000-call.c | 8 .../gcc.target/powerpc/bfp/scalar-extract-exp-5.c | 2 +- .../gcc.target/powerpc/bfp/scalar-extract-sig-5.c | 2 +- .../gcc.target/powerpc/bfp/scalar-insert-exp-11.c | 2 +- 4 files changed, 7 insertions(+), 7 deletions(-) diff --git a/gcc/config/rs6000/rs6000-call.c b/gcc/config/rs6000/rs6000-call.c index c78b8b08c40..becdad73812 100644 --- a/gcc/config/rs6000/rs6000-call.c +++ b/gcc/config/rs6000/rs6000-call.c @@ -3307,7 +3307,7 @@ rs6000_invalid_builtin (enum rs6000_gen_builtins fncode) "-mvsx"); break; case ENB_IEEE128_HW: - error ("%qs requires ISA 3.0 IEEE 128-bit floating point", name); + error ("%qs requires ISA 3.0 IEEE 128-bit floating-point", name); break; case ENB_DFP: error ("%qs requires the %qs option", name, "-mhard-dfp"); @@ -5589,20 +5589,20 @@ rs6000_expand_builtin (tree exp, rtx target, rtx /* subtarget */, if (bif_is_nosoft (*bifaddr) && rs6000_isa_flags & OPTION_MASK_SOFT_FLOAT) { - error ("%<%s%> not supported with %<-msoft-float%>", + error ("%qs not supported with %<-msoft-float%>", bifaddr->bifname); return const0_rtx; } if (bif_is_no32bit (*bifaddr) && TARGET_32BIT) { - error ("%<%s%> is not supported in 32-bit mode", bifaddr->bifname); + error ("%qs is not supported in 32-bit mode", bifaddr->bifname); return const0_rtx; } if (bif_is_ibmld (*bifaddr) && !FLOAT128_2REG_P (TFmode)) { - error ("%<%s%> requires % to be IBM 128-bit format", + error ("%qs requires % to be IBM 128-bit format", bifaddr->bifname); return const0_rtx; } diff --git a/gcc/testsuite/gcc.target/powerpc/bfp/scalar-extract-exp-5.c b/gcc/testsuite/gcc.target/powerpc/bfp/scalar-extract-exp-5.c index 34184812dc5..1225613b275 100644 --- a/gcc/testsuite/gcc.target/powerpc/bfp/scalar-extract-exp-5.c +++ b/gcc/testsuite/gcc.target/powerpc/bfp/scalar-extract-exp-5.c @@ -14,7 +14,7 @@ get_exponent (__ieee128 *p) { __ieee128 source = *p; - return scalar_extract_exp (source); /* { dg-error "requires ISA 3.0 IEEE 128-bit floating point" } */ + return scalar_extract_exp (source); /* { dg-error "requires ISA 3.0 IEEE 128-bit floating-point" } */ } diff --git a/gcc/testsuite/gcc.target/powerpc/bfp/scalar-extract-sig-5.c b/gcc/testsuite/gcc.target/powerpc/bfp/scalar-extract-sig-5.c index 13c64fc3acf..adf0e4f99df 100644 --- a/gcc/testsuite/gcc.target/powerpc/bfp/scalar-extract-sig-5.c +++ b/gcc/testsuite/gcc.target/powerpc/bfp/scalar-extract-sig-5.c @@ -12,5 +12,5 @@ get_significand (__ieee128 *p) { __ieee128 source = *p; - return (long long int) __builtin_vec_scalar_extract_sig (source); /* { dg-error "requires ISA 3.0 IEEE 128-bit floating point" } */ + return (long long int) __builtin_vec_scalar_extract_sig (source); /* { dg-error "requires ISA 3.0 IEEE 128-bit floating-point" } */ } diff --git a/gcc/testsuite/gcc.target/powerpc/bfp/scalar-insert-exp-11.c b/gcc/testsuite/gcc.target/powerpc/bfp/scalar-insert-exp-11.c index a5dd852e60f..6787a67950b 100644 --- a/gcc/testsuite/gcc.target/powerpc/bfp/scalar-insert-exp-11.c +++ b/gcc/testsuite/gcc.target/powerpc/bfp/scalar-insert-exp-11.c @@ -16,5 +16,5 @@ insert_exponent (__ieee128 *significand_p, __ieee128 significand = *significand_p; unsigned long long int exponent = *exponent_p; - return scalar_insert_exp (significand, exponent); /* { dg-error "requires ISA 3.0 IEEE 128-bit floating point" } */ + return scalar_insert_exp (significand, exponent); /* { dg-error "requires ISA 3.0 IEEE 128-bit floating-point" } */ } -- 2.34.1