diff mbox series

gprof: fix disappearing inlined functions

Message ID 20260528022134.133959-2-rsaxvc@gmail.com
State New
Headers
Series gprof: fix disappearing inlined functions |

Checks

Context Check Description
linaro-tcwg-bot/tcwg_binutils_build--master-arm success Build passed
linaro-tcwg-bot/tcwg_binutils_build--master-aarch64 success Build passed
linaro-tcwg-bot/tcwg_binutils_check--master-arm success Test passed
linaro-tcwg-bot/tcwg_binutils_check--master-aarch64 success Test passed

Commit Message

Richard Allen May 28, 2026, 2:21 a.m. UTC 
  Load the function names from line/debug-symbols if available.

The following test spends most of its CPU cycles mathing the
long way around in xorshift_reverse(), but prior to this patch
gprof would show 100% CPU spent in main().

  #include <inttypes.h>
  #include <stdint.h>
  #include <stdio.h>

  static uint32_t xorshift_forward(uint32_t x) {
      x ^= x << 13;
      x ^= x >> 17;
      x ^= x << 5;
      return x;
  }

  static uint32_t xorshift_reverse(uint32_t x){
    for(uint32_t i = 1; i < 4294967295; ++i){
      x ^= x << 13;
      x ^= x >> 17;
      x ^= x << 5;
    }
    return x;
  }

  int main(){
    uint32_t x = 1;
    uint32_t x_minus = xorshift_reverse(x);
    uint32_t x_minus_plus = xorshift_forward(x_minus);
    printf("x:%" PRIu32 " x-:%" PRIu32 " x-+:%" PRIu32 "\n", x, x_minus, x_minus_plus);
    return 0;
  }



---
 gprof/corefile.c | 2 +-
 gprof/gprof.c    | 4 +---
 2 files changed, 2 insertions(+), 4 deletions(-)

Comments

H.J. Lu May 28, 2026, 2:29 a.m. UTC | #1 
On Thu, May 28, 2026 at 10:26 AM Richard Allen <rsaxvc@gmail.com> wrote:
>
> Load the function names from line/debug-symbols if available.
>
> The following test spends most of its CPU cycles mathing the
> long way around in xorshift_reverse(), but prior to this patch
> gprof would show 100% CPU spent in main().
>
>   #include <inttypes.h>
>   #include <stdint.h>
>   #include <stdio.h>
>
>   static uint32_t xorshift_forward(uint32_t x) {
>       x ^= x << 13;
>       x ^= x >> 17;
>       x ^= x << 5;
>       return x;
>   }
>
>   static uint32_t xorshift_reverse(uint32_t x){
>     for(uint32_t i = 1; i < 4294967295; ++i){
>       x ^= x << 13;
>       x ^= x >> 17;
>       x ^= x << 5;
>     }
>     return x;
>   }
>
>   int main(){
>     uint32_t x = 1;
>     uint32_t x_minus = xorshift_reverse(x);
>     uint32_t x_minus_plus = xorshift_forward(x_minus);
>     printf("x:%" PRIu32 " x-:%" PRIu32 " x-+:%" PRIu32 "\n", x, x_minus, x_minus_plus);
>     return 0;
>   }
>

Is it possible to add it to gprof test?

>
> ---
>  gprof/corefile.c | 2 +-
>  gprof/gprof.c    | 4 +---
>  2 files changed, 2 insertions(+), 4 deletions(-)
>
> diff --git a/gprof/corefile.c b/gprof/corefile.c
> index 712dd243e90..b79da05e7b2 100644
> --- a/gprof/corefile.c
> +++ b/gprof/corefile.c
> @@ -881,7 +881,7 @@ core_create_line_syms (void)
>
>             if (!get_src_info (vma, &filename, &ltab.limit->name,
>                                &ltab.limit->line_num)
> -               || (prev.name && prev.line_num == ltab.limit->line_num
> +               || (prev.name && (!line_granularity || prev.line_num == ltab.limit->line_num)
>                     && strcmp (prev.name, ltab.limit->name) == 0
>                     && filename_cmp (prev.file->name, filename) == 0))
>               continue;
> diff --git a/gprof/gprof.c b/gprof/gprof.c
> index cea269bc80f..926581fe17e 100644
> --- a/gprof/gprof.c
> +++ b/gprof/gprof.c
> @@ -641,10 +641,8 @@ symtab_init (void)
>    /* Create symbols from core image.  */
>    if (external_symbol_table)
>      core_create_syms_from (external_symbol_table);
> -  else if (line_granularity)
> -    core_create_line_syms ();
>    else
> -    core_create_function_syms ();
> +    core_create_line_syms ();
>
>    /* Translate sym specs into syms.  */
>    sym_id_parse ();
> --
> 2.47.3
>
Richard Allen May 28, 2026, 3:18 a.m. UTC | #2 
> Is it possible to add it to gprof test?

I think so. I'll try.
Richard Allen June 4, 2026, 9:14 p.m. UTC | #3 
There's a little more too this - I also need to handle optimizing compilers
interleaving instructions between inclined functions. Currently seems to
create multiple symbols with the same function name when that happens.

For testing, would it be ok to check in a small executable and gmon?
Otherwise we'd need to rerun it which will vary the results.
H.J. Lu June 4, 2026, 11:03 p.m. UTC | #4 
On Fri, Jun 5, 2026, 5:14 AM Richard Allen <rsaxvc@gmail.com> wrote:

> There's a little more too this - I also need to handle optimizing
> compilers interleaving instructions between inclined functions. Currently
> seems to create multiple symbols with the same function name when that
> happens.
>
> For testing, would it be ok to check in a small executable and gmon?
> Otherwise we'd need to rerun it which will vary the results.
>

We need a test which fails without your patch.
diff mbox series

Patch

diff --git a/gprof/corefile.c b/gprof/corefile.c
index 712dd243e90..b79da05e7b2 100644
--- a/gprof/corefile.c
+++ b/gprof/corefile.c
@@ -881,7 +881,7 @@  core_create_line_syms (void)
 
 	    if (!get_src_info (vma, &filename, &ltab.limit->name,
 			       &ltab.limit->line_num)
-		|| (prev.name && prev.line_num == ltab.limit->line_num
+		|| (prev.name && (!line_granularity || prev.line_num == ltab.limit->line_num)
 		    && strcmp (prev.name, ltab.limit->name) == 0
 		    && filename_cmp (prev.file->name, filename) == 0))
 	      continue;
diff --git a/gprof/gprof.c b/gprof/gprof.c
index cea269bc80f..926581fe17e 100644
--- a/gprof/gprof.c
+++ b/gprof/gprof.c
@@ -641,10 +641,8 @@  symtab_init (void)
   /* Create symbols from core image.  */
   if (external_symbol_table)
     core_create_syms_from (external_symbol_table);
-  else if (line_granularity)
-    core_create_line_syms ();
   else
-    core_create_function_syms ();
+    core_create_line_syms ();
 
   /* Translate sym specs into syms.  */
   sym_id_parse ();