Fix -Wformat-diag for rs6000 target.
Commit Message
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(-)
Comments
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 %<long double%> to be IBM 128-bit format",
> + error ("%qs requires %<long double%> 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" } */
> }
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 %<long double%> to be IBM 128-bit format",
> + error ("%qs requires %<long double%> 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" } */
> }
Martin Sebor via Gcc-patches <gcc-patches@gcc.gnu.org> 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
On 1/13/22 05:55, Richard Sandiford wrote:
> Martin Sebor via Gcc-patches <gcc-patches@gcc.gnu.org> 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
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
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
@@ -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 %<long double%> to be IBM 128-bit format",
+ error ("%qs requires %<long double%> to be IBM 128-bit format",
bifaddr->bifname);
return const0_rtx;
}
@@ -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" } */
}
@@ -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" } */
}
@@ -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" } */
}