public inbox for gcc-patches@gcc.gnu.org
 help / color / mirror / Atom feed
From: Richard Biener <rguenther@suse.de>
To: Indu Bhagat <indu.bhagat@oracle.com>
Cc: gcc-patches@gcc.gnu.org
Subject: Re: [PATCH V7 4/7] CTF/BTF debug formats
Date: Fri, 30 Apr 2021 08:17:49 +0200 (CEST)	[thread overview]
Message-ID: <nycvar.YFH.7.76.2104300804390.9200@zhemvz.fhfr.qr> (raw)
In-Reply-To: <d8898bc6-b217-b29b-023d-77f5e6048bf0@oracle.com>

On Thu, 29 Apr 2021, Indu Bhagat wrote:

> Hello,
> 
> On 4/29/21 5:10 AM, Richard Biener wrote:
> > On Thu, 29 Apr 2021, Jose E. Marchesi wrote:
> > 
> >> This commit introduces support for generating CTF debugging
> >> information and BTF debugging information from GCC.
> > 
> > Comments inline.
> 
> Thanks for your reviews.
> 
> My responses and questions inline at respective points.
> 
> Indu
> 
> >> +void
> >> +btf_finalize (void)
> >> +{
> >> +  btf_info_section = NULL;
> >> +
> >> +  /* Clear preprocessing state.  */
> >> +  num_vars_added = 0;
> >> +  holes.release ();
> >> +  voids.release ();
> >> +  funcs.release ();
> >> +  for (size_t i = 0; i < datasecs.length (); i++)
> >> +    datasecs[i].entries.release ();
> >> +  datasecs.release ();
> >> +
> >> +  ggc_free (btf_var_ids);
> > 
> > I think you want to NULL btf_var_ids after this.
> > 
> 
> ACK. Will take care of this and the other comments' occurences in btfout.c.
> 
> >> diff --git a/gcc/ctfc.h b/gcc/ctfc.h
> >> new file mode 100644
> >> index 00000000000..092faa0b2c3
> >> --- /dev/null
> >> +++ b/gcc/ctfc.h
> >> @@ -0,0 +1,413 @@
> >> +/* ctfc.h - Declarations and definitions related to the CTF container.
> >> +   Copyright (C) 2019,2021 Free Software Foundation, Inc.
> >> +
> >> +This file is part of GCC.
> >> +
> >> +GCC is free software; you can redistribute it and/or modify it under
> >> +the terms of the GNU General Public License as published by the Free
> >> +Software Foundation; either version 3, or (at your option) any later
> >> +version.
> >> +
> >> +GCC is distributed in the hope that it will be useful, but WITHOUT ANY
> >> +WARRANTY; without even the implied warranty of MERCHANTABILITY or
> >> +FITNESS FOR A PARTICULAR PURPOSE.  See the GNU General Public License
> >> +for more details.
> >> +
> >> +You should have received a copy of the GNU General Public License
> >> +along with GCC; see the file COPYING3.  If not see
> >> +<http://www.gnu.org/licenses/>.  */
> >> +
> >> +/* This file defines the data structures and functions used by the
> >> compiler
> >> +   to generate the CTF debug info.  The definitions below are compiler
> >> internal
> >> +   representations and closely reflect the CTF format requirements in
> >> <ctf.h>.
> >> +
> >> +   The contents of the CTF container are used eventually for emission of
> >> both
> >> +   CTF (ctfout.c) and BTF debug info (btfout.c), as the two type debug
> >> formats
> >> +   are close cousins.  */
> >> +
> >> +#ifndef GCC_CTFC_H
> >> +#define GCC_CTFC_H 1
> >> +
> >> +#include "config.h"
> >> +#include "system.h"
> >> +#include "tree.h"
> >> +#include "fold-const.h"
> >> +#include "dwarf2ctf.h"
> >> +#include "ctf.h"
> >> +#include "btf.h"
> >> +
> >> +/* Invalid CTF type ID definition.  */
> >> +
> >> +#define CTF_NULL_TYPEID 0
> >> +
> >> +/* Value to start generating the CTF type ID from.  */
> >> +
> >> +#define CTF_INIT_TYPEID 1
> >> +
> >> +/* CTF type ID.  */
> >> +
> >> +typedef unsigned long ctf_id_t;
> >> +
> >> +/* CTF string table element (list node).  */
> >> +
> >> +typedef struct GTY ((chain_next ("%h.cts_next"))) ctf_string
> > 
> > I know that DWARF takes the lead here but do all these have to
> > live in GC memory?  The reason the DWARF bits do is that they
> > point to 'tree' and that trees point to DIEs.
> > 
> 
> Not entirely sure what you mean here ? Do you mean to not tag it as GC root
> and avoid traversal for GC marking the individual strings ?

Basically think of what part of the CTF data structures can live on the
heap (since you should know lifetime pretty well).

> >> +/* CTF container structure.
> >> +   It is the context passed around when generating ctf debug info.  There
> >> is
> >> +   one container per translation unit.  */
> >> +
> >> +typedef struct GTY (()) ctf_container
> >> +{
> >> +  /* CTF Preamble.  */
> >> +  unsigned short ctfc_magic;
> >> +  unsigned char ctfc_version;
> >> +  unsigned char ctfc_flags;
> >> +  unsigned int ctfc_cuname_offset;
> >> +
> >> +  /* CTF types.  */
> >> +  hash_table <ctfc_dtd_hasher> * GTY (()) ctfc_types;
> >> +  /* CTF variables.  */
> >> +  hash_table <ctfc_dvd_hasher> * GTY (()) ctfc_vars;
> >> +
> >> +  /* CTF string table.  */
> >> +  ctf_strtable_t ctfc_strtable;
> >> +  /* Auxilliary string table.  At this time, used for keeping func arg
> >> names
> >> +     for BTF.  */
> >> +  ctf_strtable_t ctfc_aux_strtable;
> >> +
> >> +  unsigned long ctfc_num_types;
> > 
> > You are using unsigned char, short and long - if those should correspond
> > to types with a specific bit width then please use stdint types.  GCC
> > still runs on 32bit hosts where 'unsigned long' can be 32bits.
> > 
> > Not sure if the number of types can be bigger than 2 billion on 64bit
> > hosts ... (GCC is happily using unsigned int for such counts elsewhere,
> > like DECL_UID is just unsigned int).
> > 
> 
> Ok. I will take a look and switch to stdint types where applicable.
> 
> >> +  unsigned long ctfc_num_stypes;
> >> +  unsigned long ctfc_num_global_funcs;
> >> +  unsigned long ctfc_num_global_objts;
> >> +
> >> +  /* Number of vlen bytes - the variable length portion after ctf_type_t
> >> and
> >> +     ctf_stype_t in the CTF section.  This is used to calculate the
> >> offsets in
> >> +     the CTF header.  */
> >> +  unsigned long ctfc_num_vlen_bytes;
> >> +
> >> +  /* Next CTF type id to assign.  */
> >> +  ctf_id_t ctfc_nextid;
> >> +  /* List of pre-processed CTF Variables.  CTF requires that the variables
> >> +     appear in the sorted order of their names.  */
> >> +  ctf_dvdef_t ** GTY ((length ("%h.ctfc_vars ? %h.ctfc_vars->elements () :
> >> 0"))) ctfc_vars_list;
> >> +  /* List of pre-processed CTF types.  CTF requires that a shared type
> >> must
> >> +     appear before the type that uses it.  For the compiler, this means
> >> types
> >> +     are emitted in sorted order of their type IDs.  */
> >> +  ctf_dtdef_t ** GTY ((length ("%h.ctfc_types ? %h.ctfc_types->elements ()
> >> : 0"))) ctfc_types_list;
> >> +  /* List of CTF function types for global functions.  The order of global
> >> +     function entries in the CTF funcinfo section is undefined by the
> >> +     compiler.  */
> >> +  ctf_dtdef_t ** GTY ((length ("%h.ctfc_num_global_funcs")))
> >> ctfc_gfuncs_list;
> >> +  /* List of CTF variables at global scope.  The order of global object
> >> entries
> >> +     in the CTF objinfo section is undefined by the  compiler.  */
> >> +  ctf_dvdef_t ** GTY ((length ("%h.ctfc_num_global_objts")))
> >> ctfc_gobjts_list;
> >> +
> >> +  /* Following members are for debugging only.  They do not add functional
> >> +     value to the task of CTF creation.  These can be cleaned up once CTF
> >> +     generation stabilizes.  */
> >> +
> >> +  /* Keep a count of the number of bytes dumped in asm for debugging
> >> +     purposes.  */
> >> +  unsigned long ctfc_numbytes_asm;
> >> +   /* Total length of all strings in CTF.  */
> >> +  size_t ctfc_strlen;
> >> +  /* Total length of all strings in aux string table.  */
> >> +  size_t ctfc_aux_strlen;
> >> +
> >> +} ctf_container_t;
> >> +
> >> +typedef ctf_container_t * ctf_container_ref;
> >> +
> >> +extern GTY (()) ctf_container_ref tu_ctfc;
> >> +
> >> +extern const char * ctf_add_string (ctf_container_ref ctfc, const char *
> >> name,
> >> +				    uint32_t * name_offset, bool aux_str);
> >> +extern void delete_ctf_container (ctf_container_ref ctfc);
> >> +
> >> +/* If the next ctf type id is still set to the init value, no ctf records
> >> to
> >> +   report.  */
> >> +#define is_empty_container(ctfc) (((ctfc)->ctfc_nextid ==
> >> CTF_INIT_TYPEID))
> >> +
> >> +#define get_num_ctf_vars(ctfc)		(ctfc->ctfc_vars->elements ())
> >> +#define get_num_ctf_types(ctfc)		(ctfc->ctfc_types->elements
> >> ())
> >> +#define get_ctf_strtab(ctfc)		((ctfc->ctfc_strtable))
> >> +#define get_ctf_aux_strtab(ctfc)	((ctfc)->ctfc_aux_strtable)
> >> +/* Get the length of the CTF string table of the CTF container.  */
> >> +#define get_cur_ctf_str_len(ctfc)	((get_ctf_strtab(ctfc)).ctstab_len)
> >> +/* Get the length of the referenced string table.  */
> >> +#define get_cur_ctf_strtab_len(strtab)	(strtab->ctstab_len)
> >> +
> >> +#define get_ctfc_num_vlen_bytes(ctfc)	((ctfc)->ctfc_num_vlen_bytes)
> > 
> > please consider inline functions or direct accesses (or member functions)
> 
> OK. Will do.
> 
> >> diff --git a/gcc/dwarf2ctf.c b/gcc/dwarf2ctf.c
> >> new file mode 100644
> >> index 00000000000..12a9cf51371
> >> --- /dev/null
> >> +++ b/gcc/dwarf2ctf.c
> >> @@ -0,0 +1,918 @@
> >> +/* Generate CTF types and objects from the GCC DWARF.
> >> +   Copyright (C) 2021 Free Software Foundation, Inc.
> >> +
> >> +This file is part of GCC.
> >> +
> >> +GCC is free software; you can redistribute it and/or modify it under
> >> +the terms of the GNU General Public License as published by the Free
> >> +Software Foundation; either version 3, or (at your option) any later
> >> +version.
> >> +
> >> +GCC is distributed in the hope that it will be useful, but WITHOUT ANY
> >> +WARRANTY; without even the implied warranty of MERCHANTABILITY or
> >> +FITNESS FOR A PARTICULAR PURPOSE.  See the GNU General Public License
> >> +for more details.
> >> +
> >> +You should have received a copy of the GNU General Public License
> >> +along with GCC; see the file COPYING3.  If not see
> >> +<http://www.gnu.org/licenses/>.  */
> >> +
> >> +#include "config.h"
> >> +#include "system.h"
> >> +#include "coretypes.h"
> >> +#include "target.h"
> >> +#include "dwarf2out.h"
> >> +#include "dwarf2int.h"
> >> +
> >> +#include "dwarf2ctf.h"
> >> +#include "ctfc.h"
> >> +
> >> +/* Forward declarations for some routines defined in this file.  */
> >> +
> >> +static ctf_id_t gen_ctf_type (ctf_container_ref, dw_die_ref);
> >> +
> >> +/* All the DIE structures we handle come from the DWARF information
> >> +   generated by GCC.  However, there are two situations where we need
> >> +   to create our own created DIE structures because GCC doesn't
> >> +   provide them.
> >> +
> >> +   The DWARF spec suggests using a DIE with DW_tag_unspecified_type
> >> +   and name "void" in order to denote the void type.  But GCC doesn't
> >> +   follow this advice.  Still we need a DIE to act as a key for void
> >> +   types, we use ctf_void_die.
> >> +
> >> +   Also, if a subrange type corresponding to an array index does not
> >> +   specify a type then we assume it is `int'.
> >> +
> >> +   The variables below are initialized in ctf_debug_init and hold
> >> +   references to the proper DIEs.  */
> >> +
> >> +dw_die_ref ctf_void_die;
> >> +dw_die_ref ctf_array_index_die;
> > 
> > I suppose these need to be GTY(())?
> > 
> 
> Yes. Will do.
> 
> >> +
> >> +/* Some DIEs have a type description attribute, stored in a DW_AT_type
> >> +   attribute.  However, GCC generates no attribute to signify a `void'
> >> +   type.
> >> +
> >> +   This can happen in many contexts (return type of a function,
> >> +   pointed or qualified type, etc) so we use the `ctf_get_AT_type'
> >> +   function below abstracts this.  */
> >> +
> >> +static dw_die_ref ctf_get_AT_type (dw_die_ref die)
> > 
> > new line after the return type
> > 
> 
> OK. Will take a closer look and fix this for all occurences in the patch
> series.
> 
> >> +{
> >> +  dw_die_ref type_die = get_AT_ref (die, DW_AT_type);
> >> +  return (type_die ? type_die : ctf_void_die);
> >> +}
> >> +
> >> +/* Some data member DIEs have location specified as a DWARF expression
> >> +   (specifically in DWARF2).  Luckily, the expression is a simple
> >> +   DW_OP_plus_uconst with one operand set to zero.
> >> +
> >> +   Sometimes the data member location may also be negative.  In yet some
> >> other
> >> +   cases (specifically union data members), the data member location is
> >> +   non-existent.  Handle all these scenarios here to abstract this.  */
> >> +
> >> +static HOST_WIDE_INT ctf_get_AT_data_member_location (dw_die_ref die)
> > 
> > likewise.
> > 
> >> +{
> >> +  HOST_WIDE_INT field_location = 0;
> >> +  dw_attr_node * attr;
> >> +
> >> +  /* The field location (in bits) can be determined from
> >> +     either a DW_AT_data_member_location attribute or a
> >> +     DW_AT_data_bit_offset attribute.  */
> >> +  if (get_AT (die, DW_AT_data_bit_offset))
> >> +    field_location = get_AT_unsigned (die, DW_AT_data_bit_offset);
> >> +  else
> >> +    {
> >> +      attr = get_AT (die, DW_AT_data_member_location);
> >> +      if (attr && AT_class (attr) == dw_val_class_loc)
> >> +	{
> >> +	  dw_loc_descr_ref descr = AT_loc (attr);
> >> +	  /* Operand 2 must be zero; the structure is assumed to be on the
> >> +	     stack in DWARF2.  */
> >> +	  gcc_assert (!descr->dw_loc_oprnd2.v.val_unsigned);
> >> +	  gcc_assert (descr->dw_loc_oprnd2.val_class
> >> +		      == dw_val_class_unsigned_const);
> >> +	  field_location = descr->dw_loc_oprnd1.v.val_unsigned;
> >> +	}
> >> +      else
> >> +	{
> >> +	  attr = get_AT (die, DW_AT_data_member_location);
> >> +	  if (attr && AT_class (attr) == dw_val_class_const)
> >> +	    field_location = AT_int (attr);
> >> +	  else
> >> +	    field_location = (get_AT_unsigned (die,
> >> +					   DW_AT_data_member_location)
> >> +			      * 8);
> >> +	}
> >> +    }
> > 
> > so when neither of the above we return 0?  Maybe we should ICE here
> > instead.  Ada for example has variable location fields.
> > 
> 
> Yes, adding gcc_unreachable is sensible. Will do.
> 
> >> +  return field_location;
> >> +}
> >> +
> >> +/* CTF Types' and CTF Variables' Location Information.  CTF section does
> >> not
> >> +   emit location information, this is provided for BTF CO-RE use-cases.
> >> These
> >> +   functions fetch information from DWARF Die directly, as such the
> >> location
> >> +   information is not buffered in the CTF container.  */
> >> +
> >> +const char *
> >> +ctf_get_die_loc_file (dw_die_ref die)
> >> +{
> >> +  if (!die)
> >> +    return NULL;
> >> +
> >> +  struct dwarf_file_data * file;
> >> +  file = get_AT_file (die, DW_AT_decl_file);
> >> +  if (!file)
> >> +    return NULL;
> >> +
> >> +  return file->filename;
> >> +}
> >> +
> >> +unsigned int
> >> +ctf_get_die_loc_line (dw_die_ref die)
> >> +{
> >> +  if (!die)
> >> +    return 0;
> >> +
> >> +  return get_AT_unsigned (die, DW_AT_decl_line);
> >> +}
> >> +
> >> +unsigned int
> >> +ctf_get_die_loc_col (dw_die_ref die)
> >> +{
> >> +  if (!die)
> >> +    return 0;
> >> +
> >> +  return get_AT_unsigned (die, DW_AT_decl_column);
> >> +}
> >> +
> >> +/* Generate CTF for the void type.  */
> >> +
> >> +static ctf_id_t
> >> +gen_ctf_void_type (ctf_container_ref ctfc)
> >> +{
> >> +  ctf_encoding_t ctf_encoding = {0, 0, 0};
> >> +
> >> +  /* In CTF the void type is encoded as a 0-byte signed integer
> >> +     type.  */
> >> +
> >> +  ctf_encoding.cte_bits = 0;
> >> +  ctf_encoding.cte_format = CTF_INT_SIGNED;
> >> +
> >> +  gcc_assert (ctf_void_die != NULL);
> >> +  return ctf_add_integer (ctfc, CTF_ADD_ROOT, "void",
> >> +			  &ctf_encoding, ctf_void_die);
> >> +}
> >> +
> >> +/* Sizes of entities can be given in bytes or bits.  This function
> >> +   abstracts this by returning the size in bits of the given entity.
> >> +   If no DW_AT_byte_size nor DW_AT_bit_size are defined, this function
> >> +   returns 0.  */
> >> +
> >> +static uint32_t ctf_die_bitsize (dw_die_ref die)
> > 
> > there are more like this it seems (missing newline)
> > 
> 
> OK. Will take care of all the occurences.
> 
> >> diff --git a/gcc/dwarf2int.h b/gcc/dwarf2int.h
> >> index f49f51d957b..3a6f813deda 100644
> >> --- a/gcc/dwarf2int.h
> >> +++ b/gcc/dwarf2int.h
> >> @@ -36,6 +36,7 @@ dw_attr_node;
> >>   
> >>   extern dw_attr_node *get_AT (dw_die_ref, enum dwarf_attribute);
> >>   extern HOST_WIDE_INT AT_int (dw_attr_node *);
> >> +extern dw_loc_descr_ref AT_loc (dw_attr_node *);
> >>   extern unsigned HOST_WIDE_INT AT_unsigned (dw_attr_node *a);
> >>   extern dw_die_ref get_AT_ref (dw_die_ref, enum dwarf_attribute);
> >>   extern const char *get_AT_string (dw_die_ref, enum dwarf_attribute);
> >> diff --git a/gcc/dwarf2out.c b/gcc/dwarf2out.c
> >> index f9ff66c81cd..851119336ad 100644
> >> --- a/gcc/dwarf2out.c
> >> +++ b/gcc/dwarf2out.c
> >> @@ -79,6 +79,7 @@ along with GCC; see the file COPYING3.  If not see
> >>   #include "output.h"
> >>   #include "expr.h"
> >>   #include "dwarf2out.h"
> >> +#include "dwarf2ctf.h"
> >>   #include "dwarf2asm.h"
> >>   #include "dwarf2int.h"
> >>   #include "toplev.h"
> >> @@ -3660,7 +3661,6 @@ static inline dw_die_ref AT_ref (dw_attr_node *);
> >>   static inline int AT_ref_external (dw_attr_node *);
> >>   static inline void set_AT_ref_external (dw_attr_node *, int);
> >>   static void add_AT_loc (dw_die_ref, enum dwarf_attribute,
> >>   dw_loc_descr_ref);
> >> -static inline dw_loc_descr_ref AT_loc (dw_attr_node *);
> >>   static void add_AT_loc_list (dw_die_ref, enum dwarf_attribute,
> >>   			     dw_loc_list_ref);
> >>   static inline dw_loc_list_ref AT_loc_list (dw_attr_node *);
> >> @@ -4877,7 +4877,7 @@ add_AT_loc (dw_die_ref die, enum dwarf_attribute
> >> attr_kind, dw_loc_descr_ref loc
> >>     add_dwarf_attr (die, &attr);
> >>   }
> >>   
> >> -static inline dw_loc_descr_ref
> >> +dw_loc_descr_ref
> >>   AT_loc (dw_attr_node *a)
> >>   {
> >>     gcc_assert (a && AT_class (a) == dw_val_class_loc);
> >> @@ -28106,7 +28106,7 @@ dwarf2out_source_line (unsigned int line, unsigned
> >> int column,
> >>     dw_line_info_table *table;
> >>     static var_loc_view lvugid;
> >>   -  if (debug_info_level < DINFO_LEVEL_TERSE)
> >> +  if (debug_info_level < DINFO_LEVEL_TERSE || dwarf_based_debuginfo_p ())
> >>       return;
> >>   
> >>     table = cur_line_info_table;
> >> @@ -31612,6 +31612,15 @@ dwarf2out_finish (const char *filename)
> >>     unsigned char checksum[16];
> >>     char dl_section_ref[MAX_ARTIFICIAL_LABEL_BYTES];
> >>   +  /* Emit CTF/BTF debug info.  */
> >> +  if ((ctf_debug_info_level > CTFINFO_LEVEL_NONE
> >> +       || btf_debug_info_level > BTFINFO_LEVEL_NONE) && lang_GNU_C ())
> >> +    ctf_debug_finalize (filename, btf_debug_info_level >
> >> BTFINFO_LEVEL_NONE);
> >> +
> >> +  /* Skip emitting DWARF if only CTF/BTF is to be generated.  */
> >> +  if (dwarf_based_debuginfo_p ())
> >> +    return;
> >> +
> >>     /* Flush out any latecomers to the limbo party.  */
> >>     flush_limbo_die_list ();
> >>   @@ -32349,6 +32358,17 @@ note_variable_value (dw_die_ref die)
> >>     FOR_EACH_CHILD (die, c, note_variable_value (c));
> >>   }
> >>   
> >> +static void
> >> +debug_format_do_cu (dw_die_ref die)
> > 
> > debug_format_do_cu sounds like abstraction that's not really there.
> > 
> > Once more comes to the party (dwarf to stabs?  dwarf to windows?)
> > we can think of something.
> > 
> 
> Ok. Will switch this to ctf specific name for now.
> 
> >> +{
> >> +  dw_die_ref c;
> >> +
> >> +  if (!ctf_do_die (die))
> >> +    return;
> >> +
> >> +  FOR_EACH_CHILD (die, c, ctf_do_die (c));
> >> +}
> >> +
> >>   /* Perform any cleanups needed after the early debug generation pass
> >>      has run.  */
> >>   @@ -32471,6 +32491,16 @@ dwarf2out_early_finish (const char *filename)
> >>         print_die (comp_unit_die (), dump_file);
> >>       }
> >>   +  /* Generate CTF debug info.  */
> >> +  if ((ctf_debug_info_level > CTFINFO_LEVEL_NONE
> >> +       || btf_debug_info_level > BTFINFO_LEVEL_NONE) && lang_GNU_C ())
> >> +    {
> >> +      ctf_debug_init ();
> >> +      debug_format_do_cu (comp_unit_die ());
> >> +      for (limbo_die_node *node = limbo_die_list; node; node = node->next)
> >> +	debug_format_do_cu (node->die);
> >> +    }
> >> +
> > 
> > Since you have support to copy .ctf sections for LTO you have to write
> > those here.  And you have to care for fat LTO objects which for dwarf
> > results in two sets of .debug_info - I suppose for CTF you can share
> > the section for the fat and the LTO part though?  So why are you
> > writing the CFT debug in dwarf2out_finish now?
> 
> The change to writing CTF/BTF debug information in dwarf2out_finish instead of
> dwarf2out_early_finish, at this time, was driven by the needs of BTF. A BTF
> section has entries of kind BTF_KIND_DATASEC which essentially give
> information about which section a variable is in (.bss, .rodata, .text). This
> information is available later on in the compilation process and hence, BTF
> needed a longer lifetime of the CTF container to update the information in the
> CTF container. In general, keeping any future needs that may come up, it
> seemed like a better design point to defer the ctf_debug_finalize to
> dwarf2out_finish (assuming we work out it out with LTO).
> 
> Can you elaborate on what it means to say "we have two sets of .debug_info for
> fat LTO objects" ?

Fat LTO (-ffat-lto-objects) means that the compile stage will produce
final assembly _and_ LTO IR.  Such object files can participate in
a normal link (throwing away the LTO IR) and in a LTO link (throwing
away the 'assembly').  For DWARF the debug info for both pieces are
not the same so we get two sets of DWARF sections, one for the LTO IR
part and one for the 'assembly' part.  I wonder how the situation with
CTF is - I think CTF is on the same level as "early DWARF", thus it
has no references to 'assembly' symbols or the like.  But BTF seems to
be "late DWARF" which means for the 'assembly' part it needs to be
present while for the LTO IR part it would need to be generated at
the LTRANS stage?

> I am also lost a bit by the comment on "share the section for the fat and LTO
> part though". If it helps, from what I can reason, I can add that I do not see
> how in LTO mode, the CTF of a compilation unit will shift at all between the
> compile phase and the LTO phase. This should be true for CTF V3 atleast. But
> for V4, this may not be true...

Note the LTO compile phase does not invoke dwarf2out_finish at all,
it just invokes dwarf2out_early_finish which means the LTO IR objects
do not get any .ctf section (as far as I read the patch).

> For a moment, for the sake of this question, if we establish that CTF/BTF
> generation always feeds off DWARF DIEs (so there is no need to access
> type/decl tree nodes), what will it take to keep LTO support while keeping
> ctf_debug_finalize in dwarf2out_finish ?

I don't think it's possible without major surgery.  Now, it looks like
you want to emit .ctf at the LTO compile stage and BTF at the LTRANS
stage.  Note you do not have access to the CTF data produced at the
LTRANS stage.  What DWARF has access to is pointers (symbol name + offset)
to the early DWARF generated DIEs for GCCs tree decls from what it
adds DW_AT_abstract_origin references and then amending those entities
with furthr DWARF attributes.

Now, the "major surgery" could for example entail streaming the CTF
data you produce from compile through WPA to the LTRANS stage.  Or
alternatively simply reading it from the compile time produced objects
by means of the DWARF DIE references.  I don't know what kind of (if any)
references you need to produce from BTF to the CTF data (I suppose they
might share indexes into a same symbol table...).

> >> diff --git a/gcc/toplev.c b/gcc/toplev.c
> >> index bcd44d59c6d..c7550051103 100644
> >> --- a/gcc/toplev.c
> >> +++ b/gcc/toplev.c
> >> @@ -1230,6 +1230,7 @@ parse_alignment_opts (void)
> >>   static void
> >>   process_options (void)
> >>   {
> >> +  const char *language_string = lang_hooks.name;
> >>     /* Just in case lang_hooks.post_options ends up calling a debug_hook.
> >>        This can happen with incorrect pre-processed input. */
> >>     debug_hooks = &do_nothing_debug_hooks;
> >> @@ -1413,6 +1414,17 @@ process_options (void)
> >>    debug_info_level = DINFO_LEVEL_NONE;
> >>       }
> >>   +  /* CTF is supported for only C at this time.
> >> +     Compiling with -flto results in frontend language of GNU GIMPLE.  */
> > 
> > But CTF is created from dwarf2out_early_finish.  Can you not simply
> > avoid traversing DWARF CUs with a DW_AT_language that is not C?
> > Without diagnostics I mean?
> 
> This inform () below helps in the gcc-debug.exp when -gctf is added to the mix
> of debug flags to be automatically tested via gcc-dg-debug-runtest dejagnu
> procedure. That procedure now relies on this inform message to prune out the
> frontends for which the tests should not be run with -gctf (like gfortran,
> g++).

Ah, OK.

> > 
> >> +  if (!lang_GNU_C ()
> >> +      && ctf_debug_info_level > CTFINFO_LEVEL_NONE)
> >> +    {
> >> +      inform (UNKNOWN_LOCATION,
> >> +	      "CTF debug info requested, but not supported for %qs frontend",
> >> +	      language_string);
> >> +      ctf_debug_info_level = CTFINFO_LEVEL_NONE;
> >> +    }
> >> +
> 
> [...]
> 
> >> diff --git a/libiberty/simple-object.c b/libiberty/simple-object.c
> >> index 6d70b95a00b..909995dd166 100644
> >> --- a/libiberty/simple-object.c
> >> +++ b/libiberty/simple-object.c
> >> @@ -304,6 +304,9 @@ handle_lto_debug_sections (const char *name, int
> >> rename)
> >>     /* Copy over .GCC.command.line section under the same name if present.
> >>     */
> >>     else if (strcmp (name, ".GCC.command.line") == 0)
> >>       return strcpy (newname, name);
> >> +  /* Copy over .ctf section under the same name if present.  */
> >> +  else if (strcmp (name, ".ctf") == 0)
> >> +    return strcpy (newname, name);
> >>     free (newname);
> >>     return NULL;
> >>   }
> >>
> > 
> > Overall I think this is fine with the suggested changes.  You may want
> > to refactor the debug info kind into a flag based one (I've seen you
> > suggested that on IRC).
> > 
> > Richard.
> > 
> 
> Thanks again for reviewing. Yes, I have started tinkering around to make the
> write_symbols into a bitmap.
> 
> Indu
> 

-- 
Richard Biener <rguenther@suse.de>
SUSE Software Solutions Germany GmbH, Maxfeldstrasse 5, 90409 Nuernberg,
Germany; GF: Felix Imendörffer; HRB 36809 (AG Nuernberg)

  parent reply	other threads:[~2021-04-30  6:17 UTC|newest]

Thread overview: 9+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-04-29 12:10 Richard Biener
2021-04-29 20:02 ` Indu Bhagat
2021-04-29 22:08   ` Indu Bhagat
2021-04-30  6:17   ` Richard Biener [this message]
2021-04-30 16:11     ` Jose E. Marchesi
2021-04-30 18:53       ` David Faust
2021-05-03  9:01         ` Richard Biener
2021-05-07 17:50     ` Indu Bhagat
  -- strict thread matches above, loose matches on Subject: below --
2021-04-19 16:47 [PATCH V7 0/7] Support for the CTF and BTF " Jose E. Marchesi
2021-04-19 16:47 ` [PATCH V7 4/7] CTF/BTF " Jose E. Marchesi

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=nycvar.YFH.7.76.2104300804390.9200@zhemvz.fhfr.qr \
    --to=rguenther@suse.de \
    --cc=gcc-patches@gcc.gnu.org \
    --cc=indu.bhagat@oracle.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).