rs6000: Builtins test changes for test_fpscr_[d]rn_builtin_error.c

Message ID 0f4ed5e2-35ef-bc01-4bc3-b57018abf8fa@linux.ibm.com
State New
Headers
Series rs6000: Builtins test changes for test_fpscr_[d]rn_builtin_error.c |

Commit Message

Pop, Sebastian via Gcc-patches Nov. 18, 2021, 4:36 p.m. UTC
  Hi!  This is the last patch broken out of the previous test suite patch
for the new builtins support.

One advantage of the new builtins support is uniform error messages for
arguments with restricted values.  Previously this was done in many places
in an ad hoc manner, with little uniformity.  This patch adjusts the
expected error messages accordingly.

All such error messages are now one of the following:
  "argument %d must be a %d-bit unsigned literal"
  "argument %d must be a literal between %d and %d, inclusive"
  "argument %d must be a variable or a literal between %d and %d, inclusive"
  "argument %d must be either a literal %d or a literal %d"

These messages were chosen to require the fewest changes from previous
messages while still introducing uniformity.  This patch adjusts error
messages for some cases where this produces changed messages.  In
particular, some messages are improved because previously they did not
admit the possibility that an argument could hold a variable.

Tested on powerpc64le-linux-gnu and powerpc64-linux-gnu (-m32/-m64)
with no regressions.  Is this okay for trunk?

Thanks!
Bill


2021-11-17  Bill Schmidt  <wschmidt@linux.ibm.com>

gcc/testsuite/
	* gcc.target/powerpc/test_fpscr_drn_builtin_error.c: Adjust error
	messages.
	* gcc.target/powerpc/test_fpscr_rn_builtin_error.c: Likewise.
---
 .../powerpc/test_fpscr_drn_builtin_error.c           |  4 ++--
 .../gcc.target/powerpc/test_fpscr_rn_builtin_error.c | 12 ++++++------
 2 files changed, 8 insertions(+), 8 deletions(-)
  

Comments

Pop, Sebastian via Gcc-patches Dec. 1, 2021, 4:36 p.m. UTC | #1
Hi!  I'd like to ping this patch.

Thanks!
Bill

On 11/18/21 10:36 AM, Bill Schmidt wrote:
> Hi!  This is the last patch broken out of the previous test suite patch
> for the new builtins support.
>
> One advantage of the new builtins support is uniform error messages for
> arguments with restricted values.  Previously this was done in many places
> in an ad hoc manner, with little uniformity.  This patch adjusts the
> expected error messages accordingly.
>
> All such error messages are now one of the following:
>   "argument %d must be a %d-bit unsigned literal"
>   "argument %d must be a literal between %d and %d, inclusive"
>   "argument %d must be a variable or a literal between %d and %d, inclusive"
>   "argument %d must be either a literal %d or a literal %d"
>
> These messages were chosen to require the fewest changes from previous
> messages while still introducing uniformity.  This patch adjusts error
> messages for some cases where this produces changed messages.  In
> particular, some messages are improved because previously they did not
> admit the possibility that an argument could hold a variable.
>
> Tested on powerpc64le-linux-gnu and powerpc64-linux-gnu (-m32/-m64)
> with no regressions.  Is this okay for trunk?
>
> Thanks!
> Bill
>
>
> 2021-11-17  Bill Schmidt  <wschmidt@linux.ibm.com>
>
> gcc/testsuite/
> 	* gcc.target/powerpc/test_fpscr_drn_builtin_error.c: Adjust error
> 	messages.
> 	* gcc.target/powerpc/test_fpscr_rn_builtin_error.c: Likewise.
> ---
>  .../powerpc/test_fpscr_drn_builtin_error.c           |  4 ++--
>  .../gcc.target/powerpc/test_fpscr_rn_builtin_error.c | 12 ++++++------
>  2 files changed, 8 insertions(+), 8 deletions(-)
>
> diff --git a/gcc/testsuite/gcc.target/powerpc/test_fpscr_drn_builtin_error.c b/gcc/testsuite/gcc.target/powerpc/test_fpscr_drn_builtin_error.c
> index 028ab0b6d66..4f9d9e08e8a 100644
> --- a/gcc/testsuite/gcc.target/powerpc/test_fpscr_drn_builtin_error.c
> +++ b/gcc/testsuite/gcc.target/powerpc/test_fpscr_drn_builtin_error.c
> @@ -9,8 +9,8 @@ int main ()
>       __builtin_set_fpscr_drn() also support a variable as an argument but
>       can't test variable value at compile time.  */
>  
> -  __builtin_set_fpscr_drn(-1);  /* { dg-error "Argument must be a value between 0 and 7" } */ 
> -  __builtin_set_fpscr_drn(8);   /* { dg-error "Argument must be a value between 0 and 7" } */ 
> +  __builtin_set_fpscr_drn(-1);  /* { dg-error "argument 1 must be a variable or a literal between 0 and 7, inclusive" } */ 
> +  __builtin_set_fpscr_drn(8);   /* { dg-error "argument 1 must be a variable or a literal between 0 and 7, inclusive" } */ 
>  
>  }
>  
> diff --git a/gcc/testsuite/gcc.target/powerpc/test_fpscr_rn_builtin_error.c b/gcc/testsuite/gcc.target/powerpc/test_fpscr_rn_builtin_error.c
> index aea65091b0c..10391b71008 100644
> --- a/gcc/testsuite/gcc.target/powerpc/test_fpscr_rn_builtin_error.c
> +++ b/gcc/testsuite/gcc.target/powerpc/test_fpscr_rn_builtin_error.c
> @@ -8,13 +8,13 @@ int main ()
>       int arguments.  The builtins __builtin_set_fpscr_rn() also supports a
>       variable as an argument but can't test variable value at compile time.  */
>  
> -  __builtin_mtfsb0(-1);  /* { dg-error "Argument must be a constant between 0 and 31" } */
> -  __builtin_mtfsb0(32);  /* { dg-error "Argument must be a constant between 0 and 31" } */
> +  __builtin_mtfsb0(-1);  /* { dg-error "argument 1 must be a 5-bit unsigned literal" } */
> +  __builtin_mtfsb0(32);  /* { dg-error "argument 1 must be a 5-bit unsigned literal" } */
>  
> -  __builtin_mtfsb1(-1);  /* { dg-error "Argument must be a constant between 0 and 31" } */
> -  __builtin_mtfsb1(32);  /* { dg-error "Argument must be a constant between 0 and 31" } */ 
> +  __builtin_mtfsb1(-1);  /* { dg-error "argument 1 must be a 5-bit unsigned literal" } */
> +  __builtin_mtfsb1(32);  /* { dg-error "argument 1 must be a 5-bit unsigned literal" } */ 
>  
> -  __builtin_set_fpscr_rn(-1);  /* { dg-error "Argument must be a value between 0 and 3" } */ 
> -  __builtin_set_fpscr_rn(4);   /* { dg-error "Argument must be a value between 0 and 3" } */ 
> +  __builtin_set_fpscr_rn(-1);  /* { dg-error "argument 1 must be a variable or a literal between 0 and 3, inclusive" } */ 
> +  __builtin_set_fpscr_rn(4);   /* { dg-error "argument 1 must be a variable or a literal between 0 and 3, inclusive" } */ 
>  }
>
  
Segher Boessenkool Dec. 1, 2021, 11 p.m. UTC | #2
On Thu, Nov 18, 2021 at 10:36:52AM -0600, Bill Schmidt wrote:
> Hi!  This is the last patch broken out of the previous test suite patch
> for the new builtins support.

Whew :-)

> One advantage of the new builtins support is uniform error messages for
> arguments with restricted values.  Previously this was done in many places
> in an ad hoc manner, with little uniformity.  This patch adjusts the
> expected error messages accordingly.
> 
> All such error messages are now one of the following:
>   "argument %d must be a %d-bit unsigned literal"
>   "argument %d must be a literal between %d and %d, inclusive"
>   "argument %d must be a variable or a literal between %d and %d, inclusive"
>   "argument %d must be either a literal %d or a literal %d"
> 
> These messages were chosen to require the fewest changes from previous
> messages while still introducing uniformity.  This patch adjusts error
> messages for some cases where this produces changed messages.  In
> particular, some messages are improved because previously they did not
> admit the possibility that an argument could hold a variable.

Same comment as on the previous patch.  But, okay for trunk.  Thanks!


Segher
  
Pop, Sebastian via Gcc-patches Dec. 2, 2021, 4:43 p.m. UTC | #3
Hi!

On 12/1/21 5:00 PM, Segher Boessenkool wrote:
> On Thu, Nov 18, 2021 at 10:36:52AM -0600, Bill Schmidt wrote:
>> Hi!  This is the last patch broken out of the previous test suite patch
>> for the new builtins support.
> Whew :-)
>
>> One advantage of the new builtins support is uniform error messages for
>> arguments with restricted values.  Previously this was done in many places
>> in an ad hoc manner, with little uniformity.  This patch adjusts the
>> expected error messages accordingly.
>>
>> All such error messages are now one of the following:
>>   "argument %d must be a %d-bit unsigned literal"
>>   "argument %d must be a literal between %d and %d, inclusive"
>>   "argument %d must be a variable or a literal between %d and %d, inclusive"
>>   "argument %d must be either a literal %d or a literal %d"
>>
>> These messages were chosen to require the fewest changes from previous
>> messages while still introducing uniformity.  This patch adjusts error
>> messages for some cases where this produces changed messages.  In
>> particular, some messages are improved because previously they did not
>> admit the possibility that an argument could hold a variable.
> Same comment as on the previous patch.  But, okay for trunk.  Thanks!

Thank you for all of the reviews!  I combined recent patches that need to go
upstream together to avoid bisect problems, and pushed them as r12-5752.
The new built-in infrastructure is now enabled!

I'll start working on a patch series to remove all the no longer needed code.

Thanks again for all of your help in getting this work reviewed, and for
all the improvements as a result!

Bill

>
>
> Segher
  
Segher Boessenkool Dec. 2, 2021, 10:24 p.m. UTC | #4
On Thu, Dec 02, 2021 at 10:43:24AM -0600, Bill Schmidt wrote:
> The new built-in infrastructure is now enabled!

Congratulations, and thanks for all the work!


Segher
  
Peter Bergner Dec. 3, 2021, 11:32 p.m. UTC | #5
On 12/2/21 4:24 PM, Segher Boessenkool wrote:
> On Thu, Dec 02, 2021 at 10:43:24AM -0600, Bill Schmidt wrote:
>> The new built-in infrastructure is now enabled!
> 
> Congratulations, and thanks for all the work!

A big +1!

Peter
  

Patch

diff --git a/gcc/testsuite/gcc.target/powerpc/test_fpscr_drn_builtin_error.c b/gcc/testsuite/gcc.target/powerpc/test_fpscr_drn_builtin_error.c
index 028ab0b6d66..4f9d9e08e8a 100644
--- a/gcc/testsuite/gcc.target/powerpc/test_fpscr_drn_builtin_error.c
+++ b/gcc/testsuite/gcc.target/powerpc/test_fpscr_drn_builtin_error.c
@@ -9,8 +9,8 @@  int main ()
      __builtin_set_fpscr_drn() also support a variable as an argument but
      can't test variable value at compile time.  */
 
-  __builtin_set_fpscr_drn(-1);  /* { dg-error "Argument must be a value between 0 and 7" } */ 
-  __builtin_set_fpscr_drn(8);   /* { dg-error "Argument must be a value between 0 and 7" } */ 
+  __builtin_set_fpscr_drn(-1);  /* { dg-error "argument 1 must be a variable or a literal between 0 and 7, inclusive" } */ 
+  __builtin_set_fpscr_drn(8);   /* { dg-error "argument 1 must be a variable or a literal between 0 and 7, inclusive" } */ 
 
 }
 
diff --git a/gcc/testsuite/gcc.target/powerpc/test_fpscr_rn_builtin_error.c b/gcc/testsuite/gcc.target/powerpc/test_fpscr_rn_builtin_error.c
index aea65091b0c..10391b71008 100644
--- a/gcc/testsuite/gcc.target/powerpc/test_fpscr_rn_builtin_error.c
+++ b/gcc/testsuite/gcc.target/powerpc/test_fpscr_rn_builtin_error.c
@@ -8,13 +8,13 @@  int main ()
      int arguments.  The builtins __builtin_set_fpscr_rn() also supports a
      variable as an argument but can't test variable value at compile time.  */
 
-  __builtin_mtfsb0(-1);  /* { dg-error "Argument must be a constant between 0 and 31" } */
-  __builtin_mtfsb0(32);  /* { dg-error "Argument must be a constant between 0 and 31" } */
+  __builtin_mtfsb0(-1);  /* { dg-error "argument 1 must be a 5-bit unsigned literal" } */
+  __builtin_mtfsb0(32);  /* { dg-error "argument 1 must be a 5-bit unsigned literal" } */
 
-  __builtin_mtfsb1(-1);  /* { dg-error "Argument must be a constant between 0 and 31" } */
-  __builtin_mtfsb1(32);  /* { dg-error "Argument must be a constant between 0 and 31" } */ 
+  __builtin_mtfsb1(-1);  /* { dg-error "argument 1 must be a 5-bit unsigned literal" } */
+  __builtin_mtfsb1(32);  /* { dg-error "argument 1 must be a 5-bit unsigned literal" } */ 
 
-  __builtin_set_fpscr_rn(-1);  /* { dg-error "Argument must be a value between 0 and 3" } */ 
-  __builtin_set_fpscr_rn(4);   /* { dg-error "Argument must be a value between 0 and 3" } */ 
+  __builtin_set_fpscr_rn(-1);  /* { dg-error "argument 1 must be a variable or a literal between 0 and 3, inclusive" } */ 
+  __builtin_set_fpscr_rn(4);   /* { dg-error "argument 1 must be a variable or a literal between 0 and 3, inclusive" } */ 
 }