[PATCH 0/2] Fix write_symbols for supporting multiple debug formats

[PATCH 0/2] Fix write_symbols for supporting multiple debug formats

[Indu Bhagat]

Hello,

Over the last year, we have discussed and agreed that in order to support
multiple debug formats, we keep DWARF as the default internal debug format; Any
new debug format to be supported feeds off DWARF dies. This requirement
specification has worked well for addition for CTF/BTF overall. 

There are some existing issues that need to discussed and fixed in this regard,
however. One of them is the definition and handling of write_symbols.

The current issue is that write_symbols is defined as 

   enum debug_info_type write_symbols = NO_DEBUG;

This means any new combination of debug formats needs to be explicitly
enumerated, like CTF_AND_DWARF2_DEBUG, VMS_AND_DWARF2_DEBUG etc. So to support
say, "-gctf -gbtf -g" or to make other combinations of debug formats possible,
each one needs to spelled out explicitly; this will make the handling ugly.

We discussed over IRC about the possibility of write_symbols to use bitmasks
instead. Please take a look at the patch set and let me know what you think.

BTW, the patch 2/2 [dwarf: new dwarf_debuginfo_p predicate] in this series is
the same as the one sent earlier in the CTF patch series (and has been
approved). I just include it in this patch series as it fits better here.

In a subsequent patch after these current two patches, I can work on removing
the VMS_AND_DWARF2_DEBUG symbol and replacing its usages with the appropriate
bitmasks. I would also like to review the usages of debug_type_names [] in code
diagnostics around PCH (in c-family/c-opts.c and c-family/c-pch.c) in terms of
what combination of debug formats would be allowed and such. But at this
time, the patch retains the current behaviour by simply adjusting the approach
to access the debug_type_name [].

Boostrapped and regression tested on x86_64.

Thanks,
Indu Bhagat (2):
  opts: change write_symbols to support bitmasks
  dwarf: new dwarf_debuginfo_p predicate

 gcc/c-family/c-lex.c  |   4 +-
 gcc/c-family/c-opts.c |  10 +++--
 gcc/c-family/c-pch.c  |  12 +++---
 gcc/common.opt        |   2 +-
 gcc/config/c6x/c6x.c  |   3 +-
 gcc/dwarf2cfi.c       |   9 ++--
 gcc/final.c           |  15 +++----
 gcc/flag-types.h      |  29 ++++++++++---
 gcc/flags.h           |  21 ++++++++-
 gcc/objc/objc-act.c   |   2 +-
 gcc/opts.c            | 117 +++++++++++++++++++++++++++++++++++++++++++++-----
 gcc/targhooks.c       |   2 +-
 gcc/toplev.c          |  15 ++++---
 13 files changed, 186 insertions(+), 55 deletions(-)

-- 
1.8.3.1

[PATCH 1/2] opts: change write_symbols to support bitmasks

[Indu Bhagat]

To support multiple debug formats, we need to move away from explicit
enumeration of each individual combination of debug formats.

gcc/c-family/ChangeLog:

	* c-opts.c (c_common_post_options): Adjust access to debug_type_names.
	* c-pch.c (struct c_pch_validity): Use type uint32_t.
	(pch_init): Renamed member.
	(c_common_valid_pch): Adjust access to debug_type_names.

gcc/ChangeLog:

	* common.opt: Change type to support bitmasks.
	* flag-types.h (enum debug_info_type): Rename enumerator constants.
	(NO_DEBUG): New bitmask.
	(DBX_DEBUG): Likewise.
	(DWARF2_DEBUG): Likewise.
	(XCOFF_DEBUG): Likewise.
	(VMS_DEBUG): Likewise.
	(VMS_AND_DWARF2_DEBUG): Likewise.
	* flags.h (debug_set_to_format): New function declaration.
	(debug_set_count): Likewise.
	(debug_set_names): Likewise.
	* opts.c (debug_type_masks): Array of bitmasks for debug formats.
	(debug_set_to_format): New function definition.
	(debug_set_count): Likewise.
	(debug_set_names): Likewise.
	(set_debug_level): Update access to debug_type_names.
	* toplev.c: Likewise.

gcc/objc/ChangeLog:

	* objc-act.c (synth_module_prologue): Use uint32_t instead of enum
	debug_info_type.
---
 gcc/c-family/c-opts.c |  10 +++--
 gcc/c-family/c-pch.c  |  12 +++---
 gcc/common.opt        |   2 +-
 gcc/flag-types.h      |  29 ++++++++++----
 gcc/flags.h           |  17 +++++++-
 gcc/objc/objc-act.c   |   2 +-
 gcc/opts.c            | 109 +++++++++++++++++++++++++++++++++++++++++++++-----
 gcc/toplev.c          |   9 +++--
 8 files changed, 158 insertions(+), 32 deletions(-)

diff --git a/gcc/c-family/c-opts.c b/gcc/c-family/c-opts.c
index 89e05a4..e463240 100644
--- a/gcc/c-family/c-opts.c
+++ b/gcc/c-family/c-opts.c
@@ -1112,9 +1112,13 @@ c_common_post_options (const char **pfilename)
 	  /* Only -g0 and -gdwarf* are supported with PCH, for other
 	     debug formats we warn here and refuse to load any PCH files.  */
 	  if (write_symbols != NO_DEBUG && write_symbols != DWARF2_DEBUG)
-	    warning (OPT_Wdeprecated,
-		     "the %qs debug format cannot be used with "
-		     "pre-compiled headers", debug_type_names[write_symbols]);
+	    {
+	      gcc_assert (debug_set_count (write_symbols) <= 1);
+	      warning (OPT_Wdeprecated,
+		       "the %qs debug format cannot be used with "
+		       "pre-compiled headers",
+		       debug_type_names[debug_set_to_format (write_symbols)]);
+	    }
 	}
       else if (write_symbols != NO_DEBUG && write_symbols != DWARF2_DEBUG)
 	c_common_no_more_pch ();
diff --git a/gcc/c-family/c-pch.c b/gcc/c-family/c-pch.c
index fd94c37..6804388 100644
--- a/gcc/c-family/c-pch.c
+++ b/gcc/c-family/c-pch.c
@@ -52,7 +52,7 @@ enum {
 
 struct c_pch_validity
 {
-  unsigned char debug_info_type;
+  uint32_t pch_write_symbols;
   signed char match[MATCH_SIZE];
   void (*pch_init) (void);
   size_t target_data_length;
@@ -108,7 +108,7 @@ pch_init (void)
   pch_outfile = f;
 
   memset (&v, '\0', sizeof (v));
-  v.debug_info_type = write_symbols;
+  v.pch_write_symbols = write_symbols;
   {
     size_t i;
     for (i = 0; i < MATCH_SIZE; i++)
@@ -252,13 +252,15 @@ c_common_valid_pch (cpp_reader *pfile, const char *name, int fd)
   /* The allowable debug info combinations are that either the PCH file
      was built with the same as is being used now, or the PCH file was
      built for some kind of debug info but now none is in use.  */
-  if (v.debug_info_type != write_symbols
+  if (v.pch_write_symbols != write_symbols
       && write_symbols != NO_DEBUG)
     {
+      gcc_assert (debug_set_count (v.pch_write_symbols) <= 1);
+      gcc_assert (debug_set_count (write_symbols) <= 1);
       cpp_warning (pfile, CPP_W_INVALID_PCH,
 		   "%s: created with -g%s, but used with -g%s", name,
-		   debug_type_names[v.debug_info_type],
-		   debug_type_names[write_symbols]);
+		   debug_type_names[debug_set_to_format (v.pch_write_symbols)],
+		   debug_type_names[debug_set_to_format (write_symbols)]);
       return 2;
     }
 
diff --git a/gcc/common.opt b/gcc/common.opt
index a75b44e..ffb968d 100644
--- a/gcc/common.opt
+++ b/gcc/common.opt
@@ -109,7 +109,7 @@ bool exit_after_options
 ; flag-types.h for the definitions of the different possible types of
 ; debugging information.
 Variable
-enum debug_info_type write_symbols = NO_DEBUG
+uint32_t write_symbols = NO_DEBUG
 
 ; Level of debugging information we are producing.  See flag-types.h
 ; for the definitions of the different possible levels.
diff --git a/gcc/flag-types.h b/gcc/flag-types.h
index a038c8f..d60bb30 100644
--- a/gcc/flag-types.h
+++ b/gcc/flag-types.h
@@ -24,15 +24,30 @@ along with GCC; see the file COPYING3.  If not see
 
 enum debug_info_type
 {
-  NO_DEBUG,	    /* Write no debug info.  */
-  DBX_DEBUG,	    /* Write BSD .stabs for DBX (using dbxout.c).  */
-  DWARF2_DEBUG,	    /* Write Dwarf v2 debug info (using dwarf2out.c).  */
-  XCOFF_DEBUG,	    /* Write IBM/Xcoff debug info (using dbxout.c).  */
-  VMS_DEBUG,        /* Write VMS debug info (using vmsdbgout.c).  */
-  VMS_AND_DWARF2_DEBUG /* Write VMS debug info (using vmsdbgout.c).
-                          and DWARF v2 debug info (using dwarf2out.c).  */
+  DINFO_TYPE_NONE = 0,		  /* No debug info.  */
+  DINFO_TYPE_DBX = 1,		  /* BSD .stabs for DBX.  */
+  DINFO_TYPE_DWARF2 = 2,	  /* Dwarf v2 debug info.  */
+  DINFO_TYPE_XCOFF = 3,		  /* IBM/Xcoff debug info.  */
+  DINFO_TYPE_VMS = 4,		  /* VMS debug info.  */
+  DINFO_TYPE_MAX = DINFO_TYPE_VMS /* Marker only.  */
 };
 
+#define NO_DEBUG      (0U)
+/* Write DBX debug info (using dbxout.c).  */
+#define DBX_DEBUG     (1U << DINFO_TYPE_DBX)
+/* Write DWARF2 debug info (using dwarf2out.c).  */
+#define DWARF2_DEBUG  (1U << DINFO_TYPE_DWARF2)
+/* Write IBM/XCOFF debug info (using dbxout.c).  */
+#define XCOFF_DEBUG   (1U << DINFO_TYPE_XCOFF)
+/* Write VMS debug info (using vmsdbgout.c).  */
+#define VMS_DEBUG     (1U << DINFO_TYPE_VMS)
+/* Note: Adding new definitions to handle -combination- of debug formats,
+   like VMS_AND_DWARF2_DEBUG is not recommended.  This definition remains
+   here for historical reasons.  */
+/* Write VMS debug info (using vmsdbgout.c) and DWARF v2 debug info (using
+   dwarf2out.c).  */
+#define VMS_AND_DWARF2_DEBUG  ((VMS_DEBUG | DWARF2_DEBUG))
+
 enum debug_info_levels
 {
   DINFO_LEVEL_NONE,	/* Write no debugging info.  */
diff --git a/gcc/flags.h b/gcc/flags.h
index 0c4409e..3415493 100644
--- a/gcc/flags.h
+++ b/gcc/flags.h
@@ -22,9 +22,24 @@ along with GCC; see the file COPYING3.  If not see
 
 #if !defined(IN_LIBGCC2) && !defined(IN_TARGET_LIBS) && !defined(IN_RTS)
 
-/* Names of debug_info_type, for error messages.  */
+/* Names of fundamental debug info formats indexed by enum
+   debug_info_type.  */
+
 extern const char *const debug_type_names[];
 
+/* Get enum debug_info_type of the specified debug format, for error messages.
+   Can be used only for individual debug format types.  */
+
+extern enum debug_info_type debug_set_to_format (uint32_t debug_info_set);
+
+/* Get the number of debug formats enabled for output.  */
+
+unsigned int debug_set_count (uint32_t w_symbols);
+
+/* Get the names of the debug formats enabled for output.  */
+
+const char * debug_set_names (uint32_t w_symbols);
+
 extern void strip_off_ending (char *, int);
 extern int base_of_path (const char *path, const char **base_out);
 
diff --git a/gcc/objc/objc-act.c b/gcc/objc/objc-act.c
index 1cbd586..a5aa3c5 100644
--- a/gcc/objc/objc-act.c
+++ b/gcc/objc/objc-act.c
@@ -3078,7 +3078,7 @@ static void
 synth_module_prologue (void)
 {
   tree type;
-  enum debug_info_type save_write_symbols = write_symbols;
+  uint32_t save_write_symbols = write_symbols;
   const struct gcc_debug_hooks *const save_hooks = debug_hooks;
 
   /* Suppress outputting debug symbols, because
diff --git a/gcc/opts.c b/gcc/opts.c
index 24bb641..026952f 100644
--- a/gcc/opts.c
+++ b/gcc/opts.c
@@ -37,12 +37,95 @@ along with GCC; see the file COPYING3.  If not see
 
 static void set_Wstrict_aliasing (struct gcc_options *opts, int onoff);
 
-/* Indexed by enum debug_info_type.  */
+/* Names of fundamental debug info formats indexed by enum
+   debug_info_type.  */
+
 const char *const debug_type_names[] =
 {
   "none", "stabs", "dwarf-2", "xcoff", "vms"
 };
 
+/* Bitmasks of fundamental debug info formats indexed by enum
+   debug_info_type.  */
+
+static uint32_t debug_type_masks[] =
+{
+  NO_DEBUG, DBX_DEBUG, DWARF2_DEBUG, XCOFF_DEBUG, VMS_DEBUG
+};
+
+/* Names of the set of debug formats requested by user.  Updated and accessed
+   via debug_set_names.  */
+
+static char df_set_names[sizeof "none stabs dwarf-2 xcoff vms"];
+
+/* Get enum debug_info_type of the specified debug format, for error messages.
+   Can be used only for individual debug format types.  */
+
+enum debug_info_type
+debug_set_to_format (uint32_t debug_info_set)
+{
+  int idx = 0;
+  enum debug_info_type dinfo_type = DINFO_TYPE_NONE;
+  /* Find first set bit.  */
+  if (debug_info_set)
+    idx = exact_log2 (debug_info_set & - debug_info_set);
+  /* Check that only one bit is set, if at all.  This function is meant to be
+     used only for vanilla debug_info_set bitmask values, i.e. for individual
+     debug format types upto DINFO_TYPE_MAX.  */
+  gcc_assert ((debug_info_set & (debug_info_set - 1)) == 0);
+  dinfo_type = (enum debug_info_type)idx;
+  gcc_assert (dinfo_type <= DINFO_TYPE_MAX);
+  return dinfo_type;
+}
+
+/* Get the number of debug formats enabled for output.  */
+
+unsigned int
+debug_set_count (uint32_t w_symbols)
+{
+  unsigned int count = 0;
+  while (w_symbols)
+    {
+      ++ count;
+      w_symbols &= ~ (w_symbols & - w_symbols);
+    }
+  return count;
+}
+
+/* Get the names of the debug formats enabled for output.  */
+
+const char *
+debug_set_names (uint32_t w_symbols)
+{
+  uint32_t df_mask = 0;
+  /* Reset the string to be returned.  */
+  memset (df_set_names, 0, sizeof (df_set_names));
+  /* Get the popcount.  */
+  int num_set_df = debug_set_count (w_symbols);
+  /* Iterate over the debug formats.  Add name string for those enabled.  */
+  for (int i = DINFO_TYPE_NONE; i <= DINFO_TYPE_MAX; i++)
+    {
+      df_mask = debug_type_masks[i];
+      if (w_symbols & df_mask)
+	{
+	  strcat (df_set_names, debug_type_names[i]);
+	  num_set_df--;
+	  if (num_set_df)
+	    strcat (df_set_names, " ");
+	  else
+	    break;
+	}
+      else if (!w_symbols)
+	{
+	  /* No debug formats enabled.  */
+	  gcc_assert (i == DINFO_TYPE_NONE);
+	  strcat (df_set_names, debug_type_names[i]);
+	  break;
+	}
+    }
+  return df_set_names;
+}
+
 /* Parse the -femit-struct-debug-detailed option value
    and set the flag variables. */
 
@@ -190,7 +273,7 @@ static const char use_diagnosed_msg[] = N_("Uses of this option are diagnosed.")
 
 typedef char *char_p; /* For DEF_VEC_P.  */
 
-static void set_debug_level (enum debug_info_type type, int extended,
+static void set_debug_level (uint32_t dinfo, int extended,
 			     const char *arg, struct gcc_options *opts,
 			     struct gcc_options *opts_set,
 			     location_t loc);
@@ -3027,17 +3110,17 @@ fast_math_flags_struct_set_p (struct cl_optimization *opt)
 }
 
 /* Handle a debug output -g switch for options OPTS
-   (OPTS_SET->x_write_symbols storing whether a debug type was passed
+   (OPTS_SET->x_write_symbols storing whether a debug format was passed
    explicitly), location LOC.  EXTENDED is true or false to support
    extended output (2 is special and means "-ggdb" was given).  */
 static void
-set_debug_level (enum debug_info_type type, int extended, const char *arg,
+set_debug_level (uint32_t dinfo, int extended, const char *arg,
 		 struct gcc_options *opts, struct gcc_options *opts_set,
 		 location_t loc)
 {
   opts->x_use_gnu_debug_info_extensions = extended;
 
-  if (type == NO_DEBUG)
+  if (dinfo == NO_DEBUG)
     {
       if (opts->x_write_symbols == NO_DEBUG)
 	{
@@ -3058,14 +3141,18 @@ set_debug_level (enum debug_info_type type, int extended, const char *arg,
     }
   else
     {
-      /* Does it conflict with an already selected type?  */
+      /* Does it conflict with an already selected debug format?  */
       if (opts_set->x_write_symbols != NO_DEBUG
 	  && opts->x_write_symbols != NO_DEBUG
-	  && type != opts->x_write_symbols)
-	error_at (loc, "debug format %qs conflicts with prior selection",
-		  debug_type_names[type]);
-      opts->x_write_symbols = type;
-      opts_set->x_write_symbols = type;
+	  && dinfo != opts->x_write_symbols)
+	{
+	  gcc_assert (debug_set_count (dinfo) <= 1);
+	  error_at (loc, "debug format %qs conflicts with prior selection",
+		    debug_type_names[debug_set_to_format (dinfo)]);
+	}
+
+      opts->x_write_symbols = dinfo;
+      opts_set->x_write_symbols = dinfo;
     }
 
   /* A debug flag without a level defaults to level 2.
diff --git a/gcc/toplev.c b/gcc/toplev.c
index d8cc254..a0ccec4 100644
--- a/gcc/toplev.c
+++ b/gcc/toplev.c
@@ -1460,9 +1460,12 @@ process_options (void)
     debug_hooks = &dwarf2_lineno_debug_hooks;
 #endif
   else
-    error_at (UNKNOWN_LOCATION,
-	      "target system does not support the %qs debug format",
-	      debug_type_names[write_symbols]);
+    {
+      gcc_assert (debug_set_count (write_symbols) <= 1);
+      error_at (UNKNOWN_LOCATION,
+		"target system does not support the %qs debug format",
+		debug_type_names[debug_set_to_format (write_symbols)]);
+    }
 
   /* We know which debug output will be used so we can set flag_var_tracking
      and flag_var_tracking_uninit if the user has not specified them.  */
-- 
1.8.3.1

Re: [PATCH 1/2] opts: change write_symbols to support bitmasks

[Richard Biener]

On Thu, May 6, 2021 at 2:31 AM Indu Bhagat via Gcc-patches
<gcc-patches@gcc.gnu.org> wrote:
>
> To support multiple debug formats, we need to move away from explicit
> enumeration of each individual combination of debug formats.

debug_set_names with its static buffer seems unused?  You wire quite some
APIs with gcc_assert on having a single bit set - that doesn't look forward
looking.

I suppose the BTF followups will "fix" this, but see comments below.

> gcc/c-family/ChangeLog:
>
>         * c-opts.c (c_common_post_options): Adjust access to debug_type_names.
>         * c-pch.c (struct c_pch_validity): Use type uint32_t.
>         (pch_init): Renamed member.
>         (c_common_valid_pch): Adjust access to debug_type_names.
>
> gcc/ChangeLog:
>
>         * common.opt: Change type to support bitmasks.
>         * flag-types.h (enum debug_info_type): Rename enumerator constants.
>         (NO_DEBUG): New bitmask.
>         (DBX_DEBUG): Likewise.
>         (DWARF2_DEBUG): Likewise.
>         (XCOFF_DEBUG): Likewise.
>         (VMS_DEBUG): Likewise.
>         (VMS_AND_DWARF2_DEBUG): Likewise.
>         * flags.h (debug_set_to_format): New function declaration.
>         (debug_set_count): Likewise.
>         (debug_set_names): Likewise.
>         * opts.c (debug_type_masks): Array of bitmasks for debug formats.
>         (debug_set_to_format): New function definition.
>         (debug_set_count): Likewise.
>         (debug_set_names): Likewise.
>         (set_debug_level): Update access to debug_type_names.
>         * toplev.c: Likewise.
>
> gcc/objc/ChangeLog:
>
>         * objc-act.c (synth_module_prologue): Use uint32_t instead of enum
>         debug_info_type.
> ---
>  gcc/c-family/c-opts.c |  10 +++--
>  gcc/c-family/c-pch.c  |  12 +++---
>  gcc/common.opt        |   2 +-
>  gcc/flag-types.h      |  29 ++++++++++----
>  gcc/flags.h           |  17 +++++++-
>  gcc/objc/objc-act.c   |   2 +-
>  gcc/opts.c            | 109 +++++++++++++++++++++++++++++++++++++++++++++-----
>  gcc/toplev.c          |   9 +++--
>  8 files changed, 158 insertions(+), 32 deletions(-)
>
> diff --git a/gcc/c-family/c-opts.c b/gcc/c-family/c-opts.c
> index 89e05a4..e463240 100644
> --- a/gcc/c-family/c-opts.c
> +++ b/gcc/c-family/c-opts.c
> @@ -1112,9 +1112,13 @@ c_common_post_options (const char **pfilename)
>           /* Only -g0 and -gdwarf* are supported with PCH, for other
>              debug formats we warn here and refuse to load any PCH files.  */
>           if (write_symbols != NO_DEBUG && write_symbols != DWARF2_DEBUG)
> -           warning (OPT_Wdeprecated,
> -                    "the %qs debug format cannot be used with "
> -                    "pre-compiled headers", debug_type_names[write_symbols]);
> +           {
> +             gcc_assert (debug_set_count (write_symbols) <= 1);

Why this assert?  Iff then simply include the count check in the
condition of the warning.

> +             warning (OPT_Wdeprecated,
> +                      "the %qs debug format cannot be used with "
> +                      "pre-compiled headers",
> +                      debug_type_names[debug_set_to_format (write_symbols)]);

Maybe simply emit another diagnostic if debug_set_count > 1.

> +           }
>         }
>        else if (write_symbols != NO_DEBUG && write_symbols != DWARF2_DEBUG)
>         c_common_no_more_pch ();
> diff --git a/gcc/c-family/c-pch.c b/gcc/c-family/c-pch.c
> index fd94c37..6804388 100644
> --- a/gcc/c-family/c-pch.c
> +++ b/gcc/c-family/c-pch.c
> @@ -52,7 +52,7 @@ enum {
>
>  struct c_pch_validity
>  {
> -  unsigned char debug_info_type;
> +  uint32_t pch_write_symbols;
>    signed char match[MATCH_SIZE];
>    void (*pch_init) (void);
>    size_t target_data_length;
> @@ -108,7 +108,7 @@ pch_init (void)
>    pch_outfile = f;
>
>    memset (&v, '\0', sizeof (v));
> -  v.debug_info_type = write_symbols;
> +  v.pch_write_symbols = write_symbols;
>    {
>      size_t i;
>      for (i = 0; i < MATCH_SIZE; i++)
> @@ -252,13 +252,15 @@ c_common_valid_pch (cpp_reader *pfile, const char *name, int fd)
>    /* The allowable debug info combinations are that either the PCH file
>       was built with the same as is being used now, or the PCH file was
>       built for some kind of debug info but now none is in use.  */
> -  if (v.debug_info_type != write_symbols
> +  if (v.pch_write_symbols != write_symbols
>        && write_symbols != NO_DEBUG)
>      {
> +      gcc_assert (debug_set_count (v.pch_write_symbols) <= 1);
> +      gcc_assert (debug_set_count (write_symbols) <= 1);

So the read-in PCH will have at most one bit set but I don't think
you can assert on write_symbols here.

Otherwise looks OK.  Did you check for write_symbols uses in FEs and targets?

Richard.

>        cpp_warning (pfile, CPP_W_INVALID_PCH,
>                    "%s: created with -g%s, but used with -g%s", name,
> -                  debug_type_names[v.debug_info_type],
> -                  debug_type_names[write_symbols]);
> +                  debug_type_names[debug_set_to_format (v.pch_write_symbols)],
> +                  debug_type_names[debug_set_to_format (write_symbols)]);
>        return 2;
>      }
>
> diff --git a/gcc/common.opt b/gcc/common.opt
> index a75b44e..ffb968d 100644
> --- a/gcc/common.opt
> +++ b/gcc/common.opt
> @@ -109,7 +109,7 @@ bool exit_after_options
>  ; flag-types.h for the definitions of the different possible types of
>  ; debugging information.
>  Variable
> -enum debug_info_type write_symbols = NO_DEBUG
> +uint32_t write_symbols = NO_DEBUG
>
>  ; Level of debugging information we are producing.  See flag-types.h
>  ; for the definitions of the different possible levels.
> diff --git a/gcc/flag-types.h b/gcc/flag-types.h
> index a038c8f..d60bb30 100644
> --- a/gcc/flag-types.h
> +++ b/gcc/flag-types.h
> @@ -24,15 +24,30 @@ along with GCC; see the file COPYING3.  If not see
>
>  enum debug_info_type
>  {
> -  NO_DEBUG,        /* Write no debug info.  */
> -  DBX_DEBUG,       /* Write BSD .stabs for DBX (using dbxout.c).  */
> -  DWARF2_DEBUG,            /* Write Dwarf v2 debug info (using dwarf2out.c).  */
> -  XCOFF_DEBUG,     /* Write IBM/Xcoff debug info (using dbxout.c).  */
> -  VMS_DEBUG,        /* Write VMS debug info (using vmsdbgout.c).  */
> -  VMS_AND_DWARF2_DEBUG /* Write VMS debug info (using vmsdbgout.c).
> -                          and DWARF v2 debug info (using dwarf2out.c).  */
> +  DINFO_TYPE_NONE = 0,           /* No debug info.  */
> +  DINFO_TYPE_DBX = 1,            /* BSD .stabs for DBX.  */
> +  DINFO_TYPE_DWARF2 = 2,         /* Dwarf v2 debug info.  */
> +  DINFO_TYPE_XCOFF = 3,                  /* IBM/Xcoff debug info.  */
> +  DINFO_TYPE_VMS = 4,            /* VMS debug info.  */
> +  DINFO_TYPE_MAX = DINFO_TYPE_VMS /* Marker only.  */
>  };
>
> +#define NO_DEBUG      (0U)
> +/* Write DBX debug info (using dbxout.c).  */
> +#define DBX_DEBUG     (1U << DINFO_TYPE_DBX)
> +/* Write DWARF2 debug info (using dwarf2out.c).  */
> +#define DWARF2_DEBUG  (1U << DINFO_TYPE_DWARF2)
> +/* Write IBM/XCOFF debug info (using dbxout.c).  */
> +#define XCOFF_DEBUG   (1U << DINFO_TYPE_XCOFF)
> +/* Write VMS debug info (using vmsdbgout.c).  */
> +#define VMS_DEBUG     (1U << DINFO_TYPE_VMS)
> +/* Note: Adding new definitions to handle -combination- of debug formats,
> +   like VMS_AND_DWARF2_DEBUG is not recommended.  This definition remains
> +   here for historical reasons.  */
> +/* Write VMS debug info (using vmsdbgout.c) and DWARF v2 debug info (using
> +   dwarf2out.c).  */
> +#define VMS_AND_DWARF2_DEBUG  ((VMS_DEBUG | DWARF2_DEBUG))
> +
>  enum debug_info_levels
>  {
>    DINFO_LEVEL_NONE,    /* Write no debugging info.  */
> diff --git a/gcc/flags.h b/gcc/flags.h
> index 0c4409e..3415493 100644
> --- a/gcc/flags.h
> +++ b/gcc/flags.h
> @@ -22,9 +22,24 @@ along with GCC; see the file COPYING3.  If not see
>
>  #if !defined(IN_LIBGCC2) && !defined(IN_TARGET_LIBS) && !defined(IN_RTS)
>
> -/* Names of debug_info_type, for error messages.  */
> +/* Names of fundamental debug info formats indexed by enum
> +   debug_info_type.  */
> +
>  extern const char *const debug_type_names[];
>
> +/* Get enum debug_info_type of the specified debug format, for error messages.
> +   Can be used only for individual debug format types.  */
> +
> +extern enum debug_info_type debug_set_to_format (uint32_t debug_info_set);
> +
> +/* Get the number of debug formats enabled for output.  */
> +
> +unsigned int debug_set_count (uint32_t w_symbols);
> +
> +/* Get the names of the debug formats enabled for output.  */
> +
> +const char * debug_set_names (uint32_t w_symbols);
> +
>  extern void strip_off_ending (char *, int);
>  extern int base_of_path (const char *path, const char **base_out);
>
> diff --git a/gcc/objc/objc-act.c b/gcc/objc/objc-act.c
> index 1cbd586..a5aa3c5 100644
> --- a/gcc/objc/objc-act.c
> +++ b/gcc/objc/objc-act.c
> @@ -3078,7 +3078,7 @@ static void
>  synth_module_prologue (void)
>  {
>    tree type;
> -  enum debug_info_type save_write_symbols = write_symbols;
> +  uint32_t save_write_symbols = write_symbols;
>    const struct gcc_debug_hooks *const save_hooks = debug_hooks;
>
>    /* Suppress outputting debug symbols, because
> diff --git a/gcc/opts.c b/gcc/opts.c
> index 24bb641..026952f 100644
> --- a/gcc/opts.c
> +++ b/gcc/opts.c
> @@ -37,12 +37,95 @@ along with GCC; see the file COPYING3.  If not see
>
>  static void set_Wstrict_aliasing (struct gcc_options *opts, int onoff);
>
> -/* Indexed by enum debug_info_type.  */
> +/* Names of fundamental debug info formats indexed by enum
> +   debug_info_type.  */
> +
>  const char *const debug_type_names[] =
>  {
>    "none", "stabs", "dwarf-2", "xcoff", "vms"
>  };
>
> +/* Bitmasks of fundamental debug info formats indexed by enum
> +   debug_info_type.  */
> +
> +static uint32_t debug_type_masks[] =
> +{
> +  NO_DEBUG, DBX_DEBUG, DWARF2_DEBUG, XCOFF_DEBUG, VMS_DEBUG
> +};
> +
> +/* Names of the set of debug formats requested by user.  Updated and accessed
> +   via debug_set_names.  */
> +
> +static char df_set_names[sizeof "none stabs dwarf-2 xcoff vms"];
> +
> +/* Get enum debug_info_type of the specified debug format, for error messages.
> +   Can be used only for individual debug format types.  */
> +
> +enum debug_info_type
> +debug_set_to_format (uint32_t debug_info_set)
> +{
> +  int idx = 0;
> +  enum debug_info_type dinfo_type = DINFO_TYPE_NONE;
> +  /* Find first set bit.  */
> +  if (debug_info_set)
> +    idx = exact_log2 (debug_info_set & - debug_info_set);
> +  /* Check that only one bit is set, if at all.  This function is meant to be
> +     used only for vanilla debug_info_set bitmask values, i.e. for individual
> +     debug format types upto DINFO_TYPE_MAX.  */
> +  gcc_assert ((debug_info_set & (debug_info_set - 1)) == 0);
> +  dinfo_type = (enum debug_info_type)idx;
> +  gcc_assert (dinfo_type <= DINFO_TYPE_MAX);
> +  return dinfo_type;
> +}
> +
> +/* Get the number of debug formats enabled for output.  */
> +
> +unsigned int
> +debug_set_count (uint32_t w_symbols)
> +{
> +  unsigned int count = 0;
> +  while (w_symbols)
> +    {
> +      ++ count;
> +      w_symbols &= ~ (w_symbols & - w_symbols);
> +    }
> +  return count;
> +}
> +
> +/* Get the names of the debug formats enabled for output.  */
> +
> +const char *
> +debug_set_names (uint32_t w_symbols)
> +{
> +  uint32_t df_mask = 0;
> +  /* Reset the string to be returned.  */
> +  memset (df_set_names, 0, sizeof (df_set_names));
> +  /* Get the popcount.  */
> +  int num_set_df = debug_set_count (w_symbols);
> +  /* Iterate over the debug formats.  Add name string for those enabled.  */
> +  for (int i = DINFO_TYPE_NONE; i <= DINFO_TYPE_MAX; i++)
> +    {
> +      df_mask = debug_type_masks[i];
> +      if (w_symbols & df_mask)
> +       {
> +         strcat (df_set_names, debug_type_names[i]);
> +         num_set_df--;
> +         if (num_set_df)
> +           strcat (df_set_names, " ");
> +         else
> +           break;
> +       }
> +      else if (!w_symbols)
> +       {
> +         /* No debug formats enabled.  */
> +         gcc_assert (i == DINFO_TYPE_NONE);
> +         strcat (df_set_names, debug_type_names[i]);
> +         break;
> +       }
> +    }
> +  return df_set_names;
> +}
> +
>  /* Parse the -femit-struct-debug-detailed option value
>     and set the flag variables. */
>
> @@ -190,7 +273,7 @@ static const char use_diagnosed_msg[] = N_("Uses of this option are diagnosed.")
>
>  typedef char *char_p; /* For DEF_VEC_P.  */
>
> -static void set_debug_level (enum debug_info_type type, int extended,
> +static void set_debug_level (uint32_t dinfo, int extended,
>                              const char *arg, struct gcc_options *opts,
>                              struct gcc_options *opts_set,
>                              location_t loc);
> @@ -3027,17 +3110,17 @@ fast_math_flags_struct_set_p (struct cl_optimization *opt)
>  }
>
>  /* Handle a debug output -g switch for options OPTS
> -   (OPTS_SET->x_write_symbols storing whether a debug type was passed
> +   (OPTS_SET->x_write_symbols storing whether a debug format was passed
>     explicitly), location LOC.  EXTENDED is true or false to support
>     extended output (2 is special and means "-ggdb" was given).  */
>  static void
> -set_debug_level (enum debug_info_type type, int extended, const char *arg,
> +set_debug_level (uint32_t dinfo, int extended, const char *arg,
>                  struct gcc_options *opts, struct gcc_options *opts_set,
>                  location_t loc)
>  {
>    opts->x_use_gnu_debug_info_extensions = extended;
>
> -  if (type == NO_DEBUG)
> +  if (dinfo == NO_DEBUG)
>      {
>        if (opts->x_write_symbols == NO_DEBUG)
>         {
> @@ -3058,14 +3141,18 @@ set_debug_level (enum debug_info_type type, int extended, const char *arg,
>      }
>    else
>      {
> -      /* Does it conflict with an already selected type?  */
> +      /* Does it conflict with an already selected debug format?  */
>        if (opts_set->x_write_symbols != NO_DEBUG
>           && opts->x_write_symbols != NO_DEBUG
> -         && type != opts->x_write_symbols)
> -       error_at (loc, "debug format %qs conflicts with prior selection",
> -                 debug_type_names[type]);
> -      opts->x_write_symbols = type;
> -      opts_set->x_write_symbols = type;
> +         && dinfo != opts->x_write_symbols)
> +       {
> +         gcc_assert (debug_set_count (dinfo) <= 1);
> +         error_at (loc, "debug format %qs conflicts with prior selection",
> +                   debug_type_names[debug_set_to_format (dinfo)]);
> +       }
> +
> +      opts->x_write_symbols = dinfo;
> +      opts_set->x_write_symbols = dinfo;
>      }
>
>    /* A debug flag without a level defaults to level 2.
> diff --git a/gcc/toplev.c b/gcc/toplev.c
> index d8cc254..a0ccec4 100644
> --- a/gcc/toplev.c
> +++ b/gcc/toplev.c
> @@ -1460,9 +1460,12 @@ process_options (void)
>      debug_hooks = &dwarf2_lineno_debug_hooks;
>  #endif
>    else
> -    error_at (UNKNOWN_LOCATION,
> -             "target system does not support the %qs debug format",
> -             debug_type_names[write_symbols]);
> +    {
> +      gcc_assert (debug_set_count (write_symbols) <= 1);
> +      error_at (UNKNOWN_LOCATION,
> +               "target system does not support the %qs debug format",
> +               debug_type_names[debug_set_to_format (write_symbols)]);
> +    }
>
>    /* We know which debug output will be used so we can set flag_var_tracking
>       and flag_var_tracking_uninit if the user has not specified them.  */
> --
> 1.8.3.1
>

Re: [PATCH 1/2] opts: change write_symbols to support bitmasks

[Indu Bhagat]

On 5/10/21 6:11 AM, Richard Biener wrote:
> On Thu, May 6, 2021 at 2:31 AM Indu Bhagat via Gcc-patches
> <gcc-patches@gcc.gnu.org> wrote:
>>
>> To support multiple debug formats, we need to move away from explicit
>> enumeration of each individual combination of debug formats.
> 
> debug_set_names with its static buffer seems unused?  You wire quite some
> APIs with gcc_assert on having a single bit set - that doesn't look forward
> looking.
> 
> I suppose the BTF followups will "fix" this, but see comments below.

Thanks for your feedback.

Yes, I intended to fix them when I added the CTF/BTF as then I would 
have a way to debug/test it more meaningfully. Because of this, the 
debug_set_names and the associated static buffer were unused in V1 indeed.

For V2, taking your inputs, I have now fixed the uses in c-opts.c and 
c-pch.c at least.

> 
>> gcc/c-family/ChangeLog:
>>
>>          * c-opts.c (c_common_post_options): Adjust access to debug_type_names.
>>          * c-pch.c (struct c_pch_validity): Use type uint32_t.
>>          (pch_init): Renamed member.
>>          (c_common_valid_pch): Adjust access to debug_type_names.
>>
>> gcc/ChangeLog:
>>
>>          * common.opt: Change type to support bitmasks.
>>          * flag-types.h (enum debug_info_type): Rename enumerator constants.
>>          (NO_DEBUG): New bitmask.
>>          (DBX_DEBUG): Likewise.
>>          (DWARF2_DEBUG): Likewise.
>>          (XCOFF_DEBUG): Likewise.
>>          (VMS_DEBUG): Likewise.
>>          (VMS_AND_DWARF2_DEBUG): Likewise.
>>          * flags.h (debug_set_to_format): New function declaration.
>>          (debug_set_count): Likewise.
>>          (debug_set_names): Likewise.
>>          * opts.c (debug_type_masks): Array of bitmasks for debug formats.
>>          (debug_set_to_format): New function definition.
>>          (debug_set_count): Likewise.
>>          (debug_set_names): Likewise.
>>          (set_debug_level): Update access to debug_type_names.
>>          * toplev.c: Likewise.
>>
>> gcc/objc/ChangeLog:
>>
>>          * objc-act.c (synth_module_prologue): Use uint32_t instead of enum
>>          debug_info_type.
>> ---
>>   gcc/c-family/c-opts.c |  10 +++--
>>   gcc/c-family/c-pch.c  |  12 +++---
>>   gcc/common.opt        |   2 +-
>>   gcc/flag-types.h      |  29 ++++++++++----
>>   gcc/flags.h           |  17 +++++++-
>>   gcc/objc/objc-act.c   |   2 +-
>>   gcc/opts.c            | 109 +++++++++++++++++++++++++++++++++++++++++++++-----
>>   gcc/toplev.c          |   9 +++--
>>   8 files changed, 158 insertions(+), 32 deletions(-)
>>
>> diff --git a/gcc/c-family/c-opts.c b/gcc/c-family/c-opts.c
>> index 89e05a4..e463240 100644
>> --- a/gcc/c-family/c-opts.c
>> +++ b/gcc/c-family/c-opts.c
>> @@ -1112,9 +1112,13 @@ c_common_post_options (const char **pfilename)
>>            /* Only -g0 and -gdwarf* are supported with PCH, for other
>>               debug formats we warn here and refuse to load any PCH files.  */
>>            if (write_symbols != NO_DEBUG && write_symbols != DWARF2_DEBUG)
>> -           warning (OPT_Wdeprecated,
>> -                    "the %qs debug format cannot be used with "
>> -                    "pre-compiled headers", debug_type_names[write_symbols]);
>> +           {
>> +             gcc_assert (debug_set_count (write_symbols) <= 1);
> 
> Why this assert?  Iff then simply include the count check in the
> condition of the warning.
> 
>> +             warning (OPT_Wdeprecated,
>> +                      "the %qs debug format cannot be used with "
>> +                      "pre-compiled headers",
>> +                      debug_type_names[debug_set_to_format (write_symbols)]);
> 
> Maybe simply emit another diagnostic if debug_set_count > 1.
> 

OK. I have removed the asserts. The code now uses debug_set_names 
uniformly. I have changed the diagnostic message, as there can be 
multiple debug formats for PCH at some point. So,

from "the 'XXX' debug format cannot be used with pre-compiled headers"
to   "the 'XXX YYY' debug info cannot be used with pre-compiled headers"
if multiple debug formats were enabled.

>> +           }
>>          }
>>         else if (write_symbols != NO_DEBUG && write_symbols != DWARF2_DEBUG)
>>          c_common_no_more_pch ();
>> diff --git a/gcc/c-family/c-pch.c b/gcc/c-family/c-pch.c
>> index fd94c37..6804388 100644
>> --- a/gcc/c-family/c-pch.c
>> +++ b/gcc/c-family/c-pch.c
>> @@ -52,7 +52,7 @@ enum {
>>
>>   struct c_pch_validity
>>   {
>> -  unsigned char debug_info_type;
>> +  uint32_t pch_write_symbols;
>>     signed char match[MATCH_SIZE];
>>     void (*pch_init) (void);
>>     size_t target_data_length;
>> @@ -108,7 +108,7 @@ pch_init (void)
>>     pch_outfile = f;
>>
>>     memset (&v, '\0', sizeof (v));
>> -  v.debug_info_type = write_symbols;
>> +  v.pch_write_symbols = write_symbols;
>>     {
>>       size_t i;
>>       for (i = 0; i < MATCH_SIZE; i++)
>> @@ -252,13 +252,15 @@ c_common_valid_pch (cpp_reader *pfile, const char *name, int fd)
>>     /* The allowable debug info combinations are that either the PCH file
>>        was built with the same as is being used now, or the PCH file was
>>        built for some kind of debug info but now none is in use.  */
>> -  if (v.debug_info_type != write_symbols
>> +  if (v.pch_write_symbols != write_symbols
>>         && write_symbols != NO_DEBUG)
>>       {
>> +      gcc_assert (debug_set_count (v.pch_write_symbols) <= 1);
>> +      gcc_assert (debug_set_count (write_symbols) <= 1);
> 
> So the read-in PCH will have at most one bit set but I don't think
> you can assert on write_symbols here.
> 

I switched both to use debug_set_names.

> Otherwise looks OK.  Did you check for write_symbols uses in FEs and targets?
> 
> Richard.

Yes, I have. I must admit I have gone back and forth in my mind on this. 
My initial thinking was to adjust only those checks where I expect more 
than 1 write_symbols. Because in most contexts, e.g., (write_symbols == 
XCOFF_DEBUG) is a stricter check than (write_symbols & XCOFF_DEBUG), in 
some sense.

Anyway, after you raised this point, however, I looked again, and have 
added some dwarf_debuginfo_p usages in the second patch. Basically, I 
replaced some write_symbols == DWARF2_DEBUG with dwarf_debuginfo_p () 
usage. Other than DWARF2_DEBUG, at this time, no other debug format is 
expected to be written out together with other debug formats.

As for other usages of write_symbols
  - I plan to get rid of the VMS_AND_DWARF2_DEBUG symbol in a subsequent 
patch (this should deal with most usages in vmsdbgout.c).

I will post V2 next.

A question though - I have regression tested the V2 patch set on x86_64. 
Although the sort of changes in the backend files can be OK, I am happy 
to take any further suggestions for testing this patch set.

Thanks
Indu

Re: [PATCH 1/2] opts: change write_symbols to support bitmasks

[Richard Biener]

On Tue, May 11, 2021 at 4:45 PM Indu Bhagat <indu.bhagat@oracle.com> wrote:
>
> On 5/10/21 6:11 AM, Richard Biener wrote:
> > On Thu, May 6, 2021 at 2:31 AM Indu Bhagat via Gcc-patches
> > <gcc-patches@gcc.gnu.org> wrote:
> >>
> >> To support multiple debug formats, we need to move away from explicit
> >> enumeration of each individual combination of debug formats.
> >
> > debug_set_names with its static buffer seems unused?  You wire quite some
> > APIs with gcc_assert on having a single bit set - that doesn't look forward
> > looking.
> >
> > I suppose the BTF followups will "fix" this, but see comments below.
>
> Thanks for your feedback.
>
> Yes, I intended to fix them when I added the CTF/BTF as then I would
> have a way to debug/test it more meaningfully. Because of this, the
> debug_set_names and the associated static buffer were unused in V1 indeed.
>
> For V2, taking your inputs, I have now fixed the uses in c-opts.c and
> c-pch.c at least.
>
> >
> >> gcc/c-family/ChangeLog:
> >>
> >>          * c-opts.c (c_common_post_options): Adjust access to debug_type_names.
> >>          * c-pch.c (struct c_pch_validity): Use type uint32_t.
> >>          (pch_init): Renamed member.
> >>          (c_common_valid_pch): Adjust access to debug_type_names.
> >>
> >> gcc/ChangeLog:
> >>
> >>          * common.opt: Change type to support bitmasks.
> >>          * flag-types.h (enum debug_info_type): Rename enumerator constants.
> >>          (NO_DEBUG): New bitmask.
> >>          (DBX_DEBUG): Likewise.
> >>          (DWARF2_DEBUG): Likewise.
> >>          (XCOFF_DEBUG): Likewise.
> >>          (VMS_DEBUG): Likewise.
> >>          (VMS_AND_DWARF2_DEBUG): Likewise.
> >>          * flags.h (debug_set_to_format): New function declaration.
> >>          (debug_set_count): Likewise.
> >>          (debug_set_names): Likewise.
> >>          * opts.c (debug_type_masks): Array of bitmasks for debug formats.
> >>          (debug_set_to_format): New function definition.
> >>          (debug_set_count): Likewise.
> >>          (debug_set_names): Likewise.
> >>          (set_debug_level): Update access to debug_type_names.
> >>          * toplev.c: Likewise.
> >>
> >> gcc/objc/ChangeLog:
> >>
> >>          * objc-act.c (synth_module_prologue): Use uint32_t instead of enum
> >>          debug_info_type.
> >> ---
> >>   gcc/c-family/c-opts.c |  10 +++--
> >>   gcc/c-family/c-pch.c  |  12 +++---
> >>   gcc/common.opt        |   2 +-
> >>   gcc/flag-types.h      |  29 ++++++++++----
> >>   gcc/flags.h           |  17 +++++++-
> >>   gcc/objc/objc-act.c   |   2 +-
> >>   gcc/opts.c            | 109 +++++++++++++++++++++++++++++++++++++++++++++-----
> >>   gcc/toplev.c          |   9 +++--
> >>   8 files changed, 158 insertions(+), 32 deletions(-)
> >>
> >> diff --git a/gcc/c-family/c-opts.c b/gcc/c-family/c-opts.c
> >> index 89e05a4..e463240 100644
> >> --- a/gcc/c-family/c-opts.c
> >> +++ b/gcc/c-family/c-opts.c
> >> @@ -1112,9 +1112,13 @@ c_common_post_options (const char **pfilename)
> >>            /* Only -g0 and -gdwarf* are supported with PCH, for other
> >>               debug formats we warn here and refuse to load any PCH files.  */
> >>            if (write_symbols != NO_DEBUG && write_symbols != DWARF2_DEBUG)
> >> -           warning (OPT_Wdeprecated,
> >> -                    "the %qs debug format cannot be used with "
> >> -                    "pre-compiled headers", debug_type_names[write_symbols]);
> >> +           {
> >> +             gcc_assert (debug_set_count (write_symbols) <= 1);
> >
> > Why this assert?  Iff then simply include the count check in the
> > condition of the warning.
> >
> >> +             warning (OPT_Wdeprecated,
> >> +                      "the %qs debug format cannot be used with "
> >> +                      "pre-compiled headers",
> >> +                      debug_type_names[debug_set_to_format (write_symbols)]);
> >
> > Maybe simply emit another diagnostic if debug_set_count > 1.
> >
>
> OK. I have removed the asserts. The code now uses debug_set_names
> uniformly. I have changed the diagnostic message, as there can be
> multiple debug formats for PCH at some point. So,
>
> from "the 'XXX' debug format cannot be used with pre-compiled headers"
> to   "the 'XXX YYY' debug info cannot be used with pre-compiled headers"
> if multiple debug formats were enabled.
>
> >> +           }
> >>          }
> >>         else if (write_symbols != NO_DEBUG && write_symbols != DWARF2_DEBUG)
> >>          c_common_no_more_pch ();
> >> diff --git a/gcc/c-family/c-pch.c b/gcc/c-family/c-pch.c
> >> index fd94c37..6804388 100644
> >> --- a/gcc/c-family/c-pch.c
> >> +++ b/gcc/c-family/c-pch.c
> >> @@ -52,7 +52,7 @@ enum {
> >>
> >>   struct c_pch_validity
> >>   {
> >> -  unsigned char debug_info_type;
> >> +  uint32_t pch_write_symbols;
> >>     signed char match[MATCH_SIZE];
> >>     void (*pch_init) (void);
> >>     size_t target_data_length;
> >> @@ -108,7 +108,7 @@ pch_init (void)
> >>     pch_outfile = f;
> >>
> >>     memset (&v, '\0', sizeof (v));
> >> -  v.debug_info_type = write_symbols;
> >> +  v.pch_write_symbols = write_symbols;
> >>     {
> >>       size_t i;
> >>       for (i = 0; i < MATCH_SIZE; i++)
> >> @@ -252,13 +252,15 @@ c_common_valid_pch (cpp_reader *pfile, const char *name, int fd)
> >>     /* The allowable debug info combinations are that either the PCH file
> >>        was built with the same as is being used now, or the PCH file was
> >>        built for some kind of debug info but now none is in use.  */
> >> -  if (v.debug_info_type != write_symbols
> >> +  if (v.pch_write_symbols != write_symbols
> >>         && write_symbols != NO_DEBUG)
> >>       {
> >> +      gcc_assert (debug_set_count (v.pch_write_symbols) <= 1);
> >> +      gcc_assert (debug_set_count (write_symbols) <= 1);
> >
> > So the read-in PCH will have at most one bit set but I don't think
> > you can assert on write_symbols here.
> >
>
> I switched both to use debug_set_names.
>
> > Otherwise looks OK.  Did you check for write_symbols uses in FEs and targets?
> >
> > Richard.
>
> Yes, I have. I must admit I have gone back and forth in my mind on this.
> My initial thinking was to adjust only those checks where I expect more
> than 1 write_symbols. Because in most contexts, e.g., (write_symbols ==
> XCOFF_DEBUG) is a stricter check than (write_symbols & XCOFF_DEBUG), in
> some sense.
>
> Anyway, after you raised this point, however, I looked again, and have
> added some dwarf_debuginfo_p usages in the second patch. Basically, I
> replaced some write_symbols == DWARF2_DEBUG with dwarf_debuginfo_p ()
> usage. Other than DWARF2_DEBUG, at this time, no other debug format is
> expected to be written out together with other debug formats.
>
> As for other usages of write_symbols
>   - I plan to get rid of the VMS_AND_DWARF2_DEBUG symbol in a subsequent
> patch (this should deal with most usages in vmsdbgout.c).
>
> I will post V2 next.
>
> A question though - I have regression tested the V2 patch set on x86_64.
> Although the sort of changes in the backend files can be OK, I am happy
> to take any further suggestions for testing this patch set.

If you touch backends in non-trivial ways it's at least good to test it
still builds in a simple cross configuration which means
configuring for one of the supported target triplets and doing
make all-gcc (that only builds host objects so you don't need
binutils or target headers for this)

Richard.

> Thanks
> Indu

Re: [PATCH 1/2] opts: change write_symbols to support bitmasks

[Indu Bhagat]

On 5/12/21 12:31 AM, Richard Biener wrote:
>>> Otherwise looks OK.  Did you check for write_symbols uses in FEs and targets?
>>>
>>> Richard.
>> Yes, I have. I must admit I have gone back and forth in my mind on this.
>> My initial thinking was to adjust only those checks where I expect more
>> than 1 write_symbols. Because in most contexts, e.g., (write_symbols ==
>> XCOFF_DEBUG) is a stricter check than (write_symbols & XCOFF_DEBUG), in
>> some sense.
>>
>> Anyway, after you raised this point, however, I looked again, and have
>> added some dwarf_debuginfo_p usages in the second patch. Basically, I
>> replaced some write_symbols == DWARF2_DEBUG with dwarf_debuginfo_p ()
>> usage. Other than DWARF2_DEBUG, at this time, no other debug format is
>> expected to be written out together with other debug formats.
>>
>> As for other usages of write_symbols
>>    - I plan to get rid of the VMS_AND_DWARF2_DEBUG symbol in a subsequent
>> patch (this should deal with most usages in vmsdbgout.c).
>>
>> I will post V2 next.
>>
>> A question though - I have regression tested the V2 patch set on x86_64.
>> Although the sort of changes in the backend files can be OK, I am happy
>> to take any further suggestions for testing this patch set.
> If you touch backends in non-trivial ways it's at least good to test it
> still builds in a simple cross configuration which means
> configuring for one of the supported target triplets and doing
> make all-gcc (that only builds host objects so you don't need
> binutils or target headers for this)
> 
> Richard.
> 

Thanks. I tested the patch set for select triplets containing 
c6x/mips/powerpc and cygwin/darwin. Found and fixed the build issues.

Will be posting V3 next.

Thanks
Indu

[PATCH 2/2] dwarf: new dwarf_debuginfo_p predicate

[Indu Bhagat]

This patch introduces a dwarf_debuginfo_p predicate that abstracts and
replaces complex checks on write_symbols.

gcc/c-family/ChangeLog:

	* c-lex.c (init_c_lex): Use dwarf_debuginfo_p.

gcc/ChangeLog:

	* config/c6x/c6x.c (c6x_output_file_unwind): Use dwarf_debuginfo_p.
	* dwarf2cfi.c (cfi_label_required_p): Likewise.
	(dwarf2out_do_frame): Likewise.
	* final.c (dwarf2_debug_info_emitted_p): Likewise.
	(final_scan_insn_1): Likewise.
	* flags.h (dwarf_debuginfo_p): New function declaration.
	* opts.c (dwarf_debuginfo_p): New function definition.
	* targhooks.c (default_debug_unwind_info): Use dwarf_debuginfo_p.
	* toplev.c (process_options): Likewise.
---
 gcc/c-family/c-lex.c |  4 ++--
 gcc/config/c6x/c6x.c |  3 +--
 gcc/dwarf2cfi.c      |  9 ++++-----
 gcc/final.c          | 15 ++++++---------
 gcc/flags.h          |  4 ++++
 gcc/opts.c           |  8 ++++++++
 gcc/targhooks.c      |  2 +-
 gcc/toplev.c         |  6 ++----
 8 files changed, 28 insertions(+), 23 deletions(-)

diff --git a/gcc/c-family/c-lex.c b/gcc/c-family/c-lex.c
index 6374b72..5174b22 100644
--- a/gcc/c-family/c-lex.c
+++ b/gcc/c-family/c-lex.c
@@ -27,6 +27,7 @@ along with GCC; see the file COPYING3.  If not see
 #include "stor-layout.h"
 #include "c-pragma.h"
 #include "debug.h"
+#include "flags.h"
 #include "file-prefix-map.h" /* remap_macro_filename()  */
 #include "langhooks.h"
 #include "attribs.h"
@@ -87,8 +88,7 @@ init_c_lex (void)
 
   /* Set the debug callbacks if we can use them.  */
   if ((debug_info_level == DINFO_LEVEL_VERBOSE
-       && (write_symbols == DWARF2_DEBUG
-	   || write_symbols == VMS_AND_DWARF2_DEBUG))
+       && dwarf_debuginfo_p ())
       || flag_dump_go_spec != NULL)
     {
       cb->define = cb_define;
diff --git a/gcc/config/c6x/c6x.c b/gcc/config/c6x/c6x.c
index f9ad1e5..a10e2f8 100644
--- a/gcc/config/c6x/c6x.c
+++ b/gcc/config/c6x/c6x.c
@@ -439,8 +439,7 @@ c6x_output_file_unwind (FILE * f)
     {
       if (flag_unwind_tables || flag_exceptions)
 	{
-	  if (write_symbols == DWARF2_DEBUG
-	      || write_symbols == VMS_AND_DWARF2_DEBUG)
+	  if (dwarf_debuginfo_p ())
 	    asm_fprintf (f, "\t.cfi_sections .debug_frame, .c6xabi.exidx\n");
 	  else
 	    asm_fprintf (f, "\t.cfi_sections .c6xabi.exidx\n");
diff --git a/gcc/dwarf2cfi.c b/gcc/dwarf2cfi.c
index 362ff3f..c27ac19 100644
--- a/gcc/dwarf2cfi.c
+++ b/gcc/dwarf2cfi.c
@@ -39,7 +39,7 @@ along with GCC; see the file COPYING3.  If not see
 #include "expr.h"		/* init_return_column_size */
 #include "output.h"		/* asm_out_file */
 #include "debug.h"		/* dwarf2out_do_frame, dwarf2out_do_cfi_asm */
-
+#include "flags.h"		/* dwarf_debuginfo_p */
 
 /* ??? Poison these here until it can be done generically.  They've been
    totally replaced in this file; make sure it stays that way.  */
@@ -2289,8 +2289,7 @@ cfi_label_required_p (dw_cfi_ref cfi)
 
   if (dwarf_version == 2
       && debug_info_level > DINFO_LEVEL_TERSE
-      && (write_symbols == DWARF2_DEBUG
-	  || write_symbols == VMS_AND_DWARF2_DEBUG))
+      && dwarf_debuginfo_p ())
     {
       switch (cfi->dw_cfi_opc)
 	{
@@ -3557,9 +3556,9 @@ bool
 dwarf2out_do_frame (void)
 {
   /* We want to emit correct CFA location expressions or lists, so we
-     have to return true if we're going to output debug info, even if
+     have to return true if we're going to generate debug info, even if
      we're not going to output frame or unwind info.  */
-  if (write_symbols == DWARF2_DEBUG || write_symbols == VMS_AND_DWARF2_DEBUG)
+  if (dwarf_debuginfo_p ())
     return true;
 
   if (saved_do_cfi_asm > 0)
diff --git a/gcc/final.c b/gcc/final.c
index ba4285d..794702f 100644
--- a/gcc/final.c
+++ b/gcc/final.c
@@ -1428,7 +1428,8 @@ asm_str_count (const char *templ)
 static bool
 dwarf2_debug_info_emitted_p (tree decl)
 {
-  if (write_symbols != DWARF2_DEBUG && write_symbols != VMS_AND_DWARF2_DEBUG)
+  /* When DWARF2 debug info is not generated internally.  */
+  if (!dwarf_debuginfo_p ())
     return false;
 
   if (DECL_IGNORED_P (decl))
@@ -2298,10 +2299,8 @@ final_scan_insn_1 (rtx_insn *insn, FILE *file, int optimize_p ATTRIBUTE_UNUSED,
 	  break;
 
 	case NOTE_INSN_BLOCK_BEG:
-	  if (debug_info_level == DINFO_LEVEL_NORMAL
-	      || debug_info_level == DINFO_LEVEL_VERBOSE
-	      || write_symbols == DWARF2_DEBUG
-	      || write_symbols == VMS_AND_DWARF2_DEBUG
+	  if (debug_info_level >= DINFO_LEVEL_NORMAL
+	      || dwarf_debuginfo_p ()
 	      || write_symbols == VMS_DEBUG)
 	    {
 	      int n = BLOCK_NUMBER (NOTE_BLOCK (insn));
@@ -2336,10 +2335,8 @@ final_scan_insn_1 (rtx_insn *insn, FILE *file, int optimize_p ATTRIBUTE_UNUSED,
 	case NOTE_INSN_BLOCK_END:
 	  maybe_output_next_view (seen);
 
-	  if (debug_info_level == DINFO_LEVEL_NORMAL
-	      || debug_info_level == DINFO_LEVEL_VERBOSE
-	      || write_symbols == DWARF2_DEBUG
-	      || write_symbols == VMS_AND_DWARF2_DEBUG
+	  if (debug_info_level >= DINFO_LEVEL_NORMAL
+	      || dwarf_debuginfo_p ()
 	      || write_symbols == VMS_DEBUG)
 	    {
 	      int n = BLOCK_NUMBER (NOTE_BLOCK (insn));
diff --git a/gcc/flags.h b/gcc/flags.h
index 3415493..cc7b79b 100644
--- a/gcc/flags.h
+++ b/gcc/flags.h
@@ -40,6 +40,10 @@ unsigned int debug_set_count (uint32_t w_symbols);
 
 const char * debug_set_names (uint32_t w_symbols);
 
+/* Return true iff DWARF2 debug info is enabled.  */
+
+extern bool dwarf_debuginfo_p ();
+
 extern void strip_off_ending (char *, int);
 extern int base_of_path (const char *path, const char **base_out);
 
diff --git a/gcc/opts.c b/gcc/opts.c
index 026952f..3e0b046 100644
--- a/gcc/opts.c
+++ b/gcc/opts.c
@@ -126,6 +126,14 @@ debug_set_names (uint32_t w_symbols)
   return df_set_names;
 }
 
+/* Return TRUE iff dwarf2 debug info is enabled.  */
+
+bool
+dwarf_debuginfo_p ()
+{
+  return (write_symbols & DWARF2_DEBUG);
+}
+
 /* Parse the -femit-struct-debug-detailed option value
    and set the flag variables. */
 
diff --git a/gcc/targhooks.c b/gcc/targhooks.c
index 952fad4..267ba47 100644
--- a/gcc/targhooks.c
+++ b/gcc/targhooks.c
@@ -1975,7 +1975,7 @@ default_debug_unwind_info (void)
 
   /* Otherwise, only turn it on if dwarf2 debugging is enabled.  */
 #ifdef DWARF2_DEBUGGING_INFO
-  if (write_symbols == DWARF2_DEBUG || write_symbols == VMS_AND_DWARF2_DEBUG)
+  if (dwarf_debuginfo_p ())
     return UI_DWARF2;
 #endif
 
diff --git a/gcc/toplev.c b/gcc/toplev.c
index a0ccec4..0e11d6a 100644
--- a/gcc/toplev.c
+++ b/gcc/toplev.c
@@ -1526,8 +1526,7 @@ process_options (void)
     debug_nonbind_markers_p
       = (optimize
 	 && debug_info_level >= DINFO_LEVEL_NORMAL
-	 && (write_symbols == DWARF2_DEBUG
-	     || write_symbols == VMS_AND_DWARF2_DEBUG)
+	 && dwarf_debuginfo_p ()
 	 && !(flag_selective_scheduling || flag_selective_scheduling2));
 
   if (dwarf2out_as_loc_support == AUTODETECT_VALUE)
@@ -1542,8 +1541,7 @@ process_options (void)
       debug_variable_location_views
 	= (flag_var_tracking
 	   && debug_info_level >= DINFO_LEVEL_NORMAL
-	   && (write_symbols == DWARF2_DEBUG
-	       || write_symbols == VMS_AND_DWARF2_DEBUG)
+	   && dwarf_debuginfo_p ()
 	   && !dwarf_strict
 	   && dwarf2out_as_loc_support
 	   && dwarf2out_as_locview_support);
-- 
1.8.3.1