This is the mail archive of the gcc-patches@gcc.gnu.org mailing list for the GCC project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

Re: [PATCH] Introduce instance discriminators


On Jul 19, 2018, Richard Biener <richard.guenther@gmail.com> wrote:

> Oh, that probably wasn't omitted on purpose.  Cary said it was used
> for profiling but I can't see any such use.

> Is the instance discriminator stuff also used for profiling?

Not that I know, but...  I probably wouldn't know yet ;-)

Anyway, it was easy enough to implement this:

>> I suspect there might be a way to assign instance discriminator numbers
>> to individual function DECLs, and then walk up the lexical block tree to
>> identify the DECL containing the block so as to obtain the discriminator
>> number.

and then, in a subsequent patch, I went ahead and added support for LTO,
saving and recovering discriminator info for instances and, while at
that, for basic blocks too.

Besides sucessfully regstrapping the first two patches on
x86_64-linux-gnu, I have tested this patchset with an additional
bootstrap with the third (throw-away) patch, that adds -gnateS to Ada
compilations in gcc/ada, libada and gnattools.  I also tested the saving
and restoring of discriminators for LTO by manually inspecting the line
number tables in LTO-recompiled executables, to check that they retained
the instance or BB discriminator numbers that went into the non-LTO
object files.

Ok to install the first two patches?  (the third is just for reference)


Introduce instance discriminators

From: Alexandre Oliva <oliva@adacore.com>

With -gnateS, the Ada compiler sets itself up to output discriminators
for different instantiations of generics, but the middle and back ends
have lacked support for that.  This patch introduces the missing bits,
translating the GNAT-internal representation of the per-file instance
map to an instance_table that maps decls to instance discriminators.


From: Alexandre Oliva  <oliva@adacore.com>, Olivier Hainque  <hainque@adacore.com>
for  gcc/ChangeLog

	* debug.h (decl_to_instance_map_t): New type.
	(decl_to_instance_map): Declare.
	(maybe_create_decl_to_instance_map): New inline function.
    	* final.c (bb_discriminator, last_bb_discriminator): New statics,
    	to track basic block discriminators.
    	(final_start_function_1): Initialize them.
    	(final_scan_insn_1): On NOTE_INSN_BASIC_BLOCK, track
	bb_discriminator.
	(decl_to_instance_map): New variable.
	(map_decl_to_instance, maybe_set_discriminator): New functions.
    	(notice_source_line): Set discriminator.

for  gcc/ada

	* trans.c: Include debug.h.
	(file_map): New static variable.
	(gigi): Set it.  Create decl_to_instance_map when needed.
	(Subprogram_Body_to_gnu): Pass gnu_subprog_decl to...
	(Sloc_to_locus): ... this.  Add decl parm, map it to instance.
	* gigi.h (Sloc_to_locus): Adjust declaration.

for  gcc/testsuite/ChangeLog

	* gnat.dg/dinst.adb: New.
	* gnat.dg/dinst_pkg.ads, gnat.dg/dinst_pkg.adb: New.
---
 gcc/ada/gcc-interface/gigi.h        |    2 +
 gcc/ada/gcc-interface/trans.c       |   29 ++++++++++++---
 gcc/debug.h                         |   15 ++++++++
 gcc/final.c                         |   70 +++++++++++++++++++++++++++++++++--
 gcc/testsuite/gnat.dg/dinst.adb     |   20 ++++++++++
 gcc/testsuite/gnat.dg/dinst_pkg.adb |    7 ++++
 gcc/testsuite/gnat.dg/dinst_pkg.ads |    4 ++
 7 files changed, 137 insertions(+), 10 deletions(-)
 create mode 100644 gcc/testsuite/gnat.dg/dinst.adb
 create mode 100644 gcc/testsuite/gnat.dg/dinst_pkg.adb
 create mode 100644 gcc/testsuite/gnat.dg/dinst_pkg.ads

diff --git a/gcc/ada/gcc-interface/gigi.h b/gcc/ada/gcc-interface/gigi.h
index a75cb9094491..b890195cefc3 100644
--- a/gcc/ada/gcc-interface/gigi.h
+++ b/gcc/ada/gcc-interface/gigi.h
@@ -285,7 +285,7 @@ extern void process_type (Entity_Id gnat_entity);
    location and false if it doesn't.  If CLEAR_COLUMN is true, set the column
    information to 0.  */
 extern bool Sloc_to_locus (Source_Ptr Sloc, location_t *locus,
-			   bool clear_column = false);
+			   bool clear_column = false, const_tree decl = 0);
 
 /* Post an error message.  MSG is the error message, properly annotated.
    NODE is the node at which to post the error and the node to use for the
diff --git a/gcc/ada/gcc-interface/trans.c b/gcc/ada/gcc-interface/trans.c
index 31e098a0c707..0371d00fce18 100644
--- a/gcc/ada/gcc-interface/trans.c
+++ b/gcc/ada/gcc-interface/trans.c
@@ -41,6 +41,7 @@
 #include "stmt.h"
 #include "varasm.h"
 #include "output.h"
+#include "debug.h"
 #include "libfuncs.h"	/* For set_stack_check_libfunc.  */
 #include "tree-iterator.h"
 #include "gimplify.h"
@@ -255,6 +256,12 @@ static tree create_init_temporary (const char *, tree, tree *, Node_Id);
 static const char *extract_encoding (const char *) ATTRIBUTE_UNUSED;
 static const char *decode_name (const char *) ATTRIBUTE_UNUSED;
 
+/* This makes gigi's file_info_ptr visible in this translation unit,
+   so that Sloc_to_locus can look it up when deciding whether to map
+   decls to instances.  */
+
+static struct File_Info_Type *file_map;
+
 /* This is the main program of the back-end.  It sets up all the table
    structures and then generates code.  */
 
@@ -300,6 +307,12 @@ gigi (Node_Id gnat_root,
 
   type_annotate_only = (gigi_operating_mode == 1);
 
+  if (Generate_SCO_Instance_Table != 0)
+    {
+      file_map = file_info_ptr;
+      maybe_create_decl_to_instance_map (number_file);
+    }
+
   for (i = 0; i < number_file; i++)
     {
       /* Use the identifier table to make a permanent copy of the filename as
@@ -701,6 +714,7 @@ gigi (Node_Id gnat_root,
     }
 
   /* Destroy ourselves.  */
+  file_map = NULL;
   destroy_gnat_decl ();
   destroy_gnat_utils ();
 
@@ -3771,7 +3785,7 @@ Subprogram_Body_to_gnu (Node_Id gnat_node)
     }
 
   /* Set the line number in the decl to correspond to that of the body.  */
-  if (!Sloc_to_locus (Sloc (gnat_node), &locus))
+  if (!Sloc_to_locus (Sloc (gnat_node), &locus, false, gnu_subprog_decl))
     locus = input_location;
   DECL_SOURCE_LOCATION (gnu_subprog_decl) = locus;
 
@@ -9970,12 +9984,14 @@ maybe_implicit_deref (tree exp)
   return exp;
 }
 
-/* Convert SLOC into LOCUS.  Return true if SLOC corresponds to a source code
-   location and false if it doesn't.  If CLEAR_COLUMN is true, set the column
-   information to 0.  */
+/* Convert SLOC into LOCUS.  Return true if SLOC corresponds to a
+   source code location and false if it doesn't.  If CLEAR_COLUMN is
+   true, set the column information to 0.  If DECL is given and SLOC
+   refers to a File with an instance, map DECL to that instance.  */
 
 bool
-Sloc_to_locus (Source_Ptr Sloc, location_t *locus, bool clear_column)
+Sloc_to_locus (Source_Ptr Sloc, location_t *locus, bool clear_column,
+	       const_tree decl)
 {
   if (Sloc == No_Location)
     return false;
@@ -9999,6 +10015,9 @@ Sloc_to_locus (Source_Ptr Sloc, location_t *locus, bool clear_column)
   *locus
     = linemap_position_for_line_and_column (line_table, map, line, column);
 
+  if (file_map && file_map[file - 1].Instance)
+    decl_to_instance_map->put (decl, file_map[file - 1].Instance);
+
   return true;
 }
 
diff --git a/gcc/debug.h b/gcc/debug.h
index 126e56e8c8d7..3f78d0602252 100644
--- a/gcc/debug.h
+++ b/gcc/debug.h
@@ -256,4 +256,19 @@ extern bool dwarf2out_default_as_locview_support (void);
 extern const struct gcc_debug_hooks *
 dump_go_spec_init (const char *, const struct gcc_debug_hooks *);
 
+/* Instance discriminator mapping table.  See final.c.  */
+typedef hash_map<const_tree, int> decl_to_instance_map_t;
+extern decl_to_instance_map_t *decl_to_instance_map;
+
+/* Allocate decl_to_instance_map with COUNT slots to begin wtih, if it
+ * hasn't been allocated yet.  */
+
+static inline decl_to_instance_map_t *
+maybe_create_decl_to_instance_map (int count = 13)
+{
+  if (!decl_to_instance_map)
+    decl_to_instance_map = new decl_to_instance_map_t (count);
+  return decl_to_instance_map;
+}
+
 #endif /* !GCC_DEBUG_H  */
diff --git a/gcc/final.c b/gcc/final.c
index 6fa4acdaa2e9..a8338e0394c1 100644
--- a/gcc/final.c
+++ b/gcc/final.c
@@ -122,12 +122,20 @@ static int last_linenum;
 /* Column number of last NOTE.  */
 static int last_columnnum;
 
-/* Last discriminator written to assembly.  */
+/* Discriminator written to assembly.  */
 static int last_discriminator;
 
-/* Discriminator of current block.  */
+/* Discriminator to be written to assembly for current instruction.
+   Note: actual usage depends on loc_discriminator_kind setting.  */
 static int discriminator;
 
+/* Discriminator identifying current basic block among others sharing
+   the same locus.  */
+static int bb_discriminator;
+
+/* Basic block discriminator for previous instruction.  */
+static int last_bb_discriminator;
+
 /* Highest line number in current block.  */
 static int high_block_linenum;
 
@@ -1701,6 +1709,7 @@ final_start_function_1 (rtx_insn **firstp, FILE *file, int *seen,
   last_linenum = LOCATION_LINE (prologue_location);
   last_columnnum = LOCATION_COLUMN (prologue_location);
   last_discriminator = discriminator = 0;
+  last_bb_discriminator = bb_discriminator = 0;
 
   high_block_linenum = high_function_linenum = last_linenum;
 
@@ -2236,8 +2245,7 @@ final_scan_insn_1 (rtx_insn *insn, FILE *file, int optimize_p ATTRIBUTE_UNUSED,
 	  if (targetm.asm_out.unwind_emit)
 	    targetm.asm_out.unwind_emit (asm_out_file, insn);
 
-          discriminator = NOTE_BASIC_BLOCK (insn)->discriminator;
-
+	  bb_discriminator = NOTE_BASIC_BLOCK (insn)->discriminator;
 	  break;
 
 	case NOTE_INSN_EH_REGION_BEG:
@@ -3144,6 +3152,58 @@ final_scan_insn (rtx_insn *insn, FILE *file, int optimize_p,
 }
 
 
+
+/* Map DECLs to instance discriminators.  This is allocated and
+   defined in ada/gcc-interfaces/trans.c, when compiling with -gnateS.  */
+
+decl_to_instance_map_t *decl_to_instance_map;
+
+/* Return the instance number assigned to DECL.  */
+
+static inline int
+map_decl_to_instance (const_tree decl)
+{
+  int *inst;
+
+  if (!decl_to_instance_map || !decl || !DECL_P (decl))
+    return 0;
+
+  inst = decl_to_instance_map->get (decl);
+
+  if (!inst)
+    return 0;
+
+  return *inst;
+}
+
+/* Set DISCRIMINATOR to the appropriate value, possibly derived from LOC.  */
+
+static inline void
+maybe_set_discriminator (location_t loc)
+{
+  if (!decl_to_instance_map)
+    discriminator = bb_discriminator;
+  else
+    {
+      tree block = LOCATION_BLOCK (loc);
+
+      while (block && TREE_CODE (block) == BLOCK
+	     && !inlined_function_outer_scope_p (block))
+	block = BLOCK_SUPERCONTEXT (block);
+
+      tree decl;
+
+      if (!block)
+	decl = current_function_decl;
+      else if (DECL_P (block))
+	decl = block;
+      else
+	decl = block_ultimate_origin (block);
+
+      discriminator = map_decl_to_instance (decl);
+    }
+}
+
 /* Return whether a source line note needs to be emitted before INSN.
    Sets IS_STMT to TRUE if the line should be marked as a possible
    breakpoint location.  */
@@ -3178,6 +3238,7 @@ notice_source_line (rtx_insn *insn, bool *is_stmt)
       filename = xloc.file;
       linenum = xloc.line;
       columnnum = xloc.column;
+      maybe_set_discriminator (loc);
       force_source_line = true;
     }
   else if (override_filename)
@@ -3192,6 +3253,7 @@ notice_source_line (rtx_insn *insn, bool *is_stmt)
       filename = xloc.file;
       linenum = xloc.line;
       columnnum = xloc.column;
+      maybe_set_discriminator (INSN_LOCATION (insn));
     }
   else
     {
diff --git a/gcc/testsuite/gnat.dg/dinst.adb b/gcc/testsuite/gnat.dg/dinst.adb
new file mode 100644
index 000000000000..460e6c5f914f
--- /dev/null
+++ b/gcc/testsuite/gnat.dg/dinst.adb
@@ -0,0 +1,20 @@
+-- { dg-do compile { target *-*-gnu* } }
+-- { dg-options "-gnateS -gdwarf -g -O -gno-column-info" }
+-- { dg-final { scan-assembler "loc \[0-9] 5 \[0-9]( is_stmt \[0-9])? discriminator 1\n" } } */
+-- { dg-final { scan-assembler-not "loc \[0-9] 5 \[0-9]( is_stmt \[0-9])? discriminator 2\n" } } */
+-- { dg-final { scan-assembler "loc \[0-9] 5 \[0-9]( is_stmt \[0-9])? discriminator 3\n" } } */
+-- { dg-final { scan-assembler "loc \[0-9] 5 \[0-9]( is_stmt \[0-9])? discriminator 4\n" } } */
+
+
+with DInst_Pkg;
+procedure DInst is
+   package I1 is new DInst_Pkg; -- instance 1 
+   package I2 is new DInst_Pkg; -- instance 2
+   package I3 is new DInst_Pkg; -- instance 3
+   package I4 is new DInst_Pkg; -- instance 4
+begin
+   I1.Foo;
+   -- I2.Foo;
+   I3.Foo;
+   I4.Foo;
+end;
diff --git a/gcc/testsuite/gnat.dg/dinst_pkg.adb b/gcc/testsuite/gnat.dg/dinst_pkg.adb
new file mode 100644
index 000000000000..09a9baea1e4e
--- /dev/null
+++ b/gcc/testsuite/gnat.dg/dinst_pkg.adb
@@ -0,0 +1,7 @@
+with Ada.Text_IO; use Ada.Text_IO;
+package body DInst_Pkg is
+   procedure Foo is
+   begin
+      Put_Line ("hello there");
+   end;
+end;
diff --git a/gcc/testsuite/gnat.dg/dinst_pkg.ads b/gcc/testsuite/gnat.dg/dinst_pkg.ads
new file mode 100644
index 000000000000..d22afdbcd6af
--- /dev/null
+++ b/gcc/testsuite/gnat.dg/dinst_pkg.ads
@@ -0,0 +1,4 @@
+generic
+package DInst_Pkg is
+   procedure Foo;
+end;


Save discriminator info for LTO

From: Alexandre Oliva <oliva@adacore.com>

for  gcc/ChangeLog

	* gimple-streamer-in.c (input_bb): Restore BB discriminator.
	* gimple-streamer-out.c (output_bb): Save it.
	* lto-streamer-in.c (input_struct_function_base): Restore
	instance discriminator if available.  Create map on demand.
	* lto-streamer-out.c (output_struct_function_base): Save it if
	available.
	* final.c (decl_to_instance_map): Document LTO strategy.
---
 gcc/final.c               |    6 +++++-
 gcc/gimple-streamer-in.c  |    1 +
 gcc/gimple-streamer-out.c |    1 +
 gcc/lto-streamer-in.c     |    8 ++++++++
 gcc/lto-streamer-out.c    |    8 ++++++++
 5 files changed, 23 insertions(+), 1 deletion(-)

diff --git a/gcc/final.c b/gcc/final.c
index a8338e0394c1..842e5e067db7 100644
--- a/gcc/final.c
+++ b/gcc/final.c
@@ -3154,7 +3154,11 @@ final_scan_insn (rtx_insn *insn, FILE *file, int optimize_p,
 
 
 /* Map DECLs to instance discriminators.  This is allocated and
-   defined in ada/gcc-interfaces/trans.c, when compiling with -gnateS.  */
+   defined in ada/gcc-interfaces/trans.c, when compiling with -gnateS.
+   Mappings from this table are saved and restored for LTO, so
+   link-time compilation will have this map set, at least in
+   partitions containing at least one DECL with an associated instance
+   discriminator.  */
 
 decl_to_instance_map_t *decl_to_instance_map;
 
diff --git a/gcc/gimple-streamer-in.c b/gcc/gimple-streamer-in.c
index 6ffef29bf1f8..31ba4cc4e00e 100644
--- a/gcc/gimple-streamer-in.c
+++ b/gcc/gimple-streamer-in.c
@@ -270,6 +270,7 @@ input_bb (struct lto_input_block *ib, enum LTO_tags tag,
     bb->count
       = bb->count.apply_scale (count_materialization_scale, REG_BR_PROB_BASE);
   bb->flags = streamer_read_hwi (ib);
+  bb->discriminator = streamer_read_hwi (ib);
 
   /* LTO_bb1 has statements.  LTO_bb0 does not.  */
   if (tag == LTO_bb0)
diff --git a/gcc/gimple-streamer-out.c b/gcc/gimple-streamer-out.c
index d120aa902952..3a2368047cc6 100644
--- a/gcc/gimple-streamer-out.c
+++ b/gcc/gimple-streamer-out.c
@@ -212,6 +212,7 @@ output_bb (struct output_block *ob, basic_block bb, struct function *fn)
   streamer_write_uhwi (ob, bb->index);
   bb->count.stream_out (ob);
   streamer_write_hwi (ob, bb->flags);
+  streamer_write_hwi (ob, bb->discriminator);
 
   if (!gsi_end_p (bsi) || phi_nodes (bb))
     {
diff --git a/gcc/lto-streamer-in.c b/gcc/lto-streamer-in.c
index 8529c82376b8..4ddcc8f7ddd9 100644
--- a/gcc/lto-streamer-in.c
+++ b/gcc/lto-streamer-in.c
@@ -1013,6 +1013,14 @@ input_struct_function_base (struct function *fn, struct data_in *data_in,
   /* Input the function start and end loci.  */
   fn->function_start_locus = stream_input_location_now (&bp, data_in);
   fn->function_end_locus = stream_input_location_now (&bp, data_in);
+
+  /* Restore the instance discriminators if present.  */
+  int instance_number = bp_unpack_value (&bp, 1);
+  if (instance_number)
+    {
+      instance_number = bp_unpack_value (&bp, sizeof (int) * CHAR_BIT);
+      maybe_create_decl_to_instance_map ()->put (fn->decl, instance_number);
+    }
 }
 
 
diff --git a/gcc/lto-streamer-out.c b/gcc/lto-streamer-out.c
index 78b90e7f5962..9e28d678342c 100644
--- a/gcc/lto-streamer-out.c
+++ b/gcc/lto-streamer-out.c
@@ -2038,6 +2038,14 @@ output_struct_function_base (struct output_block *ob, struct function *fn)
   stream_output_location (ob, &bp, fn->function_start_locus);
   stream_output_location (ob, &bp, fn->function_end_locus);
 
+  /* Save the instance discriminator if present.  */
+  int *instance_number_p = NULL;
+  if (decl_to_instance_map)
+    instance_number_p = decl_to_instance_map->get (fn->decl);
+  bp_pack_value (&bp, !!instance_number_p, 1);
+  if (instance_number_p)
+    bp_pack_value (&bp, *instance_number_p, sizeof (int) * CHAR_BIT);
+
   streamer_write_bitpack (&bp);
 }
 

This patch is just for reference.  It's what I used in a bootstrap to
get more coverage in testing the function discovery logic to extract
instance numbers.  It caught a number of problems in my initial
attempts.

diff --git a/gcc/ada/gcc-interface/Make-lang.in b/gcc/ada/gcc-interface/Make-lang.in
index d51d3973b4d1..1625989ae585 100644
--- a/gcc/ada/gcc-interface/Make-lang.in
+++ b/gcc/ada/gcc-interface/Make-lang.in
@@ -45,7 +45,7 @@ RMDIR = rm -rf
 
 
 # Extra flags to pass to recursive makes.
-COMMON_ADAFLAGS= -gnatpg
+COMMON_ADAFLAGS= -gnatpg -gnateS
 ifeq ($(TREECHECKING),)
 CHECKING_ADAFLAGS=
 else
diff --git a/gcc/ada/gcc-interface/Makefile.in b/gcc/ada/gcc-interface/Makefile.in
index 601f23afc1c6..c710a5e37fdf 100644
--- a/gcc/ada/gcc-interface/Makefile.in
+++ b/gcc/ada/gcc-interface/Makefile.in
@@ -104,13 +104,13 @@ TEXI2DVI = texi2dvi
 TEXI2PDF = texi2pdf
 GNATBIND_FLAGS = -static -x
 ADA_CFLAGS =
-ADAFLAGS = -W -Wall -gnatpg -gnata
+ADAFLAGS = -W -Wall -gnatpg -gnata -gnateS
 FORCE_DEBUG_ADAFLAGS = -g
 NO_INLINE_ADAFLAGS = -fno-inline
 NO_OMIT_ADAFLAGS = -fno-omit-frame-pointer
 NO_SIBLING_ADAFLAGS = -fno-optimize-sibling-calls
 NO_REORDER_ADAFLAGS = -fno-toplevel-reorder
-GNATLIBFLAGS = -W -Wall -gnatpg -nostdinc
+GNATLIBFLAGS = -W -Wall -gnatpg -gnateS -nostdinc
 GNATLIBCFLAGS = -g -O2
 # Pretend that _Unwind_GetIPInfo is available for the target by default.  This
 # should be autodetected during the configuration of libada and passed down to
diff --git a/gnattools/Makefile.in b/gnattools/Makefile.in
index d19147fe5d74..6ecae0297b74 100644
--- a/gnattools/Makefile.in
+++ b/gnattools/Makefile.in
@@ -52,7 +52,7 @@ WARN_CFLAGS = @warn_cflags@
 ADA_CFLAGS=@ADA_CFLAGS@
 
 # Variables for gnattools.
-ADAFLAGS= -gnatpg -gnata
+ADAFLAGS= -gnatpg -gnata -gnateS
 
 # For finding the GCC build dir, which is used far too much
 GCC_DIR=../gcc
diff --git a/libada/Makefile.in b/libada/Makefile.in
index a2f6afef838d..ecda73ae1bbd 100644
--- a/libada/Makefile.in
+++ b/libada/Makefile.in
@@ -58,7 +58,7 @@ LDFLAGS=
 # The tedious process of getting CFLAGS right.
 CFLAGS=-g
 PICFLAG = @PICFLAG@
-GNATLIBFLAGS= -W -Wall -gnatpg -nostdinc
+GNATLIBFLAGS= -W -Wall -gnatpg -gnateS -nostdinc
 GNATLIBCFLAGS= -g -O2
 GNATLIBCFLAGS_FOR_C = -W -Wall $(GNATLIBCFLAGS) $(CFLAGS_FOR_TARGET) \
 	-fexceptions -DIN_RTS @have_getipinfo@ @have_capability@


-- 
Alexandre Oliva, freedom fighter   https://FSFLA.org/blogs/lxo
Be the change, be Free!         FSF Latin America board member
GNU Toolchain Engineer                Free Software Evangelist


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]