[PATCH 3.1/11] Explicitly initialize the macro-generated pass fields (was Re: [PATCH 03/11] Handwritten part of conversion of passes to C++ classes)

[David Malcolm <dmalcolm at redhat dot com>]

On Mon, 2013-07-29 at 15:41 -0600, Jeff Law wrote:
> On 07/26/2013 09:04 AM, David Malcolm wrote:
> > This patch is the hand-written part of the conversion of passes from
> > C structs to C++ classes.  It does not work without the subsequent
> > autogenerated part, which is huge.
> [ ... ]
> With the changes from pipeline -> pass_manager this is fine.  As is the 
> follow-up autogenerated patch.

Thanks.

The current status is that Jeff has approved patches 1-5 for the trunk,
and patches 6-11 haven't yet been reviewed by a global reviewer.  I have
committed patches 1 and 2, having bootstrapped+tested each in turn, but
I can't commit any more of these without further review - details
follow.

I had only bootstrapped and tested the combination of all 11 patches
together, so I've been attempting to test the approved patches.  For
reference:
  * Patch 3 is the handwritten part of the conversion of passes to C++
    classes
  * Patch 4 is the autogenerated part
  * Patch 5 adds -fno-rtti to the testsuite when building plugins
  * Patch 6 is the not-yet-approved patch currently named "Rewrite how
instances of passes are cloned" (but that's not all it does, see below).

I had thought that the combination of patch 3 + 4 + 5 would work as a
unit, and hoped to commit each of these after testing the combination of
(3, 4, 5), but upon attempting a bootstrap I found two bugs:

  (a) patch 3 adds an gcc_assert to the pass_manager constructor to
ensure that pass instance fields are NULL when created, to ensure that
the macro-driven logic is correct.   However, the instance fields
haven't been explicitly initialized at that point, leading to sporadic
assertion failures.  This wasn't an issue for the full patch series
since patch 11 adds an operator new for pass_manager, allocating it from
the GC heap, using the *cleared* allocator:
   void*
   pass_manager::operator new (size_t sz)
   {
     return ggc_internal_cleared_alloc_stat (sz MEM_STAT_INFO);
   }
hence ensuring that the fields are zeroed.  So patch 3 works with patch
11, but not without.  In the meantime, I'm attaching a new patch "3.1",
which explicitly zeroes these fields early on in the pass_manager ctor.

  (b) with just these patches, although static_pass_number for every
pass instance is correct, the dumpfile *switch* numbering is wrong,
leading to numerous testsuite failures (e.g. "unrecognized command line
option '-fdump-tree-dce2'") .  Patch 6 is needed: it does the necessary
fixups at the right time to ensure that the per-pass-instance dump files
get the correct switch names (I'll add some more notes on this to that
email).

With the attached patch, the combination of patches (03, 03.1, 04, 05
*and* 06) successfully bootstraps on x86_64-unknown-linux-gnu, and all
testcases show the same results as an unpatched build (relative to
r201397).

So I'm looking for review of the attached patch, and of at least patch 6
before I can proceed with this.  AIUI, Jeff is away at the moment, so
another global reviewer would need to do it.

Thanks
Dave

>From 49d7df3c645459a0f90179dfb1a24cef459b186e Mon Sep 17 00:00:00 2001
From: David Malcolm <dmalcolm@redhat.com>
Date: Wed, 31 Jul 2013 14:20:07 -0400
Subject: [PATCH 03.1/15] Explicitly initialize the macro-generated pass fields
 to NULL

gcc/

	* passes.c (pass_manager::pass_manager): Explicitly initialize
	the macro-generated pass fields to NULL.
---
 gcc/passes.c | 22 ++++++++++++++++++++++
 1 file changed, 22 insertions(+)

diff --git a/gcc/passes.c b/gcc/passes.c
index fcbd630..c4f4f39 100644
--- a/gcc/passes.c
+++ b/gcc/passes.c
@@ -1352,6 +1352,28 @@ pass_manager::pass_manager (context *ctxt)
   GCC_PASS_LISTS
 #undef DEF_PASS_LIST
 
+  /* For safety, NULL-initialize the various fields for individual pass
+     instances (mainly so that the:
+	gcc_assert (NULL == PASS ## _ ## NUM);
+     sees an initial NULL value).
+
+     We cannot use member initializers for this since pass-instances.def
+     contains semicolons.  */
+
+#define INSERT_PASSES_AFTER(PASS)
+#define PUSH_INSERT_PASSES_WITHIN(PASS)
+#define POP_INSERT_PASSES()
+#define NEXT_PASS(PASS, NUM) PASS ## _ ## NUM = NULL;
+#define TERMINATE_PASS_LIST()
+
+#include "pass-instances.def"
+
+#undef INSERT_PASSES_AFTER
+#undef PUSH_INSERT_PASSES_WITHIN
+#undef POP_INSERT_PASSES
+#undef NEXT_PASS
+#undef TERMINATE_PASS_LIST
+
   /* Build the tree of passes.  */
 
 #define INSERT_PASSES_AFTER(PASS) \
-- 
1.7.11.7