Re: [PATCH 05/14] lib/vsprintf.c: help gcc make number() smaller

[Andy Shevchenko]

On Mon, Nov 23, 2015 at 11:29 PM, Rasmus Villemoes
<linux@xxxxxxxxxxxxxxxxxx> wrote:
> One consequence of the reorganization of struct printf_spec to make
> field_width 24 bits was that number() gained about 180 bytes. Since
> spec is never passed to other functions, we can help gcc make number()
> lose most of that extra weight by using local variables for the field
> width and precision.
>

Reviewed-by: Andy Shevchenko <andy.shevchenko@xxxxxxxxx>

One minor nitpick below

> Signed-off-by: Rasmus Villemoes <linux@xxxxxxxxxxxxxxxxxx>
> ---
>  lib/vsprintf.c | 26 ++++++++++++++------------
>  1 file changed, 14 insertions(+), 12 deletions(-)
>
> diff --git a/lib/vsprintf.c b/lib/vsprintf.c
> index 01c3aa638582..d7e27c54fa00 100644
> --- a/lib/vsprintf.c
> +++ b/lib/vsprintf.c
> @@ -399,6 +399,8 @@ char *number(char *buf, char *end, unsigned long long num,
>         int need_pfx = ((spec.flags & SPECIAL) && spec.base != 10);
>         int i;
>         bool is_zero = num == 0LL;
> +       int field_width = spec.field_width;
> +       int precision = spec.precision;

I would put these before int i; But your choice.

>
>         /* locase = 0 or 0x20. ORing digits or letters with 'locase'
>          * produces same digits or (maybe lowercased) letters */
> @@ -410,20 +412,20 @@ char *number(char *buf, char *end, unsigned long long num,
>                 if ((signed long long)num < 0) {
>                         sign = '-';
>                         num = -(signed long long)num;
> -                       spec.field_width--;
> +                       field_width--;
>                 } else if (spec.flags & PLUS) {
>                         sign = '+';
> -                       spec.field_width--;
> +                       field_width--;
>                 } else if (spec.flags & SPACE) {
>                         sign = ' ';
> -                       spec.field_width--;
> +                       field_width--;
>                 }
>         }
>         if (need_pfx) {
>                 if (spec.base == 16)
> -                       spec.field_width -= 2;
> +                       field_width -= 2;
>                 else if (!is_zero)
> -                       spec.field_width--;
> +                       field_width--;
>         }
>
>         /* generate full string in tmp[], in reverse order */
> @@ -445,12 +447,12 @@ char *number(char *buf, char *end, unsigned long long num,
>         }
>
>         /* printing 100 using %2d gives "100", not "00" */
> -       if (i > spec.precision)
> -               spec.precision = i;
> +       if (i > precision)
> +               precision = i;
>         /* leading space padding */
> -       spec.field_width -= spec.precision;
> +       field_width -= precision;
>         if (!(spec.flags & (ZEROPAD | LEFT))) {
> -               while (--spec.field_width >= 0) {
> +               while (--field_width >= 0) {
>                         if (buf < end)
>                                 *buf = ' ';
>                         ++buf;
> @@ -479,14 +481,14 @@ char *number(char *buf, char *end, unsigned long long num,
>         if (!(spec.flags & LEFT)) {
>                 char c = ' ' + (spec.flags & ZEROPAD);
>                 BUILD_BUG_ON(' ' + ZEROPAD != '0');
> -               while (--spec.field_width >= 0) {
> +               while (--field_width >= 0) {
>                         if (buf < end)
>                                 *buf = c;
>                         ++buf;
>                 }
>         }
>         /* hmm even more zero padding? */
> -       while (i <= --spec.precision) {
> +       while (i <= --precision) {
>                 if (buf < end)
>                         *buf = '0';
>                 ++buf;
> @@ -498,7 +500,7 @@ char *number(char *buf, char *end, unsigned long long num,
>                 ++buf;
>         }
>         /* trailing space padding */
> -       while (--spec.field_width >= 0) {
> +       while (--field_width >= 0) {
>                 if (buf < end)
>                         *buf = ' ';
>                 ++buf;
> --
> 2.6.1
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> the body of a message to majordomo@xxxxxxxxxxxxxxx
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at  http://www.tux.org/lkml/



-- 
With Best Regards,
Andy Shevchenko
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/