On 5/22/19 11:40 AM, Jakub Jelinek wrote:
> On Wed, May 22, 2019 at 10:07:33AM +0200, Martin Liška wrote:
>> On 5/22/19 9:50 AM, Jakub Jelinek wrote:
>>> On Wed, May 22, 2019 at 09:39:52AM +0200, Martin Liška wrote:
>>>> The patch is about using of uintptr_t instead unsigned long that's
>>>> being used for printing value of a pointer.
>>>>
>>>> Patch can bootstrap on x86_64-linux-gnu and survives regression tests.
>>>>
>>>> Ready to be installed?
>>>
>>> No. You haven't adjusted the fprintf format strings for this change,
>>> so it will break build on any target where uintptr_t is not unsigned long.
>>>
>>> I don't see any gcc/ code using the PRIxPTR and similar macros, so no idea
>>> about the portability of that, but I'd think we just should keep that as is.
>>
>> Ok.
>
> If the problem is just warnings, perhaps you could do
> (unsigned long) (uintptr_t) casts instead of directly to (unsigned long),
> ideally only for the options with pointer types.
I'm sending patch for it.
> Seems we have:
> else if (otype ~ "^const char \\**$")
> var_opt_string[n_opt_string++] = name;
> else
> var_opt_other[n_opt_other++] = name;
> and so the
> print " fputs (\"\\n\", file);";
> for (i = 0; i < n_opt_other; i++) {
> print " if (ptr1->x_" var_opt_other[i] " != ptr2->x_"
> var_opt_other[i] ")";
> print " fprintf (file, \"%*s%s (%#lx/%#lx)\\n\",";
> print " indent_to, \"\",";
> print " \"" var_opt_other[i] "\",";
> print " (unsigned long)ptr1->x_" var_opt_other[i] ",";
> print " (unsigned long)ptr2->x_" var_opt_other[i] ");";
> print "";
> }
> printing isn't done for strings, we use in that case
> for (i = 0; i < n_opt_string; i++) {
> name = var_opt_string[i]
> print " if (ptr1->x_" name " != ptr2->x_" name "";
> print " || (!ptr1->x_" name" || !ptr2->x_" name
> print " || strcmp (ptr1->x_" name", ptr2->x_" name ")))";
> print " fprintf (file, \"%*s%s (%s/%s)\\n\",";
> print " indent_to, \"\",";
> print " \"" name "\",";
> print " ptr1->x_" name ",";
> print " ptr2->x_" name ");";
> print "";
> }
> instead. So, the only problem is in:
> print " fputs (\"\\n\", file);";
> for (i = 0; i < n_target_other; i++) {
> print " if (ptr->x_" var_target_other[i] ")";
> hwi = host_wide_int[var_target_other[i]]
> if (hwi == "yes")
> print " fprintf (file, \"%*s%s (%#\" HOST_WIDE_INT_PRINT
> \"x)\\n\",";
> else
> print " fprintf (file, \"%*s%s (%#lx)\\n\",";
> print " indent, \"\",";
> print " \"" var_target_other[i] "\",";
> if (hwi == "yes")
> print " ptr->x_" var_target_other[i] ");";
> else
> print " (unsigned long)ptr->x_"
> var_target_other[i] ");";
> print "";
> }
> and another similar block with ptr1/ptr2 later. So, I wonder if you just
> shouldn't follow what is done for var_opt_string vs. var_opt_other even in
> the var_target_other case, do:
> + else if (otype ~ "^const char \\**$")
> + var_target_string[n_target_string++] = name;
> else
> var_target_other[n_target_other++] = name;
> + initialization etc. and handle var_target_string differently from
> var_target_other for the printing (print it actually with %s instead of
> %#lx).
No, you want to print a value, which can be an integer type, or a pointer
to an array. So you don't want to use %s.
Martin
>
> Jakub
>
>From a8e5bd9cd741cba2841c13e5120ded3669e396a4 Mon Sep 17 00:00:00 2001
From: Martin Liska <[email protected]>
Date: Wed, 22 May 2019 13:17:03 +0200
Subject: [PATCH] Patch candidate.
---
gcc/optc-save-gen.awk | 12 ++++++------
1 file changed, 6 insertions(+), 6 deletions(-)
diff --git a/gcc/optc-save-gen.awk b/gcc/optc-save-gen.awk
index 7ecd1eb9cc7..de590c682a8 100644
--- a/gcc/optc-save-gen.awk
+++ b/gcc/optc-save-gen.awk
@@ -213,7 +213,7 @@ for (i = 0; i < n_opt_other; i++) {
print " fprintf (file, \"%*s%s (%#lx)\\n\",";
print " indent_to, \"\",";
print " \"" var_opt_other[i] "\",";
- print " (unsigned long)ptr->x_" var_opt_other[i] ");";
+ print " (unsigned long)(intptr_t)ptr->x_" var_opt_other[i] ");";
print "";
}
@@ -278,8 +278,8 @@ for (i = 0; i < n_opt_other; i++) {
print " fprintf (file, \"%*s%s (%#lx/%#lx)\\n\",";
print " indent_to, \"\",";
print " \"" var_opt_other[i] "\",";
- print " (unsigned long)ptr1->x_" var_opt_other[i] ",";
- print " (unsigned long)ptr2->x_" var_opt_other[i] ");";
+ print " (unsigned long)(intptr_t)ptr1->x_" var_opt_other[i] ",";
+ print " (unsigned long)(intptr_t)ptr2->x_" var_opt_other[i] ");";
print "";
}
@@ -490,7 +490,7 @@ for (i = 0; i < n_target_other; i++) {
if (hwi == "yes")
print " ptr->x_" var_target_other[i] ");";
else
- print " (unsigned long)ptr->x_" var_target_other[i] ");";
+ print " (unsigned long)(intptr_t)ptr->x_" var_target_other[i] ");";
print "";
}
@@ -559,8 +559,8 @@ for (i = 0; i < n_target_other; i++) {
print " ptr2->x_" var_target_other[i] ");";
}
else {
- print " (unsigned long)ptr1->x_" var_target_other[i] ",";
- print " (unsigned long)ptr2->x_" var_target_other[i] ");";
+ print " (unsigned long)(intptr_t)ptr1->x_" var_target_other[i] ",";
+ print " (unsigned long)(intptr_t)ptr2->x_" var_target_other[i] ");";
}
print "";
}
--
2.21.0
