diff mbox

make RTL/TREE/IPA dump kind an index

Message ID 9eb0ff26-4b3a-a1ab-c0ae-6e39639ab179@acm.org
State New
Headers show

Commit Message

Nathan Sidwell May 9, 2017, 7 p.m. UTC 
Currently, the TDF_foo flags serve 3 purposes:
1) what kind of dump
2) how detailed to print it
3) auxiliary message control

This addresses #1, which currently uses a bit mask of 
TDF_{TREE,RTL,IPA}, of which exactly one must be set.  The patch changes 
things so that these are now an index value (I hesitate to say 
enumeration, because they're still raw ints).  A TDF_KIND(X) accessor 
extracts this value.  (I left the spare bit between the TDF_KIND_MASK 
and TDF_ADDRESS for the moment.)

In addition I added 'TDF_LANG' for language-specific dump control, of 
which -fdump-translation-unit and -fdump-class-hierarchy become.  And 
can also be controlled by -fdump-lang-all. (rather than -fdump-tree-all)

Next move will be to move -fdump-class-hierarchy into a more generic 
structure (and -fdump-translation-unit, if my patch to remove it is not 
accepted).

ok?

nathan

Comments

Richard Biener May 10, 2017, 9:05 a.m. UTC | #1 
On Tue, May 9, 2017 at 9:00 PM, Nathan Sidwell <nathan@acm.org> wrote:
> Currently, the TDF_foo flags serve 3 purposes:
> 1) what kind of dump
> 2) how detailed to print it
> 3) auxiliary message control
>
> This addresses #1, which currently uses a bit mask of TDF_{TREE,RTL,IPA}, of
> which exactly one must be set.  The patch changes things so that these are
> now an index value (I hesitate to say enumeration, because they're still raw
> ints).  A TDF_KIND(X) accessor extracts this value.  (I left the spare bit
> between the TDF_KIND_MASK and TDF_ADDRESS for the moment.)
>
> In addition I added 'TDF_LANG' for language-specific dump control, of which
> -fdump-translation-unit and -fdump-class-hierarchy become.  And can also be
> controlled by -fdump-lang-all. (rather than -fdump-tree-all)
>
> Next move will be to move -fdump-class-hierarchy into a more generic
> structure (and -fdump-translation-unit, if my patch to remove it is not
> accepted).
>
> ok?

   TDI_nested,                  /* dump each function after unnesting it */
+
+  TDI_lang_all,                        /* enable all the language dumps.  */

extra vertical space

+
+#define TDF_ADDRESS    (1 << 3)        /* dump node addresses */

this leaves 1 << 2 unused.

Otherwise looks like a great cleanup.  You might want to coordinate with
Martin a bit here.  It also looks like with this we can start re-using
bits when they are restricted to one TDF_KIND.

Thanks,
Richard.

>
> nathan
> --
> Nathan Sidwell
Nathan Sidwell May 10, 2017, 11:40 a.m. UTC | #2 
On 05/10/2017 05:05 AM, Richard Biener wrote:
> On Tue, May 9, 2017 at 9:00 PM, Nathan Sidwell <nathan@acm.org> wrote:

> +
> +#define TDF_ADDRESS    (1 << 3)        /* dump node addresses */
> 
> this leaves 1 << 2 unused.

Yes, that was intentional (though I suspect my note about it was 
hidden).  As you say, I expect further cleanup and didn't want 
gratuitous churn.  I'll add a comment about bit 2 being free.

> Otherwise looks like a great cleanup.  You might want to coordinate with
> Martin a bit here.  It also looks like with this we can start re-using
> bits when they are restricted to one TDF_KIND.

Indeed, we coordinated a bit yesterday.  Thanks for review!

nathan
Rainer Orth May 11, 2017, 12:28 p.m. UTC | #3 
Hi Nathan,

> On 05/10/2017 05:05 AM, Richard Biener wrote:
>> On Tue, May 9, 2017 at 9:00 PM, Nathan Sidwell <nathan@acm.org> wrote:
>
>> +
>> +#define TDF_ADDRESS    (1 << 3)        /* dump node addresses */
>>
>> this leaves 1 << 2 unused.
>
> Yes, that was intentional (though I suspect my note about it was hidden).
> As you say, I expect further cleanup and didn't want gratuitous churn.
> I'll add a comment about bit 2 being free.
>
>> Otherwise looks like a great cleanup.  You might want to coordinate with
>> Martin a bit here.  It also looks like with this we can start re-using
>> bits when they are restricted to one TDF_KIND.
>
> Indeed, we coordinated a bit yesterday.  Thanks for review!

it seems your patch caused

UNRESOLVED: g++.dg/inherit/covariant7.C  -std=gnu++11  scan-tree-dump class "24    .*c6::_ZTcv0_n16_v0_n12_NV2c62f2Ev"
UNRESOLVED: g++.dg/inherit/covariant7.C  -std=gnu++11  scan-tree-dump class "28    .*c6::f2"
UNRESOLVED: g++.dg/inherit/covariant7.C  -std=gnu++11  scan-tree-dump class "80    .*0"
UNRESOLVED: g++.dg/inherit/covariant7.C  -std=gnu++11  scan-tree-dump class "84    .*c6::_ZTv0_n16_NV2c62f2Ev"
UNRESOLVED: g++.dg/inherit/covariant7.C  -std=gnu++14  scan-tree-dump class "24    .*c6::_ZTcv0_n16_v0_n12_NV2c62f2Ev"
UNRESOLVED: g++.dg/inherit/covariant7.C  -std=gnu++14  scan-tree-dump class "28    .*c6::f2"
UNRESOLVED: g++.dg/inherit/covariant7.C  -std=gnu++14  scan-tree-dump class "80    .*0"
UNRESOLVED: g++.dg/inherit/covariant7.C  -std=gnu++14  scan-tree-dump class "84    .*c6::_ZTv0_n16_NV2c62f2Ev"
UNRESOLVED: g++.dg/inherit/covariant7.C  -std=gnu++98  scan-tree-dump class "24    .*c6::_ZTcv0_n16_v0_n12_NV2c62f2Ev"
UNRESOLVED: g++.dg/inherit/covariant7.C  -std=gnu++98  scan-tree-dump class "28    .*c6::f2"
UNRESOLVED: g++.dg/inherit/covariant7.C  -std=gnu++98  scan-tree-dump class "80    .*0"
UNRESOLVED: g++.dg/inherit/covariant7.C  -std=gnu++98  scan-tree-dump class "84    .*c6::_ZTv0_n16_NV2c62f2Ev"

On the gcc-7 branch, the dump file (from -fdump-class-hierarchy) was
called covariant7.C.002t.class, now it's covariant7.C.002l.class.  But
gcc/testsuite/lib/scantree.exp (scan-tree-dump) expects the NNNt.<dump>
form.

	Rainer
Nathan Sidwell May 11, 2017, 12:37 p.m. UTC | #4 
On 05/11/2017 08:28 AM, Rainer Orth wrote:
> Hi Nathan,

> UNRESOLVED: g++.dg/inherit/covariant7.C  -std=gnu++98  scan-tree-dump class "24    .*c6::_ZTcv0_n16_v0_n12_NV2c62f2Ev"
> UNRESOLVED: g++.dg/inherit/covariant7.C  -std=gnu++98  scan-tree-dump class "28    .*c6::f2"
> UNRESOLVED: g++.dg/inherit/covariant7.C  -std=gnu++98  scan-tree-dump class "80    .*0"
> UNRESOLVED: g++.dg/inherit/covariant7.C  -std=gnu++98  scan-tree-dump class "84    .*c6::_ZTv0_n16_NV2c62f2Ev"
> 
> On the gcc-7 branch, the dump file (from -fdump-class-hierarchy) was
> called covariant7.C.002t.class, now it's covariant7.C.002l.class.  But
> gcc/testsuite/lib/scantree.exp (scan-tree-dump) expects the NNNt.<dump>
> form.

oh crap, I checked for new FAILS not UNRESOLVED.  sorry about that.

nathan
diff mbox

Patch

2017-05-09  Nathan Sidwell  <nathan@acm.org>

	* dumpfile.h (TDI_lang_all): New.
	(TDF_KIND): New. Renumber others
	(TDF_LANG, TDF_TREE, TDF_RTL, TDF_IPA): Enumerate value, rather
	than bits.
	* dumpfile.c (dump_files): Mark language dumps as TDF_LANG.  add
	lang-all.
	(get_dump_file_name): Adjust suffix generation.
	(dump_enable_all): Use TDF_KIND.
	* doc/invoke.texi (-fdump-lang-all): Document.

Index: doc/invoke.texi
===================================================================
--- doc/invoke.texi	(revision 247809)
+++ doc/invoke.texi	(working copy)
@@ -543,6 +543,7 @@  Objective-C and Objective-C++ Dialects}.
 -fdump-class-hierarchy@r{[}-@var{n}@r{]} @gol
 -fdump-final-insns@r{[}=@var{file}@r{]}
 -fdump-ipa-all  -fdump-ipa-cgraph  -fdump-ipa-inline @gol
+-fdump-lang-all @gol
 -fdump-passes @gol
 -fdump-rtl-@var{pass}  -fdump-rtl-@var{pass}=@var{filename} @gol
 -fdump-statistics @gol
@@ -12970,6 +12971,10 @@  Dump after function inlining.
 
 @end table
 
+@item -fdump-lang-all
+@opindex fdump-lang-all
+Control the dumping of language-specific information.
+
 @item -fdump-passes
 @opindex fdump-passes
 Print on @file{stderr} the list of optimization passes that are turned
Index: dumpfile.c
===================================================================
--- dumpfile.c	(revision 247809)
+++ dumpfile.c	(working copy)
@@ -57,9 +57,9 @@  static struct dump_file_info dump_files[
    0, 0, 0, 0, 0, false, false},
   {".ipa-clones", "ipa-clones", NULL, NULL, NULL, NULL, NULL, TDF_IPA,
    0, 0, 0, 0, 0, false, false},
-  {".tu", "translation-unit", NULL, NULL, NULL, NULL, NULL, TDF_TREE,
+  {".tu", "translation-unit", NULL, NULL, NULL, NULL, NULL, TDF_LANG,
    0, 0, 0, 0, 1, false, false},
-  {".class", "class-hierarchy", NULL, NULL, NULL, NULL, NULL, TDF_TREE,
+  {".class", "class-hierarchy", NULL, NULL, NULL, NULL, NULL, TDF_LANG,
    0, 0, 0, 0, 2, false, false},
   {".original", "tree-original", NULL, NULL, NULL, NULL, NULL, TDF_TREE,
    0, 0, 0, 0, 3, false, false},
@@ -69,6 +69,8 @@  static struct dump_file_info dump_files[
    0, 0, 0, 0, 5, false, false},
 #define FIRST_AUTO_NUMBERED_DUMP 6
 
+  {NULL, "lang-all", NULL, NULL, NULL, NULL, NULL, TDF_LANG,
+   0, 0, 0, 0, 0, false, false},
   {NULL, "tree-all", NULL, NULL, NULL, NULL, NULL, TDF_TREE,
    0, 0, 0, 0, 0, false, false},
   {NULL, "rtl-all", NULL, NULL, NULL, NULL, NULL, TDF_RTL,
@@ -115,7 +117,7 @@  static const struct dump_option_value_in
   {"missed", MSG_MISSED_OPTIMIZATION},
   {"note", MSG_NOTE},
   {"optall", MSG_ALL},
-  {"all", ~(TDF_RAW | TDF_SLIM | TDF_LINENO | TDF_TREE | TDF_RTL | TDF_IPA
+  {"all", ~(TDF_KIND_MASK | TDF_RAW | TDF_SLIM | TDF_LINENO
 	    | TDF_STMTADDR | TDF_GRAPH | TDF_DIAGNOSTIC | TDF_VERBOSE
 	    | TDF_RHS_ONLY | TDF_NOUID | TDF_ENUMERATE_LOCALS | TDF_SCEV
 	    | TDF_GIMPLE)},
@@ -282,15 +284,11 @@  get_dump_file_name (struct dump_file_inf
     dump_id[0] = '\0';
   else
     {
-      char suffix;
-      if (dfi->pflags & TDF_TREE)
-	suffix = 't';
-      else if (dfi->pflags & TDF_IPA)
-	suffix = 'i';
-      else
-	suffix = 'r';
-
-      if (snprintf (dump_id, sizeof (dump_id), ".%03d%c", dfi->num, suffix) < 0)
+      /* LANG, TREE, RTL, IPA.  */
+      char suffix = "ltri"[TDF_KIND (dfi->pflags)];
+      
+      if (snprintf (dump_id, sizeof (dump_id), ".%03d%c", dfi->num, suffix)
+	  < 0)
 	dump_id[0] = '\0';
     }
 
@@ -657,13 +655,13 @@  int
 gcc::dump_manager::
 dump_enable_all (int flags, const char *filename)
 {
-  int ir_dump_type = (flags & (TDF_TREE | TDF_RTL | TDF_IPA));
+  int ir_dump_type = TDF_KIND (flags);
   int n = 0;
   size_t i;
 
   for (i = TDI_none + 1; i < (size_t) TDI_end; i++)
     {
-      if ((dump_files[i].pflags & ir_dump_type))
+      if (TDF_KIND (dump_files[i].pflags) == ir_dump_type)
         {
           const char *old_filename = dump_files[i].pfilename;
           dump_files[i].pstate = -1;
@@ -684,7 +682,7 @@  dump_enable_all (int flags, const char *
 
   for (i = 0; i < m_extra_dump_files_in_use; i++)
     {
-      if ((m_extra_dump_files[i].pflags & ir_dump_type))
+      if (TDF_KIND (m_extra_dump_files[i].pflags) & ir_dump_type)
         {
           const char *old_filename = m_extra_dump_files[i].pfilename;
           m_extra_dump_files[i].pstate = -1;
Index: dumpfile.h
===================================================================
--- dumpfile.h	(revision 247809)
+++ dumpfile.h	(working copy)
@@ -35,6 +35,8 @@  enum tree_dump_index
   TDI_original,			/* dump each function before optimizing it */
   TDI_generic,			/* dump each function after genericizing it */
   TDI_nested,			/* dump each function after unnesting it */
+
+  TDI_lang_all,			/* enable all the language dumps.  */
   TDI_tree_all,                 /* enable all the GENERIC/GIMPLE dumps.  */
   TDI_rtl_all,                  /* enable all the RTL dumps.  */
   TDI_ipa_all,                  /* enable all the IPA dumps.  */
@@ -47,21 +49,25 @@  enum tree_dump_index
    the DUMP_OPTIONS array in dumpfile.c. The TDF_* flags coexist with
    MSG_* flags (for -fopt-info) and the bit values must be chosen to
    allow that.  */
-#define TDF_ADDRESS	(1 << 0)	/* dump node addresses */
-#define TDF_SLIM	(1 << 1)	/* don't go wild following links */
-#define TDF_RAW  	(1 << 2)	/* don't unparse the function */
-#define TDF_DETAILS	(1 << 3)	/* show more detailed info about
+#define TDF_LANG	0	/* is a lang-specific dump.  */
+#define TDF_TREE	1	/* is a tree dump */
+#define TDF_RTL		2	/* is a RTL dump */
+#define TDF_IPA		3	/* is an IPA dump */
+#define TDF_KIND_MASK   3
+#define TDF_KIND(X) ((X) & TDF_KIND_MASK)
+
+#define TDF_ADDRESS	(1 << 3)	/* dump node addresses */
+#define TDF_SLIM	(1 << 4)	/* don't go wild following links */
+#define TDF_RAW  	(1 << 5)	/* don't unparse the function */
+#define TDF_DETAILS	(1 << 6)	/* show more detailed info about
 					   each pass */
-#define TDF_STATS	(1 << 4)	/* dump various statistics about
+#define TDF_STATS	(1 << 7)	/* dump various statistics about
 					   each pass */
-#define TDF_BLOCKS	(1 << 5)	/* display basic block boundaries */
-#define TDF_VOPS	(1 << 6)	/* display virtual operands */
-#define TDF_LINENO	(1 << 7)	/* display statement line numbers */
-#define TDF_UID		(1 << 8)	/* display decl UIDs */
-
-#define TDF_TREE	(1 << 9)	/* is a tree dump */
-#define TDF_RTL		(1 << 10)	/* is a RTL dump */
-#define TDF_IPA		(1 << 11)	/* is an IPA dump */
+#define TDF_BLOCKS	(1 << 8)	/* display basic block boundaries */
+#define TDF_VOPS	(1 << 9)	/* display virtual operands */
+#define TDF_LINENO	(1 << 10)	/* display statement line numbers */
+#define TDF_UID		(1 << 11)	/* display decl UIDs */
+
 #define TDF_STMTADDR	(1 << 12)	/* Address of stmt.  */
 
 #define TDF_GRAPH	(1 << 13)	/* a graph dump is being emitted */