gprof cleanups

gprof cleanups

[Ben Elliston]

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

I'm trying to make some of the binutils directories less cluttered
with #if 0 code that is clearly obsolete.  I started with gprof, but
in this case, I just converted them to comments.  I've used diff -w to
minimise the size of the patch as there was a bit of reformatting
required.

Committed as obvious.

2005-01-21  Ben Elliston  <bje@au.ibm.com>

	* gmon.h, alpha.c, vax.c: Remove #if 0'd chunks.

Index: gmon.h
===================================================================
RCS file: /home/bje/src-cvs/src/gprof/gmon.h,v
retrieving revision 1.4
diff -u -p -w -r1.4 gmon.h
--- gmon.h	30 Jul 2002 09:07:00 -0000	1.4
+++ gmon.h	20 Jan 2005 23:26:36 -0000
@@ -33,19 +33,20 @@
 #define GMON_HDRSIZE_BSD44_32 (4 + 4 + 4 + 4 + 4 + (3 * 4))
 #define GMON_HDRSIZE_BSD44_64 (8 + 8 + 4 + 4 + 4 + (3 * 4))
 
-#if 0 /* For documentation purposes only.  */
+/* For documentation purposes only.
+
   struct raw_phdr
     {
-      char low_pc[sizeof(void *)]; /* base pc address of sample buffer */
-      char high_pc[sizeof(void *)];/* max pc address of sampled buffer */
-      char ncnt[4];		   /* size of sample buffer (plus this
-				      header) */
-
-      char version[4];		   /* version number */
-      char profrate[4];		   /* profiling clock rate */
-      char spare[3*4];		   /* reserved */
+      char low_pc[sizeof(void *)];   -- base pc address of sample buffer
+      char high_pc[sizeof(void *)];  -- max pc address of sampled buffer
+      char ncnt[4];		     -- size of sample buffer (plus this
+				        header)
+
+      char version[4];		     -- version number
+      char profrate[4];		     -- profiling clock rate
+      char spare[3*4];		     -- reserved
     };
-#endif
+*/
 
 #define GMONVERSION     0x00051879
 
@@ -60,22 +61,22 @@
 #define GMON_HDRSIZE_OLDBSD_64 (8 + 8 + 4)
 #endif
 
-#if 0 /* For documentation purposes only.  */
+/* For documentation purposes only.
+
   struct old_raw_phdr
     {
-      char low_pc[sizeof(void *)]; /* base pc address of sample buffer */
-      char high_pc[sizeof(void *)];/* max pc address of sampled buffer */
-      char ncnt[4];		   /* size of sample buffer (plus this
-				      header) */
-#if defined (__alpha__) && defined (__osf__)
-      /*
-       * DEC's OSF v3.0 uses 4 bytes of padding to bring the header to
-       * a size that is a multiple of 8.
-       */
-      char pad[4];
-#endif
+      char low_pc[sizeof(void *)];  -- base pc address of sample buffer
+      char high_pc[sizeof(void *)]  -- max pc address of sampled buffer
+      char ncnt[4];		    -- size of sample buffer (plus this
+				       header)
+
+      if defined (__alpha__) && defined (__osf__)
+      char pad[4];		    -- DEC's OSF v3.0 uses 4 bytes of padding
+				    -- to bring the header to a size that is a
+				    -- multiple of 8.
+      endif
     };
-#endif
+*/
 
 /*
  * Histogram counters are unsigned shorts:
@@ -130,14 +131,16 @@ struct tostruct
  * as to get a packed representation (otherwise, different compilers
  * might introduce different padding):
  */
-#if 0 /* For documentation purposes only.  */
+
+/* For documentation purposes only.
+
   struct raw_arc
     {
       char from_pc[sizeof(void *)];
       char self_pc[sizeof(void *)];
       char count[sizeof(long)];
     };
-#endif
+*/
 
 /*
  * General rounding functions:
Index: alpha.c
===================================================================
RCS file: /home/bje/src-cvs/src/gprof/alpha.c,v
retrieving revision 1.8
diff -u -p -w -r1.8 alpha.c
--- alpha.c	26 May 2004 04:55:55 -0000	1.8
+++ alpha.c	20 Jan 2005 23:26:36 -0000
@@ -45,9 +45,9 @@
 #define Jxx_FUNC_RET		2
 #define Jxx_FUNC_JSR_COROUTINE	3
 
-#if 0
 /* Here to document only.  We can't use this when cross compiling as
-   the bitfield layout might not be the same as native.  */
+   the bitfield layout might not be the same as native.
+
 typedef union
   {
     struct
@@ -55,14 +55,14 @@ typedef union
 	unsigned other:26;
 	unsigned op_code:6;
       }
-    a;				/* any format */
+       a;				-- any format
     struct
       {
 	int disp:21;
 	unsigned ra:5;
 	unsigned op_code:6;
       }
-    b;				/* branch format */
+       b;				-- branch format
     struct
       {
 	int hint:14;
@@ -71,10 +71,10 @@ typedef union
 	unsigned ra:5;
 	unsigned op_code:6;
       }
-    j;				/* jump format */
+       j;				-- jump format
   }
 alpha_Instruction;
-#endif
+*/
 
 static Sym indirect_child;
 
Index: vax.c
===================================================================
RCS file: /home/bje/src-cvs/src/gprof/vax.c,v
retrieving revision 1.11
diff -u -p -w -r1.11 vax.c
--- vax.c	26 May 2004 04:55:55 -0000	1.11
+++ vax.c	20 Jan 2005 23:26:36 -0000
@@ -53,15 +53,15 @@ enum opermodes
   };
 typedef enum opermodes operandenum;
 
-#if 0
 /* Here to document only.  We can't use this when cross compiling as
-   the bitfield layout might not be the same as native.  */
+   the bitfield layout might not be the same as native.
+
 struct modebyte
   {
     unsigned int regfield:4;
     unsigned int modefield:4;
   };
-#endif
+*/
 
 /*
  * A symbol to be the child of indirect calls:


[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 189 bytes --]

Re: gprof cleanups

[Daniel Jacobowitz]

On Fri, Jan 21, 2005 at 10:29:31AM +1100, Ben Elliston wrote:
> I'm trying to make some of the binutils directories less cluttered
> with #if 0 code that is clearly obsolete.  I started with gprof, but
> in this case, I just converted them to comments.  I've used diff -w to
> minimise the size of the patch as there was a bit of reformatting
> required.
> 
> Committed as obvious.

How about adding INDENT-OFF markers, so that indent doesn't eat the
indentation?

-- 
Daniel Jacobowitz

Re: gprof cleanups

[Ben Elliston]

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

> How about adding INDENT-OFF markers, so that indent doesn't eat the
> indentation?

Sure.  What do they look like? :-)

Ben


[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 189 bytes --]

Re: gprof cleanups

[Daniel Jacobowitz]

On Fri, Jan 21, 2005 at 10:33:39AM +1100, Ben Elliston wrote:
> > How about adding INDENT-OFF markers, so that indent doesn't eat the
> > indentation?
> 
> Sure.  What do they look like? :-)

Beats me :-)  I think /* INDENT-OFF */ and /* INDENT-ON */; grep src
and you should find a bunch of them.

-- 
Daniel Jacobowitz

Re: gprof cleanups

[Ben Elliston]

Ian Lance Taylor <ian@wasabisystems.com> writes:

> I don't understand the point of this loop either before or after your
> patch.  Doesn't it just duplicate work down by the loop just a few
> lines up?
> 
>   for (sp = &default_excluded_list[0]; *sp; sp++)
>     {
>       sym_id_add (*sp, EXCL_TIME);
>       sym_id_add (*sp, EXCL_GRAPH);
>       sym_id_add (*sp, EXCL_FLAT);
>     }

I finally got around to looking at this and I agree with you.  I'll
submit a proper patch next week that removes the duplicated code.

Ben

Re: gprof cleanups

[Ian Lance Taylor]

Ben Elliston <bje@au1.ibm.com> writes:

> The following patch makes a start at cleaning up the gprof sources.
> Okay to commit?

This is OK.

> @@ -520,71 +521,36 @@
>     * functions off the flat profile:
>     */
>    if (line_granularity)
> -    {
> -      for (sp = &default_excluded_list[0]; *sp; sp++)
> -	{
> -	  sym_id_add (*sp, EXCL_FLAT);
> -	}
> -    }
> +    for (sp = &default_excluded_list[0]; *sp; sp++)
> +      sym_id_add (*sp, EXCL_FLAT);

I don't understand the point of this loop either before or after your
patch.  Doesn't it just duplicate work down by the loop just a few
lines up?

  for (sp = &default_excluded_list[0]; *sp; sp++)
    {
      sym_id_add (*sp, EXCL_TIME);
      sym_id_add (*sp, EXCL_GRAPH);
      sym_id_add (*sp, EXCL_FLAT);
    }

Ian

gprof cleanups

[Ben Elliston]

The following patch makes a start at cleaning up the gprof sources.
Okay to commit?

Ben

2004-05-26  Ben Elliston  <bje@au.ibm.com>

	* corefile.c (core_init): Use a separate local variable,
	core_sym_bytes, to make the units from bfd_get_symtab_upper_bound
	more obvious.
	(core_create_function_syms): Discard cbfd argument.  Eliminate
	`offset' variable and calculate VMA directly. Update all users.
	* corefile.h (core_create_function_syms): Update prototype.
	(core_create_line_syms): Likewise.
	* gprof.c (main): Remove #ifdef PROF_SUPPORT_IMPLEMENTED code.
	Tidy.	

Index: corefile.c
===================================================================
RCS file: /cvs/src/src/gprof/corefile.c,v
retrieving revision 1.15
diff -u -r1.15 corefile.c
--- corefile.c	11 Oct 2003 12:34:31 -0000	1.15
+++ corefile.c	26 May 2004 03:28:15 -0000
@@ -142,6 +142,7 @@
 core_init (aout_name)
      const char *aout_name;
 {
+  int core_sym_bytes;
   core_bfd = bfd_openr (aout_name, 0);
 
   if (!core_bfd)
@@ -172,15 +173,15 @@
   /* Read core's symbol table.  */
 
   /* This will probably give us more than we need, but that's ok.  */
-  core_num_syms = bfd_get_symtab_upper_bound (core_bfd);
-  if (core_num_syms < 0)
+  core_sym_bytes = bfd_get_symtab_upper_bound (core_bfd);
+  if (core_sym_bytes < 0)
     {
       fprintf (stderr, "%s: %s: %s\n", whoami, aout_name,
 	       bfd_errmsg (bfd_get_error ()));
       done (1);
     }
 
-  core_syms = (asymbol **) xmalloc (core_num_syms);
+  core_syms = (asymbol **) xmalloc (core_sym_bytes);
   core_num_syms = bfd_canonicalize_symtab (core_bfd, core_syms);
 
   if (core_num_syms < 0)
@@ -405,8 +406,7 @@
    One symbol per function is entered.  */
 
 void
-core_create_function_syms (cbfd)
-     bfd *cbfd ATTRIBUTE_UNUSED;
+core_create_function_syms ()
 {
   bfd_vma min_vma = ~(bfd_vma) 0;
   bfd_vma max_vma = 0;
@@ -592,13 +592,11 @@
    One symbol per line of source code is entered.  */
 
 void
-core_create_line_syms (cbfd)
-     bfd *cbfd;
+core_create_line_syms ()
 {
   char *prev_name, *prev_filename;
   unsigned int prev_name_len, prev_filename_len;
   bfd_vma vma, min_vma = ~(bfd_vma) 0, max_vma = 0;
-  bfd_vma offset;
   Sym *prev, dummy, *sentinel, *sym;
   const char *filename;
   int prev_line_num;
@@ -607,9 +605,9 @@
   /* Create symbols for functions as usual.  This is necessary in
      cases where parts of a program were not compiled with -g.  For
      those parts we still want to get info at the function level.  */
-  core_create_function_syms (cbfd);
+  core_create_function_syms ();
 
-  /* Pass 1 - counter number of symbols.  */
+  /* Pass 1: count the number of symbols.  */
 
   /* To find all line information, walk through all possible
      text-space addresses (one by one!) and get the debugging
@@ -617,7 +615,7 @@
      it is time to create a new symbol.
 
      Of course, this is rather slow and it would be better if
-     bfd would provide an iterator for enumerating all line infos.  */
+     BFD would provide an iterator for enumerating all line infos.  */
   prev_name_len = PATH_MAX;
   prev_filename_len = PATH_MAX;
   prev_name = xmalloc (prev_name_len);
@@ -625,12 +623,11 @@
   ltab.len = 0;
   prev_line_num = 0;
 
-  for (offset = 0; offset < core_text_sect->_raw_size; offset += min_insn_size)
+  bfd_vma vma_high = core_text_sect->vma + core_text_sect->_raw_size;
+  for (vma = core_text_sect->vma; vma < vma_high; vma += min_insn_size)
     {
       unsigned int len;
 
-      vma = core_text_sect->vma + offset;
-
       if (!get_src_info (vma, &filename, &dummy.name, &dummy.line_num)
 	  || (prev_line_num == dummy.line_num
 	      && prev_name != NULL
@@ -693,12 +690,11 @@
      lot cleaner now.  */
   prev = 0;
 
-  for (offset = 0; offset < core_text_sect->_raw_size; offset += min_insn_size)
+  for (vma = core_text_sect->vma; vma < vma_high; vma += min_insn_size)
     {
       sym_init (ltab.limit);
 
-      if (!get_src_info (core_text_sect->vma + offset, &filename,
-			 &ltab.limit->name, &ltab.limit->line_num)
+      if (!get_src_info (vma, &filename, &ltab.limit->name, &ltab.limit->line_num)
 	  || (prev && prev->line_num == ltab.limit->line_num
 	      && strcmp (prev->name, ltab.limit->name) == 0
 	      && strcmp (prev->file->name, filename) == 0))
@@ -708,7 +704,7 @@
       ltab.limit->name = xstrdup (ltab.limit->name);
       ltab.limit->file = source_file_lookup_path (filename);
 
-      ltab.limit->addr = core_text_sect->vma + offset;
+      ltab.limit->addr = vma;
 
       /* Set is_static based on the enclosing function, using either:
 	 1) the previous symbol, if it's from the same function, or
Index: corefile.h
===================================================================
RCS file: /cvs/src/src/gprof/corefile.h,v
retrieving revision 1.6
diff -u -r1.6 corefile.h
--- corefile.h	1 Feb 2002 08:24:16 -0000	1.6
+++ corefile.h	26 May 2004 03:28:15 -0000
@@ -41,7 +41,7 @@
 
 extern void core_init                  PARAMS ((const char *));
 extern void core_get_text_space        PARAMS ((bfd *));
-extern void core_create_function_syms  PARAMS ((bfd *));
-extern void core_create_line_syms      PARAMS ((bfd *));
+extern void core_create_function_syms  PARAMS ((void));
+extern void core_create_line_syms      PARAMS ((void));
 
 #endif /* corefile_h */
Index: gprof.c
===================================================================
RCS file: /cvs/src/src/gprof/gprof.c,v
retrieving revision 1.17
diff -u -r1.17 gprof.c
--- gprof.c	30 Nov 2002 08:39:44 -0000	1.17
+++ gprof.c	26 May 2004 03:28:16 -0000
@@ -483,7 +483,8 @@
       done (1);
     }
 
-  /* --sum implies --line, otherwise we'd lose b-b counts in gmon.sum */
+  /* --sum implies --line, otherwise we'd lose basic block counts in
+       gmon.sum */
   if (output_style & STYLE_SUMMARY_FILE)
     {
       line_granularity = 1;
@@ -520,71 +521,36 @@
    * functions off the flat profile:
    */
   if (line_granularity)
-    {
-      for (sp = &default_excluded_list[0]; *sp; sp++)
-	{
-	  sym_id_add (*sp, EXCL_FLAT);
-	}
-    }
+    for (sp = &default_excluded_list[0]; *sp; sp++)
+      sym_id_add (*sp, EXCL_FLAT);
 
-  /*
-   * Read symbol table from core file:
-   */
+  /* Read symbol table from core file.  */
   core_init (a_out_name);
 
-  /*
-   * If we should ignore direct function calls, we need to load
-   * to core's text-space:
-   */
+  /* If we should ignore direct function calls, we need to load to
+     core's text-space.  */
   if (ignore_direct_calls)
-    {
-      core_get_text_space (core_bfd);
-    }
+    core_get_text_space (core_bfd);
 
-  /*
-   * Create symbols from core image:
-   */
+  /* Create symbols from core image.  */
   if (line_granularity)
-    {
-      core_create_line_syms (core_bfd);
-    }
+    core_create_line_syms ();
   else
-    {
-      core_create_function_syms (core_bfd);
-    }
+    core_create_function_syms ();
 
-  /*
-   * Translate sym specs into syms:
-   */
+  /* Translate sym specs into syms.  */
   sym_id_parse ();
 
   if (file_format == FF_PROF)
     {
-#ifdef PROF_SUPPORT_IMPLEMENTED
-      /*
-       * Get information about mon.out file(s):
-       */
-      do
-	{
-	  mon_out_read (gmon_name);
-	  if (optind < argc)
-	    {
-	      gmon_name = argv[optind];
-	    }
-	}
-      while (optind++ < argc);
-#else
       fprintf (stderr,
 	       _("%s: sorry, file format `prof' is not yet supported\n"),
 	       whoami);
       done (1);
-#endif
     }
   else
     {
-      /*
-       * Get information about gmon.out file(s):
-       */
+      /* Get information about gmon.out file(s).  */
       do
 	{
 	  gmon_out_read (gmon_name);
@@ -603,19 +569,15 @@
   if (output_style == 0)
     {
       if (gmon_input & (INPUT_HISTOGRAM | INPUT_CALL_GRAPH))
-	{
-	  output_style = STYLE_FLAT_PROFILE | STYLE_CALL_GRAPH;
-	}
+	output_style = STYLE_FLAT_PROFILE | STYLE_CALL_GRAPH;
       else
-	{
-	  output_style = STYLE_EXEC_COUNTS;
-	}
+	output_style = STYLE_EXEC_COUNTS;
+
       output_style &= ~user_specified;
     }
 
-  /*
-   * Dump a gmon.sum file if requested (before any other processing!):
-   */
+  /* Dump a gmon.sum file if requested (before any other
+     processing!)  */
   if (output_style & STYLE_SUMMARY_FILE)
     {
       gmon_out_write (GMONSUM);
@@ -631,8 +593,7 @@
       cg = cg_assemble ();
     }
 
-  /* do some simple sanity checks: */
-
+  /* Do some simple sanity checks.  */
   if ((output_style & STYLE_FLAT_PROFILE)
       && !(gmon_input & INPUT_HISTOGRAM))
     {