Re: [PATCH v2 3/N] Transform TDF_{lang,tree,ipa,rtl} to dump_kind enum.

[Richard Biener]

On Fri, May 26, 2017 at 3:25 PM, Martin Liška  wrote:
> On 05/26/2017 02:31 PM, Richard Biener wrote:
>> Well, then we have to go without the constructor.  Runtime init isn't
>> an option IMHO.
>> Just use some macros for the eye candy, will you?
>>
>> Richard.
>
> Yep. I'm testing following eye candy patch.
> Ready to install after tests?

Yes.

Thanks,
Richard.

> Martin

Re: [PATCH v2 3/N] Transform TDF_{lang,tree,ipa,rtl} to dump_kind enum.

[Martin Liška]

On 05/26/2017 02:31 PM, Richard Biener wrote:
> Well, then we have to go without the constructor.  Runtime init isn't
> an option IMHO.
> Just use some macros for the eye candy, will you?
> 
> Richard.

Yep. I'm testing following eye candy patch.
Ready to install after tests?

Martin
>From 5339db7288e6ba28b65b166bc3f008c08c765834 Mon Sep 17 00:00:00 2001
From: marxin 
Date: Fri, 26 May 2017 15:08:45 +0200
Subject: [PATCH] Remove dump_file_info::dump_file_info and replace it with
 macro.

gcc/ChangeLog:

2017-05-26  Martin Liska  

	* dumpfile.c: Use newly added macro DUMP_FILE_INFO.
	* dumpfile.h (struct dump_file_info): Remove ctors.
---
 gcc/dumpfile.c | 42 ++
 gcc/dumpfile.h |  7 ---
 2 files changed, 14 insertions(+), 35 deletions(-)

diff --git a/gcc/dumpfile.c b/gcc/dumpfile.c
index 233e6b19916..b8bda3c9183 100644
--- a/gcc/dumpfile.c
+++ b/gcc/dumpfile.c
@@ -47,41 +47,27 @@ FILE *alt_dump_file = NULL;
 const char *dump_file_name;
 dump_flags_t dump_flags;
 
-CONSTEXPR dump_file_info::dump_file_info (): suffix (NULL), swtch (NULL),
-  glob (NULL), pfilename (NULL), alt_filename (NULL), pstream (NULL),
-  alt_stream (NULL), dkind (DK_none), pflags (), alt_flags (0),
-  optgroup_flags (0), pstate (0), alt_state (0), num (0), owns_strings (false),
-  graph_dump_initialized (false)
-{
-}
-
-dump_file_info::dump_file_info (const char *_suffix, const char *_swtch,
-dump_kind _dkind, int _num):
-  suffix (_suffix), swtch (_swtch), glob (NULL),
-  pfilename (NULL), alt_filename (NULL), pstream (NULL), alt_stream (NULL),
-  dkind (_dkind), pflags (), alt_flags (0), optgroup_flags (0),
-  pstate (0), alt_state (0), num (_num), owns_strings (false),
-  graph_dump_initialized (false)
-{
-}
+#define DUMP_FILE_INFO(suffix, swtch, dkind, num) \
+  {suffix, swtch, NULL, NULL, NULL, NULL, NULL, dkind, 0, 0, 0, 0, 0, num, \
+   false, false}
 
 /* Table of tree dump switches. This must be consistent with the
TREE_DUMP_INDEX enumeration in dumpfile.h.  */
 static struct dump_file_info dump_files[TDI_end] =
 {
-  dump_file_info (),
-  dump_file_info (".cgraph", "ipa-cgraph", DK_ipa, 0),
-  dump_file_info (".type-inheritance", "ipa-type-inheritance", DK_ipa, 0),
-  dump_file_info (".ipa-clones", "ipa-clones", DK_ipa, 0),
-  dump_file_info (".original", "tree-original", DK_tree, 3),
-  dump_file_info (".gimple", "tree-gimple", DK_tree, 4),
-  dump_file_info (".nested", "tree-nested", DK_tree, 5),
+  DUMP_FILE_INFO (NULL, NULL, DK_none, 0),
+  DUMP_FILE_INFO (".cgraph", "ipa-cgraph", DK_ipa, 0),
+  DUMP_FILE_INFO (".type-inheritance", "ipa-type-inheritance", DK_ipa, 0),
+  DUMP_FILE_INFO (".ipa-clones", "ipa-clones", DK_ipa, 0),
+  DUMP_FILE_INFO (".original", "tree-original", DK_tree, 3),
+  DUMP_FILE_INFO (".gimple", "tree-gimple", DK_tree, 4),
+  DUMP_FILE_INFO (".nested", "tree-nested", DK_tree, 5),
 #define FIRST_AUTO_NUMBERED_DUMP 3
 
-  dump_file_info (NULL, "lang-all", DK_lang, 0),
-  dump_file_info (NULL, "tree-all", DK_tree, 0),
-  dump_file_info (NULL, "rtl-all", DK_rtl, 0),
-  dump_file_info (NULL, "ipa-all", DK_ipa, 0),
+  DUMP_FILE_INFO (NULL, "lang-all", DK_lang, 0),
+  DUMP_FILE_INFO (NULL, "tree-all", DK_tree, 0),
+  DUMP_FILE_INFO (NULL, "rtl-all", DK_rtl, 0),
+  DUMP_FILE_INFO (NULL, "ipa-all", DK_ipa, 0),
 };
 
 /* Define a name->number mapping for a dump flag value.  */
diff --git a/gcc/dumpfile.h b/gcc/dumpfile.h
index a60777f344d..fda8d712c43 100644
--- a/gcc/dumpfile.h
+++ b/gcc/dumpfile.h
@@ -120,13 +120,6 @@ typedef uint64_t dump_flags_t;
 /* Define a tree dump switch.  */
 struct dump_file_info
 {
-  /* Constructor.  */
-  CONSTEXPR dump_file_info ();
-
-  /* Constructor.  */
-  dump_file_info (const char *_suffix, const char *_swtch, dump_kind _dkind,
-		  int _num);
-
   /* Suffix to give output file.  */
   const char *suffix;
   /* Command line dump switch.  */
-- 
2.12.2

Re: [PATCH v2 3/N] Transform TDF_{lang,tree,ipa,rtl} to dump_kind enum.

[Richard Biener]

On Thu, May 25, 2017 at 11:52 AM, Martin Liška  wrote:
> On 05/25/2017 05:35 AM, Martin Sebor wrote:
>> On 05/12/2017 07:04 AM, Martin Liška wrote:
>>> Third part removes TDF_* flags mentioned in the subject. These flags are 
>>> used
>>> to enable all passes of specific type and Nathan has recently separated 
>>> these
>>> by a new pair of macros. I hope moving these to a separate enum will help 
>>> even more.
>>
>> Just as an FYI, the addition of the user-defined constructor makes
>> the class non-trivial and so unsafe to initialize by calling memset
>> in C++ 11 and later.  In C++ 98 that GCC is compiled with, the class
>> is not a POD and so not safe to realloc (i.e., new objects must be
>> brought to life by calling a constructor).  I noticed this by testing
>> my latest patch for the new -Wclass-memassign warning but I thought
>> it might be worth noting here as well.
>
> Hi.
>
> Thanks for heads up. I'm not sure why the class is no longer POD as one can 
> define
> constructors (but not copy-ctors) in order to preserve a type to be POD?
> I'm also interested what is the new warning -Wclass-memassign about?
>
>>
>> +  /* Constructor.  */
>> +  dump_file_info ();
>> +
>> +  /* Constructor.  */
>> +  dump_file_info (const char *_suffix, const char *_swtch, dump_kind _dkind,
>> +  int _num);
>> +
>>
>> Also, making the ctor constexpr and defining it in the .c file will
>> trigger Clang -Wundefined-inline and makes the ctor unusable in other
>> sources.  AFAICS, there is no constexpr context where the constexpr
>> ctor would be needed so I'm not sure I understand its purpose.  Is
>> it constexpr to allow the static initialization of dump_files[0]?
>> (All the other elements of the array are dynamically initialized
>> by calling the other, non-constexpr ctor so that doesn't seem like
>> the answer.)
>
> Yep, that was Richi's objection to use constexpr and I know it's used
> just for the first elements of the array.
> Richi?

Well, then we have to go without the constructor.  Runtime init isn't
an option IMHO.
Just use some macros for the eye candy, will you?

Richard.

> Martin
>
>>
>> Martin
>

Re: [PATCH v2 3/N] Transform TDF_{lang,tree,ipa,rtl} to dump_kind enum.

[Nathan Sidwell]

On 05/25/2017 05:52 AM, Martin Liška wrote:

On 05/25/2017 05:35 AM, Martin Sebor wrote:

On 05/12/2017 07:04 AM, Martin Liška wrote:

Third part removes TDF_* flags mentioned in the subject. These flags are used
to enable all passes of specific type and Nathan has recently separated these
by a new pair of macros. I hope moving these to a separate enum will help even 
more.


Just as an FYI, the addition of the user-defined constructor makes
the class non-trivial and so unsafe to initialize by calling memset
in C++ 11 and later.  In C++ 98 that GCC is compiled with, the class
is not a POD and so not safe to realloc (i.e., new objects must be
brought to life by calling a constructor).  I noticed this by testing
my latest patch for the new -Wclass-memassign warning but I thought
it might be worth noting here as well.


Hi.

Thanks for heads up. I'm not sure why the class is no longer POD as one can 
define
constructors (but not copy-ctors) in order to preserve a type to be POD?


As Martin S says, it's a change from C98 to C11.  98 disallowed any ctor.

nathan

--
Nathan Sidwell

Re: [PATCH v2 3/N] Transform TDF_{lang,tree,ipa,rtl} to dump_kind enum.

[Martin Liška]

On 05/25/2017 05:35 AM, Martin Sebor wrote:
> On 05/12/2017 07:04 AM, Martin Liška wrote:
>> Third part removes TDF_* flags mentioned in the subject. These flags are used
>> to enable all passes of specific type and Nathan has recently separated these
>> by a new pair of macros. I hope moving these to a separate enum will help 
>> even more.
> 
> Just as an FYI, the addition of the user-defined constructor makes
> the class non-trivial and so unsafe to initialize by calling memset
> in C++ 11 and later.  In C++ 98 that GCC is compiled with, the class
> is not a POD and so not safe to realloc (i.e., new objects must be
> brought to life by calling a constructor).  I noticed this by testing
> my latest patch for the new -Wclass-memassign warning but I thought
> it might be worth noting here as well.

Hi.

Thanks for heads up. I'm not sure why the class is no longer POD as one can 
define
constructors (but not copy-ctors) in order to preserve a type to be POD?
I'm also interested what is the new warning -Wclass-memassign about?

> 
> +  /* Constructor.  */
> +  dump_file_info ();
> +
> +  /* Constructor.  */
> +  dump_file_info (const char *_suffix, const char *_swtch, dump_kind _dkind,
> +  int _num);
> +
> 
> Also, making the ctor constexpr and defining it in the .c file will
> trigger Clang -Wundefined-inline and makes the ctor unusable in other
> sources.  AFAICS, there is no constexpr context where the constexpr
> ctor would be needed so I'm not sure I understand its purpose.  Is
> it constexpr to allow the static initialization of dump_files[0]?
> (All the other elements of the array are dynamically initialized
> by calling the other, non-constexpr ctor so that doesn't seem like
> the answer.)

Yep, that was Richi's objection to use constexpr and I know it's used
just for the first elements of the array.
Richi?

Martin

> 
> Martin

Re: [PATCH v2 3/N] Transform TDF_{lang,tree,ipa,rtl} to dump_kind enum.

[Martin Sebor]

On 05/12/2017 07:04 AM, Martin Liška wrote:

Third part removes TDF_* flags mentioned in the subject. These flags are used
to enable all passes of specific type and Nathan has recently separated these
by a new pair of macros. I hope moving these to a separate enum will help even 
more.


Just as an FYI, the addition of the user-defined constructor makes
the class non-trivial and so unsafe to initialize by calling memset
in C++ 11 and later.  In C++ 98 that GCC is compiled with, the class
is not a POD and so not safe to realloc (i.e., new objects must be
brought to life by calling a constructor).  I noticed this by testing
my latest patch for the new -Wclass-memassign warning but I thought
it might be worth noting here as well.

+  /* Constructor.  */
+  dump_file_info ();
+
+  /* Constructor.  */
+  dump_file_info (const char *_suffix, const char *_swtch, dump_kind 
_dkind,

+ int _num);
+

Also, making the ctor constexpr and defining it in the .c file will
trigger Clang -Wundefined-inline and makes the ctor unusable in other
sources.  AFAICS, there is no constexpr context where the constexpr
ctor would be needed so I'm not sure I understand its purpose.  Is
it constexpr to allow the static initialization of dump_files[0]?
(All the other elements of the array are dynamically initialized
by calling the other, non-constexpr ctor so that doesn't seem like
the answer.)

Martin

Re: [PATCH v2 3/N] Transform TDF_{lang,tree,ipa,rtl} to dump_kind enum.

[Martin Liška]

On 05/17/2017 02:20 PM, Richard Biener wrote:
> On Fri, May 12, 2017 at 3:04 PM, Martin Liška  wrote:
>> Third part removes TDF_* flags mentioned in the subject. These flags are used
>> to enable all passes of specific type and Nathan has recently separated these
>> by a new pair of macros. I hope moving these to a separate enum will help 
>> even more.
>>
>> Patch can bootstrap on ppc64le-redhat-linux and survives regression tests.
>>
>> Ready to be installed?
> 
> Ok with the constexpr issue solved.

Thanks. I'll separate that into 2 patches, where first will extract CONSTEXPR 
macro
to ansidecl.h.

Currently running regression tests.

Martin

> 
> Thanks,
> Richard.
> 
>> Martin

Re: [PATCH v2 3/N] Transform TDF_{lang,tree,ipa,rtl} to dump_kind enum.

[Richard Biener]

On Fri, May 12, 2017 at 3:04 PM, Martin Liška  wrote:
> Third part removes TDF_* flags mentioned in the subject. These flags are used
> to enable all passes of specific type and Nathan has recently separated these
> by a new pair of macros. I hope moving these to a separate enum will help 
> even more.
>
> Patch can bootstrap on ppc64le-redhat-linux and survives regression tests.
>
> Ready to be installed?

Ok with the constexpr issue solved.

Thanks,
Richard.

> Martin

Re: [PATCH v2 3/N] Transform TDF_{lang,tree,ipa,rtl} to dump_kind enum.

[Richard Biener]

On Fri, May 12, 2017 at 3:04 PM, Martin Liška  wrote:
> Third part removes TDF_* flags mentioned in the subject. These flags are used
> to enable all passes of specific type and Nathan has recently separated these
> by a new pair of macros. I hope moving these to a separate enum will help 
> even more.
>
> Patch can bootstrap on ppc64le-redhat-linux and survives regression tests.
>
> Ready to be installed?

 /* Table of tree dump switches. This must be consistent with the
TREE_DUMP_INDEX enumeration in dumpfile.h.  */
 static struct dump_file_info dump_files[TDI_end] =
 {
...
+  dump_file_info (),

that will make it a runtime constructor unless you make dump_file_info
() constexpr.

ISTR we do have some C++11 stuff abstracted as macros.  Ah, no, just
manually in vec.h.

need to look closer at the rest.  Sth I notice is that it would be
nice to have dump_file
replaced by sth more elaborate (dump_file_info itself?), one could easily add an
implicit conversion to FILE * to make transition easy.

Well, one change at a time ;)

dump_flags, dump_file -> cdump ...

Richard.

> Martin

[PATCH v2 3/N] Transform TDF_{lang,tree,ipa,rtl} to dump_kind enum.

[Martin Liška]

Third part removes TDF_* flags mentioned in the subject. These flags are used
to enable all passes of specific type and Nathan has recently separated these
by a new pair of macros. I hope moving these to a separate enum will help even 
more.

Patch can bootstrap on ppc64le-redhat-linux and survives regression tests.

Ready to be installed?
Martin
>From 33ae777e1dc3b6026299fd19b5f2d80e60f3ca4a Mon Sep 17 00:00:00 2001
From: marxin 
Date: Wed, 10 May 2017 14:32:26 +0200
Subject: [PATCH 3/3] Transform TDF_{lang,tree,ipa,rtl} to dump_kind enum.

gcc/ChangeLog:

2017-05-12  Martin Liska  

	* cfgrtl.c (rtl_verify_edges): Remove usage of TDF_RTL.
	* dumpfile.c (dump_register): Use new enum dump_kind.
	(get_dump_file_name): Likewise.
	(dump_enable_all): Likewise.
	(dump_switch_p_1): Likewise.
	(enable_rtl_dump_file): Remove usage of TDF_RTL.
	* dumpfile.h (enum dump_kind): New enum type.
	(struct dump_file_info): Create constructor and
	format fields and comments.
	* passes.c (pass_manager::register_one_dump_file):
	Use num dump_kind.
	* statistics.c (statistics_early_init): Likewise.
	* tree-ssa-loop-prefetch.c (dump_mem_details): Replace
	TDF_TREE with TDF_SLIM.
	(gather_memory_references_ref): Likewise.
---
 gcc/cfgrtl.c |  2 +-
 gcc/dumpfile.c   | 84 +-
 gcc/dumpfile.h   | 87 +++-
 gcc/loop-unroll.c|  4 +-
 gcc/passes.c | 10 ++---
 gcc/statistics.c |  2 +-
 gcc/tree-ssa-loop-ivcanon.c  |  2 +-
 gcc/tree-ssa-loop-prefetch.c |  8 ++--
 8 files changed, 111 insertions(+), 88 deletions(-)

diff --git a/gcc/cfgrtl.c b/gcc/cfgrtl.c
index ead82d23166..aad02921392 100644
--- a/gcc/cfgrtl.c
+++ b/gcc/cfgrtl.c
@@ -2527,7 +2527,7 @@ rtl_verify_edges (void)
 	&& JUMP_P (BB_END (bb))
 	&& CROSSING_JUMP_P (BB_END (bb)))
   {
-print_rtl_with_bb (stderr, get_insns (), TDF_RTL | TDF_BLOCKS | TDF_DETAILS);
+	print_rtl_with_bb (stderr, get_insns (), TDF_BLOCKS | TDF_DETAILS);
 error ("Region crossing jump across same section in bb %i",
bb->index);
 err = 1;
diff --git a/gcc/dumpfile.c b/gcc/dumpfile.c
index ee967d825f4..ccc6db3077a 100644
--- a/gcc/dumpfile.c
+++ b/gcc/dumpfile.c
@@ -47,37 +47,41 @@ FILE *alt_dump_file = NULL;
 const char *dump_file_name;
 dump_flags_t dump_flags;
 
+dump_file_info::dump_file_info (): suffix (NULL), swtch (NULL), glob (NULL),
+  pfilename (NULL), alt_filename (NULL), pstream (NULL), alt_stream (NULL),
+  dkind (DK_none), pflags (), alt_flags (0), optgroup_flags (0),
+  pstate (0), num (0), owns_strings (false), graph_dump_initialized (false)
+{
+}
+
+dump_file_info::dump_file_info (const char *_suffix, const char *_swtch,
+dump_kind _dkind, int _num):
+  suffix (_suffix), swtch (_swtch), glob (NULL),
+  pfilename (NULL), alt_filename (NULL), pstream (NULL), alt_stream (NULL),
+  dkind (_dkind), pflags (), alt_flags (0), optgroup_flags (0),
+  pstate (0), num (_num), owns_strings (false), graph_dump_initialized (false)
+{
+}
+
 /* Table of tree dump switches. This must be consistent with the
TREE_DUMP_INDEX enumeration in dumpfile.h.  */
 static struct dump_file_info dump_files[TDI_end] =
 {
-  {NULL, NULL, NULL, NULL, NULL, NULL, NULL, 0, 0, 0, 0, 0, 0, false, false},
-  {".cgraph", "ipa-cgraph", NULL, NULL, NULL, NULL, NULL, TDF_IPA,
-   0, 0, 0, 0, 0, false, false},
-  {".type-inheritance", "ipa-type-inheritance", NULL, NULL, NULL, NULL, NULL, TDF_IPA,
-   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_LANG,
-   0, 0, 0, 0, 1, false, false},
-  {".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},
-  {".gimple", "tree-gimple", NULL, NULL, NULL, NULL, NULL, TDF_TREE,
-   0, 0, 0, 0, 4, false, false},
-  {".nested", "tree-nested", NULL, NULL, NULL, NULL, NULL, TDF_TREE,
-   0, 0, 0, 0, 5, false, false},
+  dump_file_info (),
+  dump_file_info (".cgraph", "ipa-cgraph", DK_ipa, 0),
+  dump_file_info (".type-inheritance", "ipa-type-inheritance", DK_ipa, 0),
+  dump_file_info (".ipa-clones", "ipa-clones", DK_ipa, 0),
+  dump_file_info (".tu", "translation-unit", DK_lang, 1),
+  dump_file_info (".class", "class-hierarchy", DK_lang, 2),
+  dump_file_info (".original", "tree-original", DK_tree, 3),
+  dump_file_info (".gimple", "tree-gimple", DK_tree, 4),
+  dump_file_info (".nested", "tree-nested", DK_tree, 5),
 #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,