Instance information in DWARF discriminators

The following proposed change adds a new mode of operation where
the discriminator field in DWARF debugging information is set from
information provided by a language front-end to discriminate
between distinct code instances coming from instances of a given
template. (Currently the discriminator is set by assigning
arbitrary distinct identifiers to each basic block associated with
a single source location).

This is intended primarily for the Ada language front-end (further
changes, to be submitted shortly, will enable the new mode of
operation under control of appropriate switches), in order to
allow per-instance source coverage analysis of generics. However
this might also be useful for other language front-ends, e.g. with
C++ templates.

Change tested on x86_64-linux. OK to commit?

2012-08-08  Thomas Quinot  <quinot@adacore.com>

gcc/
        * common.opt (flag_debug_instances): Define new internal flag.
        * final.c (notice_source_line): If flag_debug_instances is set,
        set discriminator to current instance id.
        * tree-cfg.c (assign_discriminator): If flag_debug_instances is set,
        nothing to do here.
	* input.h (LOCATION_INSTANCE): New macro to retrieve instance id.
	* emit-rtl.c (insn_instance): New function to retrieve instance id.
	* rtl.h (insn_instance): Declare.

libcpp/
        * include/line_map.h (struct line_map_ordinary): Add instance field.
        (expanded_location): Ditto.
        (ORDINARY_MAP_INSTANCE): Define.
        * line-map.c (linemap_expand_location): Set instance field in expanded
        location from same in set.

On Wed, Nov 28, 2012 at 4:46 PM, Thomas Quinot <quinot@adacore.com> wrote:
> The following proposed change adds a new mode of operation where
> the discriminator field in DWARF debugging information is set from
> information provided by a language front-end to discriminate
> between distinct code instances coming from instances of a given
> template. (Currently the discriminator is set by assigning
> arbitrary distinct identifiers to each basic block associated with
> a single source location).
>
> This is intended primarily for the Ada language front-end (further
> changes, to be submitted shortly, will enable the new mode of
> operation under control of appropriate switches), in order to
> allow per-instance source coverage analysis of generics. However
> this might also be useful for other language front-ends, e.g. with
> C++ templates.
>
> Change tested on x86_64-linux. OK to commit?

You need to stream the id with LTO, no?  Also increasing the size
of line_map_ordinary might not be the most welcom change ...

Documentation of the flag is missing as well.

Richard.

> 2012-08-08  Thomas Quinot  <quinot@adacore.com>
>
> gcc/
>         * common.opt (flag_debug_instances): Define new internal flag.
>         * final.c (notice_source_line): If flag_debug_instances is set,
>         set discriminator to current instance id.
>         * tree-cfg.c (assign_discriminator): If flag_debug_instances is set,
>         nothing to do here.
>         * input.h (LOCATION_INSTANCE): New macro to retrieve instance id.
>         * emit-rtl.c (insn_instance): New function to retrieve instance id.
>         * rtl.h (insn_instance): Declare.
>
> libcpp/
>         * include/line_map.h (struct line_map_ordinary): Add instance field.
>         (expanded_location): Ditto.
>         (ORDINARY_MAP_INSTANCE): Define.
>         * line-map.c (linemap_expand_location): Set instance field in expanded
>         location from same in set.
>
> Index: gcc/final.c
> ===================================================================
> --- gcc/final.c (révision 193884)
> +++ gcc/final.c (copie de travail)
> @@ -2894,6 +2894,10 @@
>      {
>        filename = insn_file (insn);
>        linenum = insn_line (insn);
> +      if (flag_debug_instances)
> +        {
> +          discriminator = insn_instance (insn);
> +        }

no {} braces

>      }
>
>    if (filename == NULL)
> Index: gcc/input.h
> ===================================================================
> --- gcc/input.h (révision 193884)
> +++ gcc/input.h (copie de travail)
> @@ -51,6 +51,7 @@
>  #define LOCATION_FILE(LOC) ((expand_location (LOC)).file)
>  #define LOCATION_LINE(LOC) ((expand_location (LOC)).line)
>  #define LOCATION_COLUMN(LOC)((expand_location (LOC)).column)
> +#define LOCATION_INSTANCE(LOC) ((expand_location (LOC)).instance)
>  #define LOCATION_LOCUS(LOC) \
>    ((IS_ADHOC_LOC(LOC)) ? get_location_from_adhoc_loc (line_table, LOC) : (LOC))
>  #define LOCATION_BLOCK(LOC) \
> Index: gcc/emit-rtl.c
> ===================================================================
> --- gcc/emit-rtl.c      (révision 193884)
> +++ gcc/emit-rtl.c      (copie de travail)
> @@ -6007,6 +6007,14 @@
>    return LOCATION_FILE (INSN_LOCATION (insn));
>  }
>
> +/* Return source instance of the statement that produced this insn.  */
> +
> +int
> +insn_instance (const_rtx insn)
> +{
> +  return LOCATION_INSTANCE (INSN_LOCATION (insn));
> +}
> +
>  /* Return true if memory model MODEL requires a pre-operation (release-style)
>     barrier or a post-operation (acquire-style) barrier.  While not universal,
>     this function matches behavior of several targets.  */
> Index: gcc/common.opt
> ===================================================================
> --- gcc/common.opt      (révision 193884)
> +++ gcc/common.opt      (copie de travail)
> @@ -158,6 +158,11 @@
>  Variable
>  int flag_debug_asm
>
> +; For generic instances, include complete instantiation chain in debugging
> +; information (ELF discriminators).
> +Variable
> +int flag_debug_instances = 0
> +
>  ; -dP causes the rtl to be emitted as a comment in assembly.
>  Variable
>  int flag_dump_rtl_in_asm
> Index: gcc/rtl.h
> ===================================================================
> --- gcc/rtl.h   (révision 193884)
> +++ gcc/rtl.h   (copie de travail)
> @@ -1917,6 +1917,7 @@
>  /* In emit-rtl.c  */
>  extern int insn_line (const_rtx);
>  extern const char * insn_file (const_rtx);
> +extern int insn_instance (const_rtx);
>  extern tree insn_scope (const_rtx);
>  extern location_t prologue_location, epilogue_location;
>
> Index: gcc/tree-cfg.c
> ===================================================================
> --- gcc/tree-cfg.c      (révision 193884)
> +++ gcc/tree-cfg.c      (copie de travail)
> @@ -768,7 +768,7 @@
>  {
>    gimple first_in_to_bb, last_in_to_bb;
>
> -  if (locus == 0 || bb->discriminator != 0)
> +  if (locus == 0 || bb->discriminator != 0 || flag_debug_instances)
>      return;
>
>    first_in_to_bb = first_non_label_stmt (bb);
> Index: libcpp/include/line-map.h
> ===================================================================
> --- libcpp/include/line-map.h   (révision 193884)
> +++ libcpp/include/line-map.h   (copie de travail)
> @@ -85,6 +85,12 @@
>
>    /* Number of the low-order source_location bits used for a column number.  */
>    unsigned int column_bits : 8;
> +
> +  /* For languages that have the notion of instantiating a given template
> +     multiple times, different line_maps can be allocated for each instance,
> +     in which case this index identifies the specific instance. Set to 0
> +     otherwise.  */
> +  int instance;
>  };
>
>  /* This is the highest possible source location encoded within an
> @@ -230,6 +236,9 @@
>  #define ORDINARY_MAP_NUMBER_OF_COLUMN_BITS(MAP) \
>    linemap_check_ordinary (MAP)->d.ordinary.column_bits
>
> +#define ORDINARY_MAP_INSTANCE(MAP) \
> +  linemap_check_ordinary (MAP)->d.ordinary.instance
> +
>  #define MACRO_MAP_MACRO(MAP) (MAP)->d.macro.macro
>
>  #define MACRO_MAP_NUM_MACRO_TOKENS(MAP) (MAP)->d.macro.n_tokens
> @@ -638,6 +647,9 @@
>
>    /* In a system header?. */
>    bool sysp;
> +
> +  /* Instance id */
> +  int instance;
>  } expanded_location;
>
>  /* This is enum is used by the function linemap_resolve_location
> Index: libcpp/line-map.c
> ===================================================================
> --- libcpp/line-map.c   (révision 193884)
> +++ libcpp/line-map.c   (copie de travail)
> @@ -1395,6 +1395,7 @@
>        xloc.line = SOURCE_LINE (map, loc);
>        xloc.column = SOURCE_COLUMN (map, loc);
>        xloc.sysp = LINEMAP_SYSP (map) != 0;
> +      xloc.instance = ORDINARY_MAP_INSTANCE(map);
>      }
>
>    return xloc;
>
> --
> Thomas Quinot, Ph.D. ** quinot@adacore.com ** Senior Software Engineer
>                AdaCore -- Paris, France -- New York, USA

* Richard Biener, 2012-11-28 :

> You need to stream the id with LTO, no?  Also increasing the size
> of line_map_ordinary might not be the most welcom change ...

Thanks for your feedback Richard, I'll take care of adding the missing
LTO pieces, and improve documentation of the flag.

As for the size impact on line_map_ordinary, indeed that's an additional
int here, but I'm not sure we allocate so many objects of that kind that
the increase is significant.

Now, an alternative might be to have an array of instance IDs stored at
the struct line_maps level, with the same indices at the set of ordinary
maps, which would be allocated only when flag_debug_instances is used;
when it is not used the cost would then be a single NULL pointer in
struct line_maps. Would that be acceptable?

Side note: I first tried to implement this option using a vec<int>,
but it appears that vec.h cannot currently be used from within libcpp.
So, I'll have to either change that (but that seems like a pretty
significant change) or else manually manage this array through xrealloc.

Thomas.

>>>>> "Thomas" == Thomas Quinot <quinot@adacore.com> writes:

Thomas> As for the size impact on line_map_ordinary, indeed that's an additional
Thomas> int here, but I'm not sure we allocate so many objects of that kind that
Thomas> the increase is significant.

Could you please measure it?
My guess is that it is worse than you think.

Thomas> Now, an alternative might be to have an array of instance IDs stored at
Thomas> the struct line_maps level, with the same indices at the set of ordinary
Thomas> maps, which would be allocated only when flag_debug_instances is used;
Thomas> when it is not used the cost would then be a single NULL pointer in
Thomas> struct line_maps. Would that be acceptable?

I thought there was already a similar approach in place.
See location_adhoc_data.
The proliferation of ways to do this is mildly worrying.

Tom