Patchwork PR 51094 - fprint_w() in output_addr_const() reinstated

login
register
mail settings
Submitter Dimitrios Apostolou
Date July 9, 2012, 7:54 p.m.
Message ID <alpine.LNX.2.02.1207092233490.4288@localhost.localdomain>
Download mbox | patch
Permalink /patch/169948/
State New
Headers show

Comments

Dimitrios Apostolou - July 9, 2012, 7:54 p.m.
Since output_addr_const() shows pretty hot in the compiler, I reinstated 
the fprint_w() call in place of fprintf().

This patch relies on two things: 2's complement representation for 
negative int and that HOST_WIDE_INT is at least as large type as long for 
all platforms (will it always be?).

Bootstrapped/tested on i386, regtested on x86_64 multilib, 
i386-pc-solaris2.10 (thanks ro), i686-darwin9 (thanks iains).


2012-07-09 Dimitrios Apostolou <jimis@gmx.net>

         * final.c, output.h (fprint_w): New function to write a
         HOST_WIDE_INT to a file, fast.
         * final.c (output_addr_const): Use fprint_w() instead of
         fprintf().
         (sprint_ul_rev): New static function to write a
         HOST_WIDE_INT to a string in reverse, fast.



Thanks,
Dimitris
Hans-Peter Nilsson - July 12, 2012, 1:23 a.m.
On Mon, 9 Jul 2012, Dimitrios Apostolou wrote:
> Since output_addr_const() shows pretty hot in the compiler, I reinstated the
> fprint_w() call in place of fprintf().
>
> This patch relies on two things: 2's complement representation for negative
> int and that HOST_WIDE_INT is at least as large type as long for all platforms
> (will it always be?).
>
> Bootstrapped/tested on i386, regtested on x86_64 multilib, i386-pc-solaris2.10
> (thanks ro), i686-darwin9 (thanks iains).
>
>
> 2012-07-09 Dimitrios Apostolou <jimis@gmx.net>
>
>         * final.c, output.h (fprint_w): New function to write a
>         HOST_WIDE_INT to a file, fast.
>         * final.c (output_addr_const): Use fprint_w() instead of
>         fprintf().
>         (sprint_ul_rev): New static function to write a
>         HOST_WIDE_INT to a string in reverse, fast.

(Non-inlined patch, so I can't quote the patch in pine.)

Non-approver review: looks ok besides a few cases of comment
formatting nits: punctuation at end and two spaces: ".  */"
You had it right in the first sentence.  (The original wasn't
right either.)

Maybe also add a comment about how much faster than fprintf this
is, besides the ", fast".

brgds, H-P
Mike Stump - July 12, 2012, 1:44 a.m.
On Jul 9, 2012, at 12:54 PM, Dimitrios Apostolou wrote:
> Since output_addr_const() shows pretty hot in the compiler, I reinstated the fprint_w() call in place of fprintf().

My review bits...  First there is no guarantee that HOST_WIDE_INT_BITSIZE is 64 or less, so [20] is unsafe longer term.  You can add an assert that it is 64 or less, that way it will be fixed when people bump things up.  long != HOST_WIDE_INT, so, that's a type violation, an assert for it would make it safer.  For performance, can't help but wonder if it would be faster to process the numbers in blocks of 9 digits, so that the wide divisions are fewer are farther between and for each one, you could then have 9 32-bit divisions which should be a bit cheaper; but, that's an issue for sprint_ul_rev, not your code.
Andreas Schwab - July 12, 2012, 7:35 a.m.
Dimitrios Apostolou <jimis@gmx.net> writes:

> @@ -3849,6 +3850,32 @@ sprint_ul_rev (char *s, unsigned long va
>    return i;
>  }
>  
> +/* Write a signed HOST_WIDE_INT as decimal to a file, fast. */
> +
> +void
> +fprint_w (FILE *f, HOST_WIDE_INT value)
> +{
> +  /* python says: len(str(2**64)) == 20 */
> +  char s[20];

From gnulib:

/* Bound on length of the string representing an unsigned integer
   value representable in B bits.  log10 (2.0) < 146/485.  The
   smallest value of B where this bound is not tight is 2621.  */
#define INT_BITS_STRLEN_BOUND(b) (((b) * 146 + 484) / 485)

Andreas.

Patch

=== modified file 'gcc/final.c'
--- gcc/final.c	2012-06-24 17:58:46 +0000

+++ gcc/final.c	2012-07-09 04:17:48 +0000

@@ -3693,7 +3693,7 @@  output_addr_const (FILE *file, rtx x)

       break;
 
     case CONST_INT:
-      fprintf (file, HOST_WIDE_INT_PRINT_DEC, INTVAL (x));

+      fprint_w (file, INTVAL (x));

       break;
 
     case CONST:
@@ -3827,11 +3827,12 @@  fprint_whex (FILE *f, unsigned HOST_WIDE

     }
 }
 
-/* Internal function that prints an unsigned long in decimal in reverse.

-   The output string IS NOT null-terminated. */

+/* Internal function that writes to a string an unsigned HOST_WIDE_INT or an

+   unsigned long in decimal in reverse.  The output string IS NOT

+   null-terminated. */

 
 static int
-sprint_ul_rev (char *s, unsigned long value)

+sprint_ul_rev (char *s, unsigned HOST_WIDE_INT value)

 {
   int i = 0;
   do
@@ -3849,6 +3850,32 @@  sprint_ul_rev (char *s, unsigned long va

   return i;
 }
 
+/* Write a signed HOST_WIDE_INT as decimal to a file, fast. */

+

+void

+fprint_w (FILE *f, HOST_WIDE_INT value)

+{

+  /* python says: len(str(2**64)) == 20 */

+  char s[20];

+  int i;

+

+  if (value >= 0)

+    i = sprint_ul_rev (s, value);

+  else

+    {

+      i = sprint_ul_rev (s, (unsigned HOST_WIDE_INT) (~value) + 1);

+      putc('-', f);

+    }

+

+  /* It's probably too small so don't bother with string reversal and fputs */

+  do

+    {

+      i--;

+      putc (s[i], f);

+    }

+  while (i != 0);

+}

+

 /* Write an unsigned long as decimal to a file, fast. */
 
 void

=== modified file 'gcc/output.h'
--- gcc/output.h	2012-06-24 17:58:46 +0000

+++ gcc/output.h	2012-07-08 09:24:59 +0000

@@ -125,6 +125,7 @@  extern void output_addr_const (FILE *, r

 #define ATTRIBUTE_ASM_FPRINTF(m, n) ATTRIBUTE_NONNULL(m)
 #endif
 
+extern void fprint_w (FILE *, HOST_WIDE_INT);

 extern void fprint_whex (FILE *, unsigned HOST_WIDE_INT);
 extern void fprint_ul (FILE *, unsigned long);
 extern int sprint_ul (char *, unsigned long);