[PATCH] Change gcc/ira-conflicts.cc build_conflict_bit_table to use size_t/%zu

[PATCH] Change gcc/ira-conflicts.cc build_conflict_bit_table to use size_t/%zu

[Jonathan Yong]

[-- Attachment #1: Type: text/plain, Size: 895 bytes --]

Attached patch OK? Copied inline for review convenience.

diff --git a/gcc/ira-conflicts.cc b/gcc/ira-conflicts.cc
index 671b4e42b6f..e966afe6cdc 100644
--- a/gcc/ira-conflicts.cc
+++ b/gcc/ira-conflicts.cc
@@ -150,9 +150,9 @@ build_conflict_bit_table (void)
    if (internal_flag_ira_verbose > 0 && ira_dump_file != NULL)
      fprintf
        (ira_dump_file,
-       "+++Allocating %ld bytes for conflict table (uncompressed size %ld)\n",
-       (long) allocated_words_num * sizeof (IRA_INT_TYPE),
-       (long) object_set_words * ira_objects_num * sizeof (IRA_INT_TYPE));
+       "+++Allocating %zu bytes for conflict table (uncompressed size %zu)\n",
+       (size_t)(allocated_words_num * sizeof (IRA_INT_TYPE)),
+       (size_t)(object_set_words * ira_objects_num * sizeof (IRA_INT_TYPE)));
  
    objects_live = sparseset_alloc (ira_objects_num);
    for (i = 0; i < ira_max_point; i++)

[-- Attachment #2: 0001-PR-target-43613-use-zu-for-build_conflict_bit_table.patch --]
[-- Type: text/x-patch, Size: 1321 bytes --]

From 48861b8578526ac199b123ff71af8f9778c396c3 Mon Sep 17 00:00:00 2001
From: Jonathan Yong <10walls@gmail.com>
Date: Thu, 1 Feb 2024 12:35:52 +0000
Subject: [PATCH] PR target/43613: use %zu for build_conflict_bit_table

LLP64 platforms like uses 32bit for long and may truncate. Use
size_t and %zu to guarantee 64bit lengths.

gcc:
	*ira-conflicts.cc: use %zu for build_conflict_bit_table.
---
 gcc/ira-conflicts.cc | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/gcc/ira-conflicts.cc b/gcc/ira-conflicts.cc
index 671b4e42b6f..e966afe6cdc 100644
--- a/gcc/ira-conflicts.cc
+++ b/gcc/ira-conflicts.cc
@@ -150,9 +150,9 @@ build_conflict_bit_table (void)
   if (internal_flag_ira_verbose > 0 && ira_dump_file != NULL)
     fprintf
       (ira_dump_file,
-       "+++Allocating %ld bytes for conflict table (uncompressed size %ld)\n",
-       (long) allocated_words_num * sizeof (IRA_INT_TYPE),
-       (long) object_set_words * ira_objects_num * sizeof (IRA_INT_TYPE));
+       "+++Allocating %zu bytes for conflict table (uncompressed size %zu)\n",
+       (size_t)(allocated_words_num * sizeof (IRA_INT_TYPE)),
+       (size_t)(object_set_words * ira_objects_num * sizeof (IRA_INT_TYPE)));
 
   objects_live = sparseset_alloc (ira_objects_num);
   for (i = 0; i < ira_max_point; i++)
-- 
2.43.0

Re: [PATCH] Change gcc/ira-conflicts.cc build_conflict_bit_table to use size_t/%zu

[Jakub Jelinek]

On Thu, Feb 01, 2024 at 12:45:31PM +0000, Jonathan Yong wrote:
> Attached patch OK? Copied inline for review convenience.

No, I think e.g. AIX doesn't support the z modifier.
I don't see %zd or %zu used anywhere except in gcc/jit/ which presumably
doesn't work on AIX.

If you really want to avoid truncation, perhaps do something like
  if (internal_flag_ira_verbose > 0 && ira_dump_file != NULL)
    {
      if (sizeof (void *) <= sizeof (long))
	fprintf (ira_dump_file,
		 "+++Allocating %lu bytes for conflict table "
		 "(uncompressed size %lu)\n",
		 (unsigned long) (sizeof (IRA_INT_TYPE) * allocated_words_num),
		 (unsigned long) (sizeof (IRA_INT_TYPE) * object_set_words
				  * ira_objects_num));
      else
	fprintf (ira_dump_file,
		 "+++Allocating %l" PRIu64 "bytes for conflict table "
		 "(uncompressed size %" PRIu64 ")\n",
		 (unsigned HOST_WIDE_INT) (sizeof (IRA_INT_TYPE)
					   * allocated_words_num),
		 (unsigned HOST_WIDE_INT) (sizeof (IRA_INT_TYPE)
					   * object_set_words
					   * ira_objects_num));
    }

> diff --git a/gcc/ira-conflicts.cc b/gcc/ira-conflicts.cc
> index 671b4e42b6f..e966afe6cdc 100644
> --- a/gcc/ira-conflicts.cc
> +++ b/gcc/ira-conflicts.cc
> @@ -150,9 +150,9 @@ build_conflict_bit_table (void)
>    if (internal_flag_ira_verbose > 0 && ira_dump_file != NULL)
>      fprintf
>        (ira_dump_file,
> -       "+++Allocating %ld bytes for conflict table (uncompressed size %ld)\n",
> -       (long) allocated_words_num * sizeof (IRA_INT_TYPE),
> -       (long) object_set_words * ira_objects_num * sizeof (IRA_INT_TYPE));
> +       "+++Allocating %zu bytes for conflict table (uncompressed size %zu)\n",
> +       (size_t)(allocated_words_num * sizeof (IRA_INT_TYPE)),
> +       (size_t)(object_set_words * ira_objects_num * sizeof (IRA_INT_TYPE)));
>    objects_live = sparseset_alloc (ira_objects_num);
>    for (i = 0; i < ira_max_point; i++)

> From 48861b8578526ac199b123ff71af8f9778c396c3 Mon Sep 17 00:00:00 2001
> From: Jonathan Yong <10walls@gmail.com>
> Date: Thu, 1 Feb 2024 12:35:52 +0000
> Subject: [PATCH] PR target/43613: use %zu for build_conflict_bit_table
> 
> LLP64 platforms like uses 32bit for long and may truncate. Use
> size_t and %zu to guarantee 64bit lengths.
> 
> gcc:
> 	*ira-conflicts.cc: use %zu for build_conflict_bit_table.
> ---
>  gcc/ira-conflicts.cc | 6 +++---
>  1 file changed, 3 insertions(+), 3 deletions(-)
> 
> diff --git a/gcc/ira-conflicts.cc b/gcc/ira-conflicts.cc
> index 671b4e42b6f..e966afe6cdc 100644
> --- a/gcc/ira-conflicts.cc
> +++ b/gcc/ira-conflicts.cc
> @@ -150,9 +150,9 @@ build_conflict_bit_table (void)
>    if (internal_flag_ira_verbose > 0 && ira_dump_file != NULL)
>      fprintf
>        (ira_dump_file,
> -       "+++Allocating %ld bytes for conflict table (uncompressed size %ld)\n",
> -       (long) allocated_words_num * sizeof (IRA_INT_TYPE),
> -       (long) object_set_words * ira_objects_num * sizeof (IRA_INT_TYPE));
> +       "+++Allocating %zu bytes for conflict table (uncompressed size %zu)\n",
> +       (size_t)(allocated_words_num * sizeof (IRA_INT_TYPE)),
> +       (size_t)(object_set_words * ira_objects_num * sizeof (IRA_INT_TYPE)));
>  
>    objects_live = sparseset_alloc (ira_objects_num);
>    for (i = 0; i < ira_max_point; i++)
> -- 
> 2.43.0
> 


	Jakub

Re: [PATCH] Change gcc/ira-conflicts.cc build_conflict_bit_table to use size_t/%zu

[Xi Ruoyao]

On Thu, 2024-02-01 at 14:01 +0100, Jakub Jelinek wrote:
> On Thu, Feb 01, 2024 at 12:45:31PM +0000, Jonathan Yong wrote:
> > Attached patch OK? Copied inline for review convenience.
> 
> No, I think e.g. AIX doesn't support the z modifier.
> I don't see %zd or %zu used anywhere except in gcc/jit/ which presumably
> doesn't work on AIX.
> 
> If you really want to avoid truncation, perhaps do something like
>   if (internal_flag_ira_verbose > 0 && ira_dump_file != NULL)
>     {
>       if (sizeof (void *) <= sizeof (long))
> 	fprintf (ira_dump_file,
> 		 "+++Allocating %lu bytes for conflict table "
> 		 "(uncompressed size %lu)\n",
> 		 (unsigned long) (sizeof (IRA_INT_TYPE) * allocated_words_num),
> 		 (unsigned long) (sizeof (IRA_INT_TYPE) * object_set_words
> 				  * ira_objects_num));
>       else
> 	fprintf (ira_dump_file,
> 		 "+++Allocating %l" PRIu64 "bytes for conflict table "
> 		 "(uncompressed size %" PRIu64 ")\n",

Should use HOST_WIDE_INT_PRINT_UNSIGNED instead of PRIu64.

> 		 (unsigned HOST_WIDE_INT) (sizeof (IRA_INT_TYPE)
> 					   * allocated_words_num),
> 		 (unsigned HOST_WIDE_INT) (sizeof (IRA_INT_TYPE)
> 					   * object_set_words
> 					   * ira_objects_num));
>     }

-- 
Xi Ruoyao <xry111@xry111.site>
School of Aerospace Science and Technology, Xidian University

Re: [PATCH] Change gcc/ira-conflicts.cc build_conflict_bit_table to use size_t/%zu

[Jakub Jelinek]

On Thu, Feb 01, 2024 at 02:01:17PM +0100, Jakub Jelinek wrote:
> On Thu, Feb 01, 2024 at 12:45:31PM +0000, Jonathan Yong wrote:
> > Attached patch OK? Copied inline for review convenience.
> 
> No, I think e.g. AIX doesn't support the z modifier.
> I don't see %zd or %zu used anywhere except in gcc/jit/ which presumably
> doesn't work on AIX.

Or hwint.h could define PRIusize_t etc. macros and some corresponding type
to be used in casts, something like
#if SIZE_MAX == LONG_LONG_MAX
#define PRIusize_t HOST_LONG_LONG_FORMAT "u"
#define fmt_size_t unsigned long long
#elif SIZE_MAX == LONG_MAX
#define PRIusize_t HOST_LONG_FORMAT "u"
#define fmt_size_t unsigned long
#elif SIZE_MAX == INT_MAX
#define PRIusize_t "u"
#define fmt_size_t unsigned int
#else
#error "Unsupported size_t"
#endif

and then use
"... %" PRIusize_t " ... ", ... (fmt_size_t) (some_size_t_expr), ...

Or at configure time figure out which exact type size_t is say (unsigned
long long vs. unsigned long vs. unsigned int) using templates and
use that info to pick the right format modifier depending on that, then
the casts aren't needed.

	Jakub

Re: [PATCH] Change gcc/ira-conflicts.cc build_conflict_bit_table to use size_t/%zu

[Jonathan Yong]

[-- Attachment #1: Type: text/plain, Size: 1380 bytes --]

On 2/1/24 13:06, Xi Ruoyao wrote:
> On Thu, 2024-02-01 at 14:01 +0100, Jakub Jelinek wrote:
>> On Thu, Feb 01, 2024 at 12:45:31PM +0000, Jonathan Yong wrote:
>>> Attached patch OK? Copied inline for review convenience.
>>
>> No, I think e.g. AIX doesn't support the z modifier.
>> I don't see %zd or %zu used anywhere except in gcc/jit/ which presumably
>> doesn't work on AIX.
>>
> 
> Should use HOST_WIDE_INT_PRINT_UNSIGNED instead of PRIu64.
> 
Updated the patch with the suggestions.

diff --git a/gcc/ira-conflicts.cc b/gcc/ira-conflicts.cc
index 671b4e42b6f..32688f9dc39 100644
--- a/gcc/ira-conflicts.cc
+++ b/gcc/ira-conflicts.cc
@@ -150,9 +150,9 @@ build_conflict_bit_table (void)
    if (internal_flag_ira_verbose > 0 && ira_dump_file != NULL)
      fprintf
        (ira_dump_file,
-       "+++Allocating %ld bytes for conflict table (uncompressed size %ld)\n",
-       (long) allocated_words_num * sizeof (IRA_INT_TYPE),
-       (long) object_set_words * ira_objects_num * sizeof (IRA_INT_TYPE));
+       "+++Allocating "HOST_WIDE_INT_PRINT_UNSIGNED" bytes for conflict table (uncompressed size "HOST_WIDE_INT_PRINT_UNSIGNED")\n",
+       (size_t)(allocated_words_num * sizeof (IRA_INT_TYPE)),
+       (size_t)(object_set_words * ira_objects_num * sizeof (IRA_INT_TYPE)));
  
    objects_live = sparseset_alloc (ira_objects_num);
    for (i = 0; i < ira_max_point; i++)

[-- Attachment #2: 0001-PR-target-43613-use-HOST_WIDE_INT_PRINT_UNSIGNED-for.patch --]
[-- Type: text/x-patch, Size: 1450 bytes --]

From 8d3294370404ceda3177bdd8fa7749bfc4d64bc6 Mon Sep 17 00:00:00 2001
From: Jonathan Yong <10walls@gmail.com>
Date: Thu, 1 Feb 2024 12:35:52 +0000
Subject: [PATCH] PR target/43613: use HOST_WIDE_INT_PRINT_UNSIGNED for
 build_conflict_bit_table

LLP64 platforms like uses 32bit for long and may truncate. Use
size_t and HOST_WIDE_INT_PRINT_UNSIGNED to guarantee 64bit lengths.

gcc:
	*ira-conflicts.cc (build_conflict_bit_table):
	use HOST_WIDE_INT_PRINT_UNSIGNED.
---
 gcc/ira-conflicts.cc | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/gcc/ira-conflicts.cc b/gcc/ira-conflicts.cc
index 671b4e42b6f..32688f9dc39 100644
--- a/gcc/ira-conflicts.cc
+++ b/gcc/ira-conflicts.cc
@@ -150,9 +150,9 @@ build_conflict_bit_table (void)
   if (internal_flag_ira_verbose > 0 && ira_dump_file != NULL)
     fprintf
       (ira_dump_file,
-       "+++Allocating %ld bytes for conflict table (uncompressed size %ld)\n",
-       (long) allocated_words_num * sizeof (IRA_INT_TYPE),
-       (long) object_set_words * ira_objects_num * sizeof (IRA_INT_TYPE));
+       "+++Allocating "HOST_WIDE_INT_PRINT_UNSIGNED" bytes for conflict table (uncompressed size "HOST_WIDE_INT_PRINT_UNSIGNED")\n",
+       (size_t)(allocated_words_num * sizeof (IRA_INT_TYPE)),
+       (size_t)(object_set_words * ira_objects_num * sizeof (IRA_INT_TYPE)));
 
   objects_live = sparseset_alloc (ira_objects_num);
   for (i = 0; i < ira_max_point; i++)
-- 
2.43.0

Re: [PATCH] Change gcc/ira-conflicts.cc build_conflict_bit_table to use size_t/%zu

[Jakub Jelinek]

On Thu, Feb 01, 2024 at 01:42:03PM +0000, Jonathan Yong wrote:
> On 2/1/24 13:06, Xi Ruoyao wrote:
> > On Thu, 2024-02-01 at 14:01 +0100, Jakub Jelinek wrote:
> > > On Thu, Feb 01, 2024 at 12:45:31PM +0000, Jonathan Yong wrote:
> > > > Attached patch OK? Copied inline for review convenience.
> > > 
> > > No, I think e.g. AIX doesn't support the z modifier.
> > > I don't see %zd or %zu used anywhere except in gcc/jit/ which presumably
> > > doesn't work on AIX.
> > > 
> > 
> > Should use HOST_WIDE_INT_PRINT_UNSIGNED instead of PRIu64.
> > 
> Updated the patch with the suggestions.

No, that is wrong.  That will break bootstrap on lots of hosts, any time
size_t is not unsigned long (if unsigned long is 64-bit) or unsigned long
long (if unsigned long is not 64-bit).
That includes e.g. all targets where size_t is unsigned int, and some others
too.

	Jakub

Re: [PATCH] Change gcc/ira-conflicts.cc build_conflict_bit_table to use size_t/%zu

[Xi Ruoyao]

On Thu, 2024-02-01 at 14:55 +0100, Jakub Jelinek wrote:
> On Thu, Feb 01, 2024 at 01:42:03PM +0000, Jonathan Yong wrote:
> > On 2/1/24 13:06, Xi Ruoyao wrote:
> > > On Thu, 2024-02-01 at 14:01 +0100, Jakub Jelinek wrote:
> > > > On Thu, Feb 01, 2024 at 12:45:31PM +0000, Jonathan Yong wrote:
> > > > > Attached patch OK? Copied inline for review convenience.
> > > > 
> > > > No, I think e.g. AIX doesn't support the z modifier.
> > > > I don't see %zd or %zu used anywhere except in gcc/jit/ which presumably
> > > > doesn't work on AIX.
> > > > 
> > > 
> > > Should use HOST_WIDE_INT_PRINT_UNSIGNED instead of PRIu64.
> > > 
> > Updated the patch with the suggestions.

I mean if you are casting it to unsigned HOST_WIDE_INT, you should use
HOST_WIDE_INT_PRINT_UNSIGNED,  If you are casting it to size_t you
cannot use it (as Jakub has explained).

When you use printf-like things you have to keep the correspondence
between format specifier and the argument itself, 

> No, that is wrong.  That will break bootstrap on lots of hosts, any time
> size_t is not unsigned long (if unsigned long is 64-bit) or unsigned long
> long (if unsigned long is not 64-bit).
> That includes e.g. all targets where size_t is unsigned int, and some others
> too.
> 
> 	Jakub
> 

-- 
Xi Ruoyao <xry111@xry111.site>
School of Aerospace Science and Technology, Xidian University

Re: [PATCH] Change gcc/ira-conflicts.cc build_conflict_bit_table to use size_t/%zu

[Jakub Jelinek]

On Thu, Feb 01, 2024 at 02:13:11PM +0100, Jakub Jelinek wrote:
> Or hwint.h could define PRIusize_t etc. macros and some corresponding type
> to be used in casts, something like
> #if SIZE_MAX == LONG_LONG_MAX
> #define PRIusize_t HOST_LONG_LONG_FORMAT "u"
> #define fmt_size_t unsigned long long
> #elif SIZE_MAX == LONG_MAX
> #define PRIusize_t HOST_LONG_FORMAT "u"
> #define fmt_size_t unsigned long
> #elif SIZE_MAX == INT_MAX
> #define PRIusize_t "u"
> #define fmt_size_t unsigned int
> #else
> #error "Unsupported size_t"
> #endif
> 
> and then use
> "... %" PRIusize_t " ... ", ... (fmt_size_t) (some_size_t_expr), ...
> 
> Or at configure time figure out which exact type size_t is say (unsigned
> long long vs. unsigned long vs. unsigned int) using templates and
> use that info to pick the right format modifier depending on that, then
> the casts aren't needed.

Note, the former is probably better and might need some fallback, because
on some platforms size_t is some weird type like __uint20 or similar.
So maybe just start with smallest to largest, #if SIZE_MAX <= INT_MAX ...
#elif SIZE_MAX <= LONG_MAX ... #else ... #endif.

	Jakub

Re: [PATCH] Change gcc/ira-conflicts.cc build_conflict_bit_table to use size_t/%zu

[Jonathan Yong]

[-- Attachment #1: Type: text/plain, Size: 1627 bytes --]

On 2/1/24 14:33, Xi Ruoyao wrote:
> 
> I mean if you are casting it to unsigned HOST_WIDE_INT, you should use
> HOST_WIDE_INT_PRINT_UNSIGNED,  If you are casting it to size_t you
> cannot use it (as Jakub has explained).
> 
> When you use printf-like things you have to keep the correspondence
> between format specifier and the argument itself,
> 
>> No, that is wrong.  That will break bootstrap on lots of hosts, any time
>> size_t is not unsigned long (if unsigned long is 64-bit) or unsigned long
>> long (if unsigned long is not 64-bit).
>> That includes e.g. all targets where size_t is unsigned int, and some others
>> too.
>>
>> 	Jakub
>>
> 

Thanks, that makes sense, tested on x86_64-linux.

diff --git a/gcc/ira-conflicts.cc b/gcc/ira-conflicts.cc
index 671b4e42b6f..ff5db18ffcc 100644
--- a/gcc/ira-conflicts.cc
+++ b/gcc/ira-conflicts.cc
@@ -150,9 +150,9 @@ build_conflict_bit_table (void)
    if (internal_flag_ira_verbose > 0 && ira_dump_file != NULL)
      fprintf
        (ira_dump_file,
-       "+++Allocating %ld bytes for conflict table (uncompressed size %ld)\n",
-       (long) allocated_words_num * sizeof (IRA_INT_TYPE),
-       (long) object_set_words * ira_objects_num * sizeof (IRA_INT_TYPE));
+       "+++Allocating "HOST_WIDE_INT_PRINT_UNSIGNED" bytes for conflict table (uncompressed size "HOST_WIDE_INT_PRINT_UNSIGNED")\n",
+       (unsigned HOST_WIDE_INT)(allocated_words_num * sizeof (IRA_INT_TYPE)),
+       (unsigned HOST_WIDE_INT)(object_set_words * ira_objects_num * sizeof (IRA_INT_TYPE)));
  
    objects_live = sparseset_alloc (ira_objects_num);
    for (i = 0; i < ira_max_point; i++)

[-- Attachment #2: 0001-PR-target-43613-use-HOST_WIDE_INT_PRINT_UNSIGNED-for.patch --]
[-- Type: text/x-patch, Size: 1524 bytes --]

From ab44628feca0b200aaaa2d3344af1a96cb1437c0 Mon Sep 17 00:00:00 2001
From: Jonathan Yong <10walls@gmail.com>
Date: Thu, 1 Feb 2024 12:35:52 +0000
Subject: [PATCH] PR target/43613: use HOST_WIDE_INT_PRINT_UNSIGNED for
 build_conflict_bit_table

LLP64 platforms like uses 32bit for long and may truncate. Use
unsigned HOST_WIDE_INT and HOST_WIDE_INT_PRINT_UNSIGNED to guarantee
64bit widths.

gcc:
	*ira-conflicts.cc (build_conflict_bit_table):
	use unsigned HOST_WIDE_INT and HOST_WIDE_INT_PRINT_UNSIGNED.
---
 gcc/ira-conflicts.cc | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/gcc/ira-conflicts.cc b/gcc/ira-conflicts.cc
index 671b4e42b6f..ff5db18ffcc 100644
--- a/gcc/ira-conflicts.cc
+++ b/gcc/ira-conflicts.cc
@@ -150,9 +150,9 @@ build_conflict_bit_table (void)
   if (internal_flag_ira_verbose > 0 && ira_dump_file != NULL)
     fprintf
       (ira_dump_file,
-       "+++Allocating %ld bytes for conflict table (uncompressed size %ld)\n",
-       (long) allocated_words_num * sizeof (IRA_INT_TYPE),
-       (long) object_set_words * ira_objects_num * sizeof (IRA_INT_TYPE));
+       "+++Allocating "HOST_WIDE_INT_PRINT_UNSIGNED" bytes for conflict table (uncompressed size "HOST_WIDE_INT_PRINT_UNSIGNED")\n",
+       (unsigned HOST_WIDE_INT)(allocated_words_num * sizeof (IRA_INT_TYPE)),
+       (unsigned HOST_WIDE_INT)(object_set_words * ira_objects_num * sizeof (IRA_INT_TYPE)));
 
   objects_live = sparseset_alloc (ira_objects_num);
   for (i = 0; i < ira_max_point; i++)
-- 
2.43.0

Re: [PATCH] Change gcc/ira-conflicts.cc build_conflict_bit_table to use size_t/%zu

[Jakub Jelinek]

On Thu, Feb 01, 2024 at 02:53:47PM +0000, Jonathan Yong wrote:
> On 2/1/24 14:33, Xi Ruoyao wrote:
> > 
> > I mean if you are casting it to unsigned HOST_WIDE_INT, you should use
> > HOST_WIDE_INT_PRINT_UNSIGNED,  If you are casting it to size_t you
> > cannot use it (as Jakub has explained).
> > 
> > When you use printf-like things you have to keep the correspondence
> > between format specifier and the argument itself,
> > 
> > > No, that is wrong.  That will break bootstrap on lots of hosts, any time
> > > size_t is not unsigned long (if unsigned long is 64-bit) or unsigned long
> > > long (if unsigned long is not 64-bit).
> > > That includes e.g. all targets where size_t is unsigned int, and some others
> > > too.
> > > 
> > > 	Jakub
> > > 
> > 
> 
> Thanks, that makes sense, tested on x86_64-linux.

No, besides the formatting being incorrect both in ChangeLog and in the
patch, this pessimizes ILP32 hosts unnecessarily.

> diff --git a/gcc/ira-conflicts.cc b/gcc/ira-conflicts.cc
> index 671b4e42b6f..ff5db18ffcc 100644
> --- a/gcc/ira-conflicts.cc
> +++ b/gcc/ira-conflicts.cc
> @@ -150,9 +150,9 @@ build_conflict_bit_table (void)
>    if (internal_flag_ira_verbose > 0 && ira_dump_file != NULL)
>      fprintf
>        (ira_dump_file,
> -       "+++Allocating %ld bytes for conflict table (uncompressed size %ld)\n",
> -       (long) allocated_words_num * sizeof (IRA_INT_TYPE),
> -       (long) object_set_words * ira_objects_num * sizeof (IRA_INT_TYPE));
> +       "+++Allocating "HOST_WIDE_INT_PRINT_UNSIGNED" bytes for conflict table (uncompressed size "HOST_WIDE_INT_PRINT_UNSIGNED")\n",
> +       (unsigned HOST_WIDE_INT)(allocated_words_num * sizeof (IRA_INT_TYPE)),
> +       (unsigned HOST_WIDE_INT)(object_set_words * ira_objects_num * sizeof (IRA_INT_TYPE)));
>    objects_live = sparseset_alloc (ira_objects_num);
>    for (i = 0; i < ira_max_point; i++)

	Jakub

Re: [PATCH] Change gcc/ira-conflicts.cc build_conflict_bit_table to use size_t/%zu

[Jakub Jelinek]

On Thu, Feb 01, 2024 at 03:55:51PM +0100, Jakub Jelinek wrote:
> No, besides the formatting being incorrect both in ChangeLog and in the
> patch, this pessimizes ILP32 hosts unnecessarily.

So like this instead?

2024-02-01  Jakub Jelinek  <jakub@redhat.com>

	* hwint.h (GCC_PRISZ, fmt_size_t, HOST_SIZE_T_PRINT_DEC,
	HOST_SIZE_T_PRINT_UNSIGNED, HOST_SIZE_T_PRINT_HEX,
	HOST_SIZE_T_PRINT_HEX_PURE): Define.
	* ira-conflicts.cc (build_conflict_bit_table): Use it.  Formatting
	fixes.

--- gcc/hwint.h.jj	2024-01-03 11:51:32.676715299 +0100
+++ gcc/hwint.h	2024-02-01 16:22:53.037013522 +0100
@@ -115,6 +115,27 @@ typedef HOST_WIDE_INT __gcc_host_wide_in
 #define HOST_WIDE_INT_PRINT_DOUBLE_HEX "0x%" PRIx64 "%016" PRIx64
 #define HOST_WIDE_INT_PRINT_PADDED_HEX "%016" PRIx64
 
+/* Similarly format modifier for printing size_t.  As not all hosts support
+   z modifier in printf, use GCC_PRISZ and cast argument to fmt_size_t.
+   So, instead of doing fprintf ("%zu\n", sizeof (x) * y); use
+   fprintf (HOST_SIZE_T_PRINT_UNSIGNED "\n",
+	    (fmt_size_t) (sizeof (x) * y));  */
+#if SIZE_MAX <= INT_MAX
+# define GCC_PRISZ ""
+# define fmt_size_t unsigned int
+#elif SIZE_MAX <= LONG_MAX
+# define GCC_PRISZ HOST_LONG_FORMAT
+# define fmt_size_t unsigned long int
+#else
+# define GCC_PRISZ HOST_LONG_LONG_FORMAT
+# define fmt_size_t unsigned long long int
+#endif
+
+#define HOST_SIZE_T_PRINT_DEC "%" GCC_PRISZ "d"
+#define HOST_SIZE_T_PRINT_UNSIGNED "%" GCC_PRISZ "u"
+#define HOST_SIZE_T_PRINT_HEX "%#" GCC_PRISZ "x"
+#define HOST_SIZE_T_PRINT_HEX_PURE "%" GCC_PRISZ "x"
+
 /* Define HOST_WIDEST_FAST_INT to the widest integer type supported
    efficiently in hardware.  (That is, the widest integer type that fits
    in a hardware register.)  Normally this is "long" but on some hosts it
--- gcc/ira-conflicts.cc.jj	2024-01-03 11:51:31.748728178 +0100
+++ gcc/ira-conflicts.cc	2024-02-01 16:12:02.156137005 +0100
@@ -115,10 +115,10 @@ build_conflict_bit_table (void)
 	    > (uint64_t) param_ira_max_conflict_table_size * 1024 * 1024)
 	  {
 	    if (internal_flag_ira_verbose > 0 && ira_dump_file != NULL)
-	      fprintf
-		(ira_dump_file,
-		 "+++Conflict table will be too big(>%dMB) -- don't use it\n",
-		 param_ira_max_conflict_table_size);
+	      fprintf (ira_dump_file,
+		       "+++Conflict table will be too big(>%dMB) "
+		       "-- don't use it\n",
+		       param_ira_max_conflict_table_size);
 	    return false;
 	  }
       }
@@ -148,11 +148,13 @@ build_conflict_bit_table (void)
 
   object_set_words = (ira_objects_num + IRA_INT_BITS - 1) / IRA_INT_BITS;
   if (internal_flag_ira_verbose > 0 && ira_dump_file != NULL)
-    fprintf
-      (ira_dump_file,
-       "+++Allocating %ld bytes for conflict table (uncompressed size %ld)\n",
-       (long) allocated_words_num * sizeof (IRA_INT_TYPE),
-       (long) object_set_words * ira_objects_num * sizeof (IRA_INT_TYPE));
+    fprintf (ira_dump_file,
+	     "+++Allocating " HOST_SIZE_T_PRINT_UNSIGNED
+	     " bytes for conflict table (uncompressed size "
+	     HOST_SIZE_T_PRINT_UNSIGNED ")\n",
+	     (fmt_size_t) (sizeof (IRA_INT_TYPE) * allocated_words_num),
+	     (fmt_size_t) (sizeof (IRA_INT_TYPE) * object_set_words
+			   * ira_objects_num));
 
   objects_live = sparseset_alloc (ira_objects_num);
   for (i = 0; i < ira_max_point; i++)


	Jakub

Re: [PATCH] Change gcc/ira-conflicts.cc build_conflict_bit_table to use size_t/%zu

[Jonathan Yong]

On 2/1/24 15:25, Jakub Jelinek wrote:
> On Thu, Feb 01, 2024 at 03:55:51PM +0100, Jakub Jelinek wrote:
>> No, besides the formatting being incorrect both in ChangeLog and in the
>> patch, this pessimizes ILP32 hosts unnecessarily.
> 
> So like this instead?
> 
> 2024-02-01  Jakub Jelinek  <jakub@redhat.com>
> 
> 	* hwint.h (GCC_PRISZ, fmt_size_t, HOST_SIZE_T_PRINT_DEC,
> 	HOST_SIZE_T_PRINT_UNSIGNED, HOST_SIZE_T_PRINT_HEX,
> 	HOST_SIZE_T_PRINT_HEX_PURE): Define.
> 	* ira-conflicts.cc (build_conflict_bit_table): Use it.  Formatting
> 	fixes.
> 
> --- gcc/hwint.h.jj	2024-01-03 11:51:32.676715299 +0100
> +++ gcc/hwint.h	2024-02-01 16:22:53.037013522 +0100
> @@ -115,6 +115,27 @@ typedef HOST_WIDE_INT __gcc_host_wide_in
>   #define HOST_WIDE_INT_PRINT_DOUBLE_HEX "0x%" PRIx64 "%016" PRIx64
>   #define HOST_WIDE_INT_PRINT_PADDED_HEX "%016" PRIx64
>   
> +/* Similarly format modifier for printing size_t.  As not all hosts support
> +   z modifier in printf, use GCC_PRISZ and cast argument to fmt_size_t.
> +   So, instead of doing fprintf ("%zu\n", sizeof (x) * y); use
> +   fprintf (HOST_SIZE_T_PRINT_UNSIGNED "\n",
> +	    (fmt_size_t) (sizeof (x) * y));  */
> +#if SIZE_MAX <= INT_MAX
> +# define GCC_PRISZ ""
> +# define fmt_size_t unsigned int
> +#elif SIZE_MAX <= LONG_MAX
> +# define GCC_PRISZ HOST_LONG_FORMAT
> +# define fmt_size_t unsigned long int
> +#else
> +# define GCC_PRISZ HOST_LONG_LONG_FORMAT
> +# define fmt_size_t unsigned long long int
> +#endif
> +
> +#define HOST_SIZE_T_PRINT_DEC "%" GCC_PRISZ "d"
> +#define HOST_SIZE_T_PRINT_UNSIGNED "%" GCC_PRISZ "u"
> +#define HOST_SIZE_T_PRINT_HEX "%#" GCC_PRISZ "x"
> +#define HOST_SIZE_T_PRINT_HEX_PURE "%" GCC_PRISZ "x"
> +

...

> -      (ira_dump_file,
> -       "+++Allocating %ld bytes for conflict table (uncompressed size %ld)\n",
> -       (long) allocated_words_num * sizeof (IRA_INT_TYPE),
> -       (long) object_set_words * ira_objects_num * sizeof (IRA_INT_TYPE));
> +    fprintf (ira_dump_file,
> +	     "+++Allocating " HOST_SIZE_T_PRINT_UNSIGNED
> +	     " bytes for conflict table (uncompressed size "
> +	     HOST_SIZE_T_PRINT_UNSIGNED ")\n",
> +	     (fmt_size_t) (sizeof (IRA_INT_TYPE) * allocated_words_num),
> +	     (fmt_size_t) (sizeof (IRA_INT_TYPE) * object_set_words
> +			   * ira_objects_num));
>   
>     objects_live = sparseset_alloc (ira_objects_num);
>     for (i = 0; i < ira_max_point; i++)
> 
> 
> 	Jakub
> 

Looks good for ILP32/LLP64.

Re: [PATCH] Change gcc/ira-conflicts.cc build_conflict_bit_table to use size_t/%zu

[Jonathan Yong]

On 2/1/24 15:33, Jonathan Yong wrote:
> On 2/1/24 15:25, Jakub Jelinek wrote:
>> On Thu, Feb 01, 2024 at 03:55:51PM +0100, Jakub Jelinek wrote:
>>> No, besides the formatting being incorrect both in ChangeLog and in the
>>> patch, this pessimizes ILP32 hosts unnecessarily.
>>
>> So like this instead?
>>
>> 2024-02-01  Jakub Jelinek  <jakub@redhat.com>
>>
>>     * hwint.h (GCC_PRISZ, fmt_size_t, HOST_SIZE_T_PRINT_DEC,
>>     HOST_SIZE_T_PRINT_UNSIGNED, HOST_SIZE_T_PRINT_HEX,
>>     HOST_SIZE_T_PRINT_HEX_PURE): Define.
>>     * ira-conflicts.cc (build_conflict_bit_table): Use it.  Formatting
>>     fixes.
>>
> 
> Looks good for ILP32/LLP64.
> 

Are you planning to push yet?

Re: [PATCH] Change gcc/ira-conflicts.cc build_conflict_bit_table to use size_t/%zu

[Jakub Jelinek]

On Fri, Feb 02, 2024 at 11:43:21PM +0000, Jonathan Yong wrote:
> On 2/1/24 15:33, Jonathan Yong wrote:
> > On 2/1/24 15:25, Jakub Jelinek wrote:
> > > On Thu, Feb 01, 2024 at 03:55:51PM +0100, Jakub Jelinek wrote:
> > > > No, besides the formatting being incorrect both in ChangeLog and in the
> > > > patch, this pessimizes ILP32 hosts unnecessarily.
> > > 
> > > So like this instead?
> > > 
> > > 2024-02-01  Jakub Jelinek  <jakub@redhat.com>
> > > 
> > >     * hwint.h (GCC_PRISZ, fmt_size_t, HOST_SIZE_T_PRINT_DEC,
> > >     HOST_SIZE_T_PRINT_UNSIGNED, HOST_SIZE_T_PRINT_HEX,
> > >     HOST_SIZE_T_PRINT_HEX_PURE): Define.
> > >     * ira-conflicts.cc (build_conflict_bit_table): Use it.  Formatting
> > >     fixes.
> > > 
> > 
> > Looks good for ILP32/LLP64.
> 
> Are you planning to push yet?

It needs to be reviewed first.

Passed successfully bootstrap/regtest on x86_64-linux and i686-linux.

Note, I think incrementally we should search for similar issues,
grep -C3 '%l[duxXi]' *.cc */*.cc | grep 'long)' | grep -v long.long | wc -l
101
is an upper bound on what should be looked at, I see at least a few dozens
from those (e.g. when the argument uses sizeof, or is say hash table
size () or elements () etc.).

Also, wonder if the pretty-printer.cc should get 'z' and 't' modifiers
added, for those we clearly can use 'z' or 't' when we implement it
ourselves.  But at least that part is stage1 material I'd say.
In translated messages we certainly can't use macros like
HOST_SIZE_T_PRINT_*, because it is then untranslatable.

	Jakub

Re: [PATCH] Change gcc/ira-conflicts.cc build_conflict_bit_table to use size_t/%zu

[Richard Biener]

On Thu, Feb 1, 2024 at 4:26 PM Jakub Jelinek <jakub@redhat.com> wrote:
>
> On Thu, Feb 01, 2024 at 03:55:51PM +0100, Jakub Jelinek wrote:
> > No, besides the formatting being incorrect both in ChangeLog and in the
> > patch, this pessimizes ILP32 hosts unnecessarily.
>
> So like this instead?

OK.

Thanks,
Richard.

> 2024-02-01  Jakub Jelinek  <jakub@redhat.com>
>
>         * hwint.h (GCC_PRISZ, fmt_size_t, HOST_SIZE_T_PRINT_DEC,
>         HOST_SIZE_T_PRINT_UNSIGNED, HOST_SIZE_T_PRINT_HEX,
>         HOST_SIZE_T_PRINT_HEX_PURE): Define.
>         * ira-conflicts.cc (build_conflict_bit_table): Use it.  Formatting
>         fixes.
>
> --- gcc/hwint.h.jj      2024-01-03 11:51:32.676715299 +0100
> +++ gcc/hwint.h 2024-02-01 16:22:53.037013522 +0100
> @@ -115,6 +115,27 @@ typedef HOST_WIDE_INT __gcc_host_wide_in
>  #define HOST_WIDE_INT_PRINT_DOUBLE_HEX "0x%" PRIx64 "%016" PRIx64
>  #define HOST_WIDE_INT_PRINT_PADDED_HEX "%016" PRIx64
>
> +/* Similarly format modifier for printing size_t.  As not all hosts support
> +   z modifier in printf, use GCC_PRISZ and cast argument to fmt_size_t.
> +   So, instead of doing fprintf ("%zu\n", sizeof (x) * y); use
> +   fprintf (HOST_SIZE_T_PRINT_UNSIGNED "\n",
> +           (fmt_size_t) (sizeof (x) * y));  */
> +#if SIZE_MAX <= INT_MAX
> +# define GCC_PRISZ ""
> +# define fmt_size_t unsigned int
> +#elif SIZE_MAX <= LONG_MAX
> +# define GCC_PRISZ HOST_LONG_FORMAT
> +# define fmt_size_t unsigned long int
> +#else
> +# define GCC_PRISZ HOST_LONG_LONG_FORMAT
> +# define fmt_size_t unsigned long long int
> +#endif
> +
> +#define HOST_SIZE_T_PRINT_DEC "%" GCC_PRISZ "d"
> +#define HOST_SIZE_T_PRINT_UNSIGNED "%" GCC_PRISZ "u"
> +#define HOST_SIZE_T_PRINT_HEX "%#" GCC_PRISZ "x"
> +#define HOST_SIZE_T_PRINT_HEX_PURE "%" GCC_PRISZ "x"
> +
>  /* Define HOST_WIDEST_FAST_INT to the widest integer type supported
>     efficiently in hardware.  (That is, the widest integer type that fits
>     in a hardware register.)  Normally this is "long" but on some hosts it
> --- gcc/ira-conflicts.cc.jj     2024-01-03 11:51:31.748728178 +0100
> +++ gcc/ira-conflicts.cc        2024-02-01 16:12:02.156137005 +0100
> @@ -115,10 +115,10 @@ build_conflict_bit_table (void)
>             > (uint64_t) param_ira_max_conflict_table_size * 1024 * 1024)
>           {
>             if (internal_flag_ira_verbose > 0 && ira_dump_file != NULL)
> -             fprintf
> -               (ira_dump_file,
> -                "+++Conflict table will be too big(>%dMB) -- don't use it\n",
> -                param_ira_max_conflict_table_size);
> +             fprintf (ira_dump_file,
> +                      "+++Conflict table will be too big(>%dMB) "
> +                      "-- don't use it\n",
> +                      param_ira_max_conflict_table_size);
>             return false;
>           }
>        }
> @@ -148,11 +148,13 @@ build_conflict_bit_table (void)
>
>    object_set_words = (ira_objects_num + IRA_INT_BITS - 1) / IRA_INT_BITS;
>    if (internal_flag_ira_verbose > 0 && ira_dump_file != NULL)
> -    fprintf
> -      (ira_dump_file,
> -       "+++Allocating %ld bytes for conflict table (uncompressed size %ld)\n",
> -       (long) allocated_words_num * sizeof (IRA_INT_TYPE),
> -       (long) object_set_words * ira_objects_num * sizeof (IRA_INT_TYPE));
> +    fprintf (ira_dump_file,
> +            "+++Allocating " HOST_SIZE_T_PRINT_UNSIGNED
> +            " bytes for conflict table (uncompressed size "
> +            HOST_SIZE_T_PRINT_UNSIGNED ")\n",
> +            (fmt_size_t) (sizeof (IRA_INT_TYPE) * allocated_words_num),
> +            (fmt_size_t) (sizeof (IRA_INT_TYPE) * object_set_words
> +                          * ira_objects_num));
>
>    objects_live = sparseset_alloc (ira_objects_num);
>    for (i = 0; i < ira_max_point; i++)
>
>
>         Jakub
>