gas: drop sprint_value()

Message ID a4f9927a-68b7-172b-a838-d1b8c310693d@suse.com
State New
Headers show
Series
  • gas: drop sprint_value()
Related show

Commit Message

H.J. Lu via Binutils April 16, 2021, 7:12 a.m.
Its (documented) behavior is unhelpful in particular in 64-bit build
environments: While printing large 32-bit numbers in decimal already
isn't very meaningful to most people, this even more so goes for yet
larger 64-bit numbers. bfd_sprintf_vma() still tries to limit the number
of digits printed (without depending on a build system property), but
uniformly produces hex output.

gas/
2021-04-XX  Jan Beulich  <jbeulich@suse.com>

	* as.h (sprint_value): Delete.
	* messages.c (sprint_value): Likewise.
	* config/tc-i386.c (offset_in_range): Use bfd_sprintf_vma in
	place of sprint_value.
	* config/tc-s390.c (s390_insert_operand): Likewise.
	* doc/internals.texi (sprint_value): Delete section.
	* write.c (fixup_segment): Likewise.
	(relax_segment): Likewise.

Comments

H.J. Lu via Binutils April 16, 2021, 7:16 a.m. | #1
Andreas,

On 16.04.2021 09:12, Jan Beulich via Binutils wrote:
> Its (documented) behavior is unhelpful in particular in 64-bit build

> environments: While printing large 32-bit numbers in decimal already

> isn't very meaningful to most people, this even more so goes for yet

> larger 64-bit numbers. bfd_sprintf_vma() still tries to limit the number

> of digits printed (without depending on a build system property), but

> uniformly produces hex output.

> 

> gas/

> 2021-04-XX  Jan Beulich  <jbeulich@suse.com>

> 

> 	* as.h (sprint_value): Delete.

> 	* messages.c (sprint_value): Likewise.

> 	* config/tc-i386.c (offset_in_range): Use bfd_sprintf_vma in

> 	place of sprint_value.

> 	* config/tc-s390.c (s390_insert_operand): Likewise.

> 	* doc/internals.texi (sprint_value): Delete section.

> 	* write.c (fixup_segment): Likewise.

> 	(relax_segment): Likewise.


the original mail bounced for the Cc to Martin. Is he no longer with
IBM (and hence the MAINTAINERS entry is stale), or was this some mail
system error (albeit then I wouldn't have expected "550 5.1.1 User
Unknown")?

Thanks, Jan

> --- a/gas/as.h

> +++ b/gas/as.h

> @@ -428,7 +428,6 @@ PRINTF_WHERE_LIKE (as_warn_where);

>  

>  void   as_abort (const char *, int, const char *) ATTRIBUTE_NORETURN;

>  void   signal_init (void);

> -void   sprint_value (char *, addressT);

>  int    had_errors (void);

>  int    had_warnings (void);

>  void   as_warn_value_out_of_range (const char *, offsetT, offsetT, offsetT,

> --- a/gas/config/tc-i386.c

> +++ b/gas/config/tc-i386.c

> @@ -2556,8 +2556,8 @@ offset_in_range (offsetT val, int size)

>      {

>        char buf1[40], buf2[40];

>  

> -      sprint_value (buf1, val);

> -      sprint_value (buf2, val & mask);

> +      bfd_sprintf_vma (stdoutput, buf1, val);

> +      bfd_sprintf_vma (stdoutput, buf2, val & mask);

>        as_warn (_("%s shortened to %s"), buf1, buf2);

>      }

>    return val & mask;

> --- a/gas/config/tc-s390.c

> +++ b/gas/config/tc-s390.c

> @@ -626,7 +626,7 @@ s390_insert_operand (unsigned char *insn

>  	      min <<= 1;

>  	      max <<= 1;

>  	    }

> -	  sprint_value (buf, val);

> +	  bfd_sprintf_vma (stdoutput, buf, val);

>  	  if (file == (char *) NULL)

>  	    as_bad (err, buf, (int) min, (int) max);

>  	  else

> --- a/gas/doc/internals.texi

> +++ b/gas/doc/internals.texi

> @@ -1918,13 +1918,6 @@ after all input has been read, but messa

>  original filename and line number that they are applicable to.

>  @end deftypefun

>  

> -@deftypefun @{@} void sprint_value (char *@var{buf}, valueT @var{val})

> -This function is helpful for converting a @code{valueT} value into printable

> -format, in case it's wider than modes that @code{*printf} can handle.  If the

> -type is narrow enough, a decimal number will be produced; otherwise, it will be

> -in hexadecimal.  The value itself is not examined to make this determination.

> -@end deftypefun

> -

>  @node Test suite

>  @section Test suite

>  @cindex test suite

> --- a/gas/messages.c

> +++ b/gas/messages.c

> @@ -356,22 +356,6 @@ signal_init (void)

>  

>  /* Support routines.  */

>  

> -void

> -sprint_value (char *buf, valueT val)

> -{

> -  if (sizeof (val) <= sizeof (long))

> -    {

> -      sprintf (buf, "%ld", (long) val);

> -      return;

> -    }

> -  if (sizeof (val) <= sizeof (bfd_vma))

> -    {

> -      sprintf_vma (buf, val);

> -      return;

> -    }

> -  abort ();

> -}

> -

>  #define HEX_MAX_THRESHOLD	1024

>  #define HEX_MIN_THRESHOLD	-(HEX_MAX_THRESHOLD)

>  

> --- a/gas/write.c

> +++ b/gas/write.c

> @@ -1110,9 +1110,9 @@ fixup_segment (fixS *fixP, segT this_seg

>  	      if ((add_number & mask) != 0 && (add_number & mask) != mask)

>  		{

>  		  char buf[50], buf2[50];

> -		  sprint_value (buf, fragP->fr_address + fixP->fx_where);

> +		  bfd_sprintf_vma (stdoutput, buf, fragP->fr_address + fixP->fx_where);

>  		  if (add_number > 1000)

> -		    sprint_value (buf2, add_number);

> +		    bfd_sprintf_vma (stdoutput, buf2, add_number);

>  		  else

>  		    sprintf (buf2, "%ld", (long) add_number);

>  		  as_bad_where (fixP->fx_file, fixP->fx_line,

> @@ -2866,7 +2866,9 @@ relax_segment (struct frag *segment_frag

>  			  if (flag_warn_displacement)

>  			    {

>  			      char buf[50];

> -			      sprint_value (buf, (addressT) lie->addnum);

> +

> +			      bfd_sprintf_vma (stdoutput, buf,

> +					       (addressT) lie->addnum);

>  			      as_warn_where (fragP->fr_file, fragP->fr_line,

>  					     _(".word %s-%s+%s didn't fit"),

>  					     S_GET_NAME (lie->add),

>
H.J. Lu via Binutils April 16, 2021, 6:46 p.m. | #2
On 4/16/21 9:16 AM, Jan Beulich wrote:
> Andreas,

> 

> On 16.04.2021 09:12, Jan Beulich via Binutils wrote:

>> Its (documented) behavior is unhelpful in particular in 64-bit build

>> environments: While printing large 32-bit numbers in decimal already

>> isn't very meaningful to most people, this even more so goes for yet

>> larger 64-bit numbers. bfd_sprintf_vma() still tries to limit the number

>> of digits printed (without depending on a build system property), but

>> uniformly produces hex output.

>>

>> gas/

>> 2021-04-XX  Jan Beulich  <jbeulich@suse.com>

>>

>> 	* as.h (sprint_value): Delete.

>> 	* messages.c (sprint_value): Likewise.

>> 	* config/tc-i386.c (offset_in_range): Use bfd_sprintf_vma in

>> 	place of sprint_value.

>> 	* config/tc-s390.c (s390_insert_operand): Likewise.

>> 	* doc/internals.texi (sprint_value): Delete section.

>> 	* write.c (fixup_segment): Likewise.

>> 	(relax_segment): Likewise.

> 

> the original mail bounced for the Cc to Martin. Is he no longer with

> IBM (and hence the MAINTAINERS entry is stale), or was this some mail

> system error (albeit then I wouldn't have expected "550 5.1.1 User

> Unknown")?

> 


Jan,

https://lwn.net/ml/linux-kernel/20190521162350.GA17107@osiris/

It is probably time to remove Martin from the list of active maintainers. I didn't want to be the
one doing it. But I'll submit a patch doing the change to avoid such confusions.

Since Martin used to be a significant contributor to Binutils and the original author of the S/390
port I would like to move his name to the "Past Maintainers" section.

Andreas

> Thanks, Jan

> 

>> --- a/gas/as.h

>> +++ b/gas/as.h

>> @@ -428,7 +428,6 @@ PRINTF_WHERE_LIKE (as_warn_where);

>>  

>>  void   as_abort (const char *, int, const char *) ATTRIBUTE_NORETURN;

>>  void   signal_init (void);

>> -void   sprint_value (char *, addressT);

>>  int    had_errors (void);

>>  int    had_warnings (void);

>>  void   as_warn_value_out_of_range (const char *, offsetT, offsetT, offsetT,

>> --- a/gas/config/tc-i386.c

>> +++ b/gas/config/tc-i386.c

>> @@ -2556,8 +2556,8 @@ offset_in_range (offsetT val, int size)

>>      {

>>        char buf1[40], buf2[40];

>>  

>> -      sprint_value (buf1, val);

>> -      sprint_value (buf2, val & mask);

>> +      bfd_sprintf_vma (stdoutput, buf1, val);

>> +      bfd_sprintf_vma (stdoutput, buf2, val & mask);

>>        as_warn (_("%s shortened to %s"), buf1, buf2);

>>      }

>>    return val & mask;

>> --- a/gas/config/tc-s390.c

>> +++ b/gas/config/tc-s390.c

>> @@ -626,7 +626,7 @@ s390_insert_operand (unsigned char *insn

>>  	      min <<= 1;

>>  	      max <<= 1;

>>  	    }

>> -	  sprint_value (buf, val);

>> +	  bfd_sprintf_vma (stdoutput, buf, val);

>>  	  if (file == (char *) NULL)

>>  	    as_bad (err, buf, (int) min, (int) max);

>>  	  else

>> --- a/gas/doc/internals.texi

>> +++ b/gas/doc/internals.texi

>> @@ -1918,13 +1918,6 @@ after all input has been read, but messa

>>  original filename and line number that they are applicable to.

>>  @end deftypefun

>>  

>> -@deftypefun @{@} void sprint_value (char *@var{buf}, valueT @var{val})

>> -This function is helpful for converting a @code{valueT} value into printable

>> -format, in case it's wider than modes that @code{*printf} can handle.  If the

>> -type is narrow enough, a decimal number will be produced; otherwise, it will be

>> -in hexadecimal.  The value itself is not examined to make this determination.

>> -@end deftypefun

>> -

>>  @node Test suite

>>  @section Test suite

>>  @cindex test suite

>> --- a/gas/messages.c

>> +++ b/gas/messages.c

>> @@ -356,22 +356,6 @@ signal_init (void)

>>  

>>  /* Support routines.  */

>>  

>> -void

>> -sprint_value (char *buf, valueT val)

>> -{

>> -  if (sizeof (val) <= sizeof (long))

>> -    {

>> -      sprintf (buf, "%ld", (long) val);

>> -      return;

>> -    }

>> -  if (sizeof (val) <= sizeof (bfd_vma))

>> -    {

>> -      sprintf_vma (buf, val);

>> -      return;

>> -    }

>> -  abort ();

>> -}

>> -

>>  #define HEX_MAX_THRESHOLD	1024

>>  #define HEX_MIN_THRESHOLD	-(HEX_MAX_THRESHOLD)

>>  

>> --- a/gas/write.c

>> +++ b/gas/write.c

>> @@ -1110,9 +1110,9 @@ fixup_segment (fixS *fixP, segT this_seg

>>  	      if ((add_number & mask) != 0 && (add_number & mask) != mask)

>>  		{

>>  		  char buf[50], buf2[50];

>> -		  sprint_value (buf, fragP->fr_address + fixP->fx_where);

>> +		  bfd_sprintf_vma (stdoutput, buf, fragP->fr_address + fixP->fx_where);

>>  		  if (add_number > 1000)

>> -		    sprint_value (buf2, add_number);

>> +		    bfd_sprintf_vma (stdoutput, buf2, add_number);

>>  		  else

>>  		    sprintf (buf2, "%ld", (long) add_number);

>>  		  as_bad_where (fixP->fx_file, fixP->fx_line,

>> @@ -2866,7 +2866,9 @@ relax_segment (struct frag *segment_frag

>>  			  if (flag_warn_displacement)

>>  			    {

>>  			      char buf[50];

>> -			      sprint_value (buf, (addressT) lie->addnum);

>> +

>> +			      bfd_sprintf_vma (stdoutput, buf,

>> +					       (addressT) lie->addnum);

>>  			      as_warn_where (fragP->fr_file, fragP->fr_line,

>>  					     _(".word %s-%s+%s didn't fit"),

>>  					     S_GET_NAME (lie->add),

>>

>
H.J. Lu via Binutils April 16, 2021, 7:22 p.m. | #3
On 4/16/21 9:12 AM, Jan Beulich wrote:
> Its (documented) behavior is unhelpful in particular in 64-bit build

> environments: While printing large 32-bit numbers in decimal already

> isn't very meaningful to most people, this even more so goes for yet

> larger 64-bit numbers. bfd_sprintf_vma() still tries to limit the number

> of digits printed (without depending on a build system property), but

> uniformly produces hex output.

> 

> gas/

> 2021-04-XX  Jan Beulich  <jbeulich@suse.com>

> 

> 	* as.h (sprint_value): Delete.

> 	* messages.c (sprint_value): Likewise.

> 	* config/tc-i386.c (offset_in_range): Use bfd_sprintf_vma in

> 	place of sprint_value.

> 	* config/tc-s390.c (s390_insert_operand): Likewise.

> 	* doc/internals.texi (sprint_value): Delete section.

> 	* write.c (fixup_segment): Likewise.

> 	(relax_segment): Likewise.


Ok for s390. Thanks!

Andreas

> 

> --- a/gas/as.h

> +++ b/gas/as.h

> @@ -428,7 +428,6 @@ PRINTF_WHERE_LIKE (as_warn_where);

> 

>  void   as_abort (const char *, int, const char *) ATTRIBUTE_NORETURN;

>  void   signal_init (void);

> -void   sprint_value (char *, addressT);

>  int    had_errors (void);

>  int    had_warnings (void);

>  void   as_warn_value_out_of_range (const char *, offsetT, offsetT, offsetT,

> --- a/gas/config/tc-i386.c

> +++ b/gas/config/tc-i386.c

> @@ -2556,8 +2556,8 @@ offset_in_range (offsetT val, int size)

>      {

>        char buf1[40], buf2[40];

> 

> -      sprint_value (buf1, val);

> -      sprint_value (buf2, val & mask);

> +      bfd_sprintf_vma (stdoutput, buf1, val);

> +      bfd_sprintf_vma (stdoutput, buf2, val & mask);

>        as_warn (_("%s shortened to %s"), buf1, buf2);

>      }

>    return val & mask;

> --- a/gas/config/tc-s390.c

> +++ b/gas/config/tc-s390.c

> @@ -626,7 +626,7 @@ s390_insert_operand (unsigned char *insn

>  	      min <<= 1;

>  	      max <<= 1;

>  	    }

> -	  sprint_value (buf, val);

> +	  bfd_sprintf_vma (stdoutput, buf, val);

>  	  if (file == (char *) NULL)

>  	    as_bad (err, buf, (int) min, (int) max);

>  	  else

> --- a/gas/doc/internals.texi

> +++ b/gas/doc/internals.texi

> @@ -1918,13 +1918,6 @@ after all input has been read, but messa

>  original filename and line number that they are applicable to.

>  @end deftypefun

> 

> -@deftypefun @{@} void sprint_value (char *@var{buf}, valueT @var{val})

> -This function is helpful for converting a @code{valueT} value into printable

> -format, in case it's wider than modes that @code{*printf} can handle.  If the

> -type is narrow enough, a decimal number will be produced; otherwise, it will be

> -in hexadecimal.  The value itself is not examined to make this determination.

> -@end deftypefun

> -

>  @node Test suite

>  @section Test suite

>  @cindex test suite

> --- a/gas/messages.c

> +++ b/gas/messages.c

> @@ -356,22 +356,6 @@ signal_init (void)

> 

>  /* Support routines.  */

> 

> -void

> -sprint_value (char *buf, valueT val)

> -{

> -  if (sizeof (val) <= sizeof (long))

> -    {

> -      sprintf (buf, "%ld", (long) val);

> -      return;

> -    }

> -  if (sizeof (val) <= sizeof (bfd_vma))

> -    {

> -      sprintf_vma (buf, val);

> -      return;

> -    }

> -  abort ();

> -}

> -

>  #define HEX_MAX_THRESHOLD	1024

>  #define HEX_MIN_THRESHOLD	-(HEX_MAX_THRESHOLD)

> 

> --- a/gas/write.c

> +++ b/gas/write.c

> @@ -1110,9 +1110,9 @@ fixup_segment (fixS *fixP, segT this_seg

>  	      if ((add_number & mask) != 0 && (add_number & mask) != mask)

>  		{

>  		  char buf[50], buf2[50];

> -		  sprint_value (buf, fragP->fr_address + fixP->fx_where);

> +		  bfd_sprintf_vma (stdoutput, buf, fragP->fr_address + fixP->fx_where);

>  		  if (add_number > 1000)

> -		    sprint_value (buf2, add_number);

> +		    bfd_sprintf_vma (stdoutput, buf2, add_number);

>  		  else

>  		    sprintf (buf2, "%ld", (long) add_number);

>  		  as_bad_where (fixP->fx_file, fixP->fx_line,

> @@ -2866,7 +2866,9 @@ relax_segment (struct frag *segment_frag

>  			  if (flag_warn_displacement)

>  			    {

>  			      char buf[50];

> -			      sprint_value (buf, (addressT) lie->addnum);

> +

> +			      bfd_sprintf_vma (stdoutput, buf,

> +					       (addressT) lie->addnum);

>  			      as_warn_where (fragP->fr_file, fragP->fr_line,

>  					     _(".word %s-%s+%s didn't fit"),

>  					     S_GET_NAME (lie->add),

>
H.J. Lu via Binutils April 17, 2021, 12:33 p.m. | #4
On Fri, Apr 16, 2021 at 09:12:45AM +0200, Jan Beulich via Binutils wrote:
> Its (documented) behavior is unhelpful in particular in 64-bit build

> environments: While printing large 32-bit numbers in decimal already

> isn't very meaningful to most people, this even more so goes for yet

> larger 64-bit numbers. bfd_sprintf_vma() still tries to limit the number

> of digits printed (without depending on a build system property), but

> uniformly produces hex output.

> 

> gas/

> 2021-04-XX  Jan Beulich  <jbeulich@suse.com>

> 

> 	* as.h (sprint_value): Delete.

> 	* messages.c (sprint_value): Likewise.

> 	* config/tc-i386.c (offset_in_range): Use bfd_sprintf_vma in

> 	place of sprint_value.

> 	* config/tc-s390.c (s390_insert_operand): Likewise.

> 	* doc/internals.texi (sprint_value): Delete section.

> 	* write.c (fixup_segment): Likewise.

> 	(relax_segment): Likewise.


OK.

-- 
Alan Modra
Australia Development Lab, IBM

Patch

--- a/gas/as.h
+++ b/gas/as.h
@@ -428,7 +428,6 @@  PRINTF_WHERE_LIKE (as_warn_where);
 
 void   as_abort (const char *, int, const char *) ATTRIBUTE_NORETURN;
 void   signal_init (void);
-void   sprint_value (char *, addressT);
 int    had_errors (void);
 int    had_warnings (void);
 void   as_warn_value_out_of_range (const char *, offsetT, offsetT, offsetT,
--- a/gas/config/tc-i386.c
+++ b/gas/config/tc-i386.c
@@ -2556,8 +2556,8 @@  offset_in_range (offsetT val, int size)
     {
       char buf1[40], buf2[40];
 
-      sprint_value (buf1, val);
-      sprint_value (buf2, val & mask);
+      bfd_sprintf_vma (stdoutput, buf1, val);
+      bfd_sprintf_vma (stdoutput, buf2, val & mask);
       as_warn (_("%s shortened to %s"), buf1, buf2);
     }
   return val & mask;
--- a/gas/config/tc-s390.c
+++ b/gas/config/tc-s390.c
@@ -626,7 +626,7 @@  s390_insert_operand (unsigned char *insn
 	      min <<= 1;
 	      max <<= 1;
 	    }
-	  sprint_value (buf, val);
+	  bfd_sprintf_vma (stdoutput, buf, val);
 	  if (file == (char *) NULL)
 	    as_bad (err, buf, (int) min, (int) max);
 	  else
--- a/gas/doc/internals.texi
+++ b/gas/doc/internals.texi
@@ -1918,13 +1918,6 @@  after all input has been read, but messa
 original filename and line number that they are applicable to.
 @end deftypefun
 
-@deftypefun @{@} void sprint_value (char *@var{buf}, valueT @var{val})
-This function is helpful for converting a @code{valueT} value into printable
-format, in case it's wider than modes that @code{*printf} can handle.  If the
-type is narrow enough, a decimal number will be produced; otherwise, it will be
-in hexadecimal.  The value itself is not examined to make this determination.
-@end deftypefun
-
 @node Test suite
 @section Test suite
 @cindex test suite
--- a/gas/messages.c
+++ b/gas/messages.c
@@ -356,22 +356,6 @@  signal_init (void)
 
 /* Support routines.  */
 
-void
-sprint_value (char *buf, valueT val)
-{
-  if (sizeof (val) <= sizeof (long))
-    {
-      sprintf (buf, "%ld", (long) val);
-      return;
-    }
-  if (sizeof (val) <= sizeof (bfd_vma))
-    {
-      sprintf_vma (buf, val);
-      return;
-    }
-  abort ();
-}
-
 #define HEX_MAX_THRESHOLD	1024
 #define HEX_MIN_THRESHOLD	-(HEX_MAX_THRESHOLD)
 
--- a/gas/write.c
+++ b/gas/write.c
@@ -1110,9 +1110,9 @@  fixup_segment (fixS *fixP, segT this_seg
 	      if ((add_number & mask) != 0 && (add_number & mask) != mask)
 		{
 		  char buf[50], buf2[50];
-		  sprint_value (buf, fragP->fr_address + fixP->fx_where);
+		  bfd_sprintf_vma (stdoutput, buf, fragP->fr_address + fixP->fx_where);
 		  if (add_number > 1000)
-		    sprint_value (buf2, add_number);
+		    bfd_sprintf_vma (stdoutput, buf2, add_number);
 		  else
 		    sprintf (buf2, "%ld", (long) add_number);
 		  as_bad_where (fixP->fx_file, fixP->fx_line,
@@ -2866,7 +2866,9 @@  relax_segment (struct frag *segment_frag
 			  if (flag_warn_displacement)
 			    {
 			      char buf[50];
-			      sprint_value (buf, (addressT) lie->addnum);
+
+			      bfd_sprintf_vma (stdoutput, buf,
+					       (addressT) lie->addnum);
 			      as_warn_where (fragP->fr_file, fragP->fr_line,
 					     _(".word %s-%s+%s didn't fit"),
 					     S_GET_NAME (lie->add),